Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(ministry brands): handler in editor-client #1951

Open
wants to merge 28 commits into
base: master
Choose a base branch
from

Conversation

DanielGherjavca
Copy link
Collaborator

@DanielGherjavca DanielGherjavca force-pushed the 24301-mb-move-handlers-in-editor-client branch from f1d557f to 5b88d8c Compare October 27, 2023 18:07
@@ -50,6 +51,12 @@ const api = {
searchCollectionItems,
getCollectionItemsIds
},
modules: {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is really need to be inside modules ?
beause is simple api handlers like handlers for Shopify

import { Literal } from "utils/types";
import { t } from "../utils/i18n";

export const request = (url: string): Promise<EkklesiaResponse> => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use request from editor-client/src/api/index.ts

};

export const fieldHaveParentsChildsKeys = (
key: Record<string, Literal> | EkklesiaParentsChilds
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe rename key in something like keys, because will be more readable and make short return

return "childs" in key && "parents" in key;
};

export const keysHaveSubkey = (
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make short return


export const getOption = (
obj: Record<string, Literal> | undefined
): ChoicesSync => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make short return

Comment on lines 36 to 38
title: String(value),
value: key
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

key should not be on title ?
and better use string reader for instead of String(value)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I receive response like this :
image

export const getOption = (
obj: Record<string, Literal> | undefined
): ChoicesSync => {
return obj
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use Object reader

): Promise<ChoicesSync> => {
const { key } = keys;

const { data = {} } = await request(url.concat("?module=", key));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of url.concat("?module=", key) use makeUrl function

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why file name is index.s.ts ?

@DanielGherjavca DanielGherjavca force-pushed the 24301-mb-move-handlers-in-editor-client branch 2 times, most recently from 4617569 to 1ee8f47 Compare November 1, 2023 08:26
@maxval1 maxval1 added review and removed review labels Nov 3, 2023
@DanielGherjavca DanielGherjavca force-pushed the 24301-mb-move-handlers-in-editor-client branch from 1ee8f47 to b3d36fe Compare November 10, 2023 16:18
@DanielGherjavca DanielGherjavca force-pushed the 24301-mb-move-handlers-in-editor-client branch from b3d36fe to 428debf Compare January 23, 2024 09:18
@DanielGherjavca DanielGherjavca force-pushed the 24301-mb-move-handlers-in-editor-client branch 2 times, most recently from 952a694 to 436dcb3 Compare February 7, 2024 11:29
@ViorelEremia ViorelEremia force-pushed the 24301-mb-move-handlers-in-editor-client branch from 436dcb3 to 27c021f Compare February 23, 2024 08:33
@DanielGherjavca DanielGherjavca force-pushed the 24301-mb-move-handlers-in-editor-client branch 2 times, most recently from 39805ed to 4285f11 Compare March 6, 2024 08:29
@DanielGherjavca DanielGherjavca force-pushed the 24301-mb-move-handlers-in-editor-client branch from 4285f11 to d72af09 Compare March 19, 2024 15:04
@zeleniucvladislav zeleniucvladislav force-pushed the 24301-mb-move-handlers-in-editor-client branch from d72af09 to 2f209e3 Compare March 19, 2024 15:59
@DanielGherjavca DanielGherjavca force-pushed the 24301-mb-move-handlers-in-editor-client branch from 2f209e3 to c0206f4 Compare March 20, 2024 11:29
@DanielGherjavca DanielGherjavca changed the base branch from develop to master March 20, 2024 11:30
@zeleniucvladislav zeleniucvladislav force-pushed the 24301-mb-move-handlers-in-editor-client branch from c0206f4 to 0f98999 Compare March 20, 2024 11:41
@DanielGherjavca DanielGherjavca force-pushed the 24301-mb-move-handlers-in-editor-client branch from 0f98999 to b555e1b Compare March 28, 2024 07:48
@maxval1 maxval1 removed their request for review September 20, 2024 04:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants