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.

Sign up for GitHub

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

Jump to bottom

WP stories #1973

Open
wants to merge 2 commits into
base: 25299-layoutsToAPI
Choose a base branch
from
Open

WP stories #1973

wants to merge 2 commits into from

Conversation

GunkaArtur
Copy link
Contributor

@GunkaArtur GunkaArtur self-assigned this Jan 25, 2024
@GunkaArtur GunkaArtur force-pushed the 25299-layoutsToAPI branch 3 times, most recently from e86cad6 to 4c15761 Compare February 5, 2024 09:29
@GunkaArtur GunkaArtur force-pushed the 25400-WP-stories branch 2 times, most recently from 459f77b to 21a819b Compare February 6, 2024 08:25
victor-pg
victor-pg requested changes Feb 7, 2024
View reviewed changes
public/editor-client/src/types/DefaultTemplate.ts Outdated
stories: Array<StoriesTemplateWithThumbs>;
}

export type StoriesAPI = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

convert to interface

public/editor-client/src/types/DefaultTemplate.ts Outdated
thumbnailHeight: number;
}

export type StoryPages = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

convert to interface

public/editor-client/src/defaultTemplates/utils.ts Outdated
project: id,
name: title,
cat: categories.split(",").map((cat) => cat.trim()),
color: "#d7fb00",
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 ok this color to be hardcoded ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have some parts that are temporary hardcoded, color and styles, in future we will fix it

public/editor-client/src/defaultTemplates/tempConverters.ts Outdated
categories: kit.categories,
types: kit.types as KitType[]
blank,
project: blank ? pages[0].id : name,
Copy link
Collaborator

Choose a reason for hiding this comment

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

move pages[0] in some const and if you have an parser for page, then use it

const page = parser(pages[0]);

public/editor-client/src/defaultTemplates/tempConverters.ts Outdated
thumbnailHeight,
thumbnailWidth,
thumbnailSrc: `${url}/${id}.jpg`,
pro: pro ?? false,
Copy link
Collaborator

Choose a reason for hiding this comment

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

pro: !!pro

public/editor-client/src/defaultTemplates/tempConverters.ts Outdated
styles,
pro: isPro,
thumbnailSrc: `${thumbUrl}/${pages[0].id}.jpg`,
thumbnailWidth: pages[0].thumbnailWidth,
Copy link
Collaborator

Choose a reason for hiding this comment

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

same here, move pages[0] in some const

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is Layout converter and will be fixed in Layout PR

@GunkaArtur GunkaArtur force-pushed the 25400-WP-stories branch 3 times, most recently from 70ae191 to a4246a6 Compare February 7, 2024 12:11
@GunkaArtur GunkaArtur force-pushed the 25299-layoutsToAPI branch 3 times, most recently from 3391208 to 20feb9d Compare February 14, 2024 13:35
@GunkaArtur GunkaArtur force-pushed the 25400-WP-stories branch 4 times, most recently from 376a03f to 75a4ae8 Compare February 19, 2024 11:47
@GunkaArtur GunkaArtur force-pushed the 25299-layoutsToAPI branch 2 times, most recently from ec4a20a to ed86dcd Compare February 22, 2024 11:43
@GunkaArtur GunkaArtur force-pushed the 25400-WP-stories branch 3 times, most recently from f8af8c4 to 2ff72cc Compare February 26, 2024 12:53
@GunkaArtur GunkaArtur force-pushed the 25400-WP-stories branch 2 times, most recently from 86b3bc9 to 1388183 Compare February 26, 2024 13:54
@GunkaArtur GunkaArtur force-pushed the 25299-layoutsToAPI branch 2 times, most recently from ff5d56a to f10764f Compare March 12, 2024 13:59
@GunkaArtur GunkaArtur force-pushed the 25400-WP-stories branch 2 times, most recently from 7341a4d to b28625c Compare March 13, 2024 08:46
@GunkaArtur GunkaArtur force-pushed the 25299-layoutsToAPI branch 2 times, most recently from ca90731 to 0e8026a Compare March 26, 2024 12:18
@GunkaArtur GunkaArtur force-pushed the 25400-WP-stories branch 3 times, most recently from 9319590 to a9b7fd9 Compare March 27, 2024 13:32
@GunkaArtur GunkaArtur force-pushed the 25299-layoutsToAPI branch 2 times, most recently from 6718ed2 to 43677e1 Compare April 23, 2024 12:55
victor-pg
victor-pg requested changes Apr 24, 2024
View reviewed changes
public/editor-client/src/api/index.ts
Comment on lines +1259 to +1261
return Json.read(data[0].pageData) as {
items: DefaultBlockWithID[];
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove casting, because of casting we don't handle some errors

as example here Json.read can return undefined
image

but you cast is as object with items inside

public/editor-client/src/api/index.ts
`${url}/get-story-page-data?project_id=${layoutId}&page_slug=${id}`
).then((r) => r.json());

const pageData = JSON.parse(data.collection);
Copy link
Collaborator

Choose a reason for hiding this comment

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

use Json.read or wrap in try/catch json parsing

public/editor-client/src/api/index.ts
Comment on lines +1291 to +1293
return pageData as {
blocks: DefaultBlock[];
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove casting

if without casting is not possible then validate it, as example if pageData will be undefined/null then return empty array

public/editor-client/src/defaultTemplates/utils.ts
name: title,
cat: categories.split(",").map((item) => item.trim()),
color,
pagesCount: Number(pagesCount),
Copy link
Collaborator

Choose a reason for hiding this comment

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

better use Num.read instead of Number(pagesCount), because Number() can return NaN

image

public/editor-client/src/defaultTemplates/utils.ts
cat: categories.length
? categories.split(",").map((cat) => cat.trim())
: [],
color: color,
Copy link
Collaborator

Choose a reason for hiding this comment

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

write short: color: color -> color

public/editor-client/src/defaultTemplates/utils.ts
return allElements;
};
export function convertToCategories(obj: Record<string, string>): Categories[] {
return Object.entries(obj).map(([_, title]) => ({
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.value(obj) instead of Object.entries(obj) to void using of _ for skip an variable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@victor-pg victor-pg victor-pg requested changes

Assignees

@GunkaArtur GunkaArtur

Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@GunkaArtur @victor-pg