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

[Layout][Next.js] Load the content styles for the RichText component #1670

Merged
merged 7 commits into from
Nov 23, 2023
Merged

[Layout][Next.js] Load the content styles for the RichText component #1670

merged 7 commits into from
Nov 23, 2023

Conversation

illiakovalenko
Copy link
Contributor

@illiakovalenko illiakovalenko commented Nov 21, 2023 •
edited by yavorsk
Loading

Description / Motivation

  • Introduced getContentStylesheetLink function
  • Next.js app fetches content styles in case ck-content class name is present in layout data
  • When prefetching content styles on each page without traversing layout data, the page load time increases by approximately 50ms, whereas the layout data traversal only takes around 0.6ms
  • We can't really identify if the field is RichText, so each field should be checked, if at least one field contains ck-content we can stop traversing and return a link
  • In case layout data doesn't contain ck-content the content stylesheet link is null

Testing Details

  • Unit Test Added
  • Manual Test/Other (Please elaborate)
    Added a ck-content class under RichText field of my custom component

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

@illiakovalenko illiakovalenko changed the title [LayoutCKEditor - Load the content styles for editor [Layout][Next.js] Load the content styles for the RichText component Nov 21, 2023
@illiakovalenko illiakovalenko requested a review from a team November 21, 2023 12:03
addy-pathania
addy-pathania requested changes Nov 23, 2023
View reviewed changes
Copy link
Contributor

@addy-pathania addy-pathania left a comment

Choose a reason for hiding this comment

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

Looks good.
See some comments.

addy-pathania

This comment was marked as duplicate.

Sign in to view

addy-pathania
addy-pathania approved these changes Nov 23, 2023
View reviewed changes
Copy link
Contributor

@addy-pathania addy-pathania left a comment

Choose a reason for hiding this comment

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

👍🏼

@addy-pathania addy-pathania merged commit e7fc692 into dev Nov 23, 2023
1 check passed
@addy-pathania addy-pathania deleted the feature/JSS-956 branch November 23, 2023 14:30
@yavorsk yavorsk mentioned this pull request Nov 30, 2023
Merged
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@addy-pathania addy-pathania addy-pathania approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@illiakovalenko @addy-pathania