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

[Editing Integration] Render clientScripts / clientData #1800

Merged
merged 3 commits into from
May 21, 2024
Merged

[Editing Integration] Render clientScripts / clientData #1800

merged 3 commits into from
May 21, 2024

Conversation

illiakovalenko
Copy link
Contributor

@illiakovalenko illiakovalenko commented May 20, 2024 •
edited
Loading

Description / Motivation

Introduced EditingScripts component to render clientScripts / clientData in editing. Pages requires scripts and data to be rendered in the app to enable editing features.
Used html script tag instead of next.js Script component since Script can't prerender the data if it's not used in a next.js custom _document.tsx file

Testing Details

  • Unit Test Added
  • Manual Test/Other - Tested manually using mocked layout data. Local dev / prod.

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 requested a review from a team May 20, 2024 13:31
art-alexeyenko
art-alexeyenko approved these changes May 20, 2024
View reviewed changes
Copy link
Contributor

@art-alexeyenko art-alexeyenko 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, but I have one concern.
Should EditingScripts be rendered at the end of the page? We have no control over what comes in client scripts. If it's our older scripts from ExpEditor days, they may cause problems when not places at the end of

@illiakovalenko
Copy link
Contributor Author

illiakovalenko commented May 20, 2024 •
edited
Loading

@art-alexeyenko Pages will access script by id (clientData), and other scripts (clientScripts) just need to fetch 3rd party scripts
I asked Stanislav about order, if scripts can be fetched asynchronous, he said that once scripts loaded, editing will be enabled
Do you know any example when it can break the page? I thought that there is no difference where Scripts will be rendered, since the request should be sent at the same time
(Upd. may be you are talking about sync scripts? I think it would make sense if we would have this problem here)

@art-alexeyenko
Copy link
Contributor

@illiakovalenko on second look through Pages, I think we'll be fine.
There were some issues in previous Sitecore versions, exclusive to Experience Editor. It failed to render correctly when scripts were not added at the end of tag.
But I see that Pages' scripts are loaded at the top of the page - so our implementation should work OK too.

@illiakovalenko illiakovalenko merged commit ba70759 into feature/editing-integration May 21, 2024
1 check passed
@illiakovalenko illiakovalenko deleted the feature/JSS-1908 branch May 21, 2024 07:10
@yavorsk yavorsk mentioned this pull request May 23, 2024
Merged
8 tasks
yavorsk added a commit that referenced this pull request May 23, 2024
@yavorsk @illiakovalenko
@addy-pathania @sc-ruslanmatkovskyi @Rmatkovsky
[Editing integration] Merge editing integration feature branch (#1802)
f24581e 
* [sitecore-jss-react] [sitecore-jss-nextjs] Add support for chrome's hydration for fields (#1773)

* render field metadata for Text field component; introduce field metadata component - wip

* rename FieldMetadata module, add unit test for Text component, add comments

* add field metadata component to Date, Image and File field components; include unit tests

* add field metadata component to link and richtext field components, include unit tests

* update FieldMetadata interfaces to prevent build errors in sitecore-jss-nextjs; component update

* export fieldmetadata component and interfaces from sitecore-jss-react

* add metadata component for nextjs link field component; include unit test

* add field metadata component to nextimage component; small fix in link field component

* unit tests for FieldMetadata

* update unit test

* introduce getFieldMetadataMarkup function and used in the field components; add unit test

* update changelog

* react - use higher order component to wrap metadata around field components

* update nextjs components to use metadata wrapper hoc; aadjust unit tests

* adjust unit tests and fix File component

* adjust image field tests; include check for media property in metadata wrapper

* some types updates

* some unit tests adjustments and metadata wrapper component update

* some FieldMetadata related renamings

* add unit test for RichText nextjs component

* update changelog

* update changelog pull request

* some type updates

* reenable file tests

* update function description

Co-authored-by: Illia Kovalenko <[email protected]>

* minor variable renaming

Co-authored-by: Illia Kovalenko <[email protected]>

* remove unnecessary commented line

* remove unnecessary undefined check

* move FieldMetada interfaces to base package; extract metadata proptypes

* move FieldMetadata under enchancments

* added some descriptions

* move and rename FieldMetadata to layout submodule of base package

* rename FieldMetadata component

* add tsdoc description for fieldmetadata component

* conditionally forwardRef in fieldMetadata

* two separate withFieldMetadata functions based on if used with forwardRef

* single withFieldMetadata function with forwardref parameter

* update with metadata unit test to test the whole structure of markup

* withMetadata refactoring wip

* Adjusted withFieldMetadata generic type

* update unit test

* wip - refactor field metadata hoc

* Updates

* Updated unit tests, simplified types

* Update

* Expose withFieldMetadata as a part of nextjs sdk

* Updated PropTypes

* Removed extra asserts

* remove media property from propTypes

---------

Co-authored-by: Illia Kovalenko <[email protected]>
Co-authored-by: illiakovalenko <[email protected]>

* [sitecore-jss-react] [Editing Integration] Add support for chrome hydration for Placeholders (#1776)

* conditinally emit edit chromes metadata for placeholders, rendering

* refactor PlaceholderWithMetadata component

* update tests

* update changelog

* remove extra lines

* remove editMode mock data

* update unit test, change code tag chrometype

* remove only

* refactor as per new requirements

* Update packages/sitecore-jss/src/layout/models.ts

Co-authored-by: Illia Kovalenko <[email protected]>

* Update packages/sitecore-jss-react/src/components/PlaceholderMetadata.tsx

Co-authored-by: Illia Kovalenko <[email protected]>

* Update packages/sitecore-jss-react/src/components/PlaceholderMetadata.tsx

Co-authored-by: Illia Kovalenko <[email protected]>

* Update packages/sitecore-jss-react/src/components/PlaceholderMetadata.tsx

Co-authored-by: Illia Kovalenko <[email protected]>

* Update CHANGELOG.md

Co-authored-by: Illia Kovalenko <[email protected]>

* refactor unit test

* change naming of props and types

* add test for placeholder

* update PlaceholderMetadata to render a component

* add test for missing component

* fix nextjs placeholder

* remove context

* reset nextjs placeholder

* update changelog

* update test data

* change PlaceholderMetadata implmentation

* Placeholder metadata refactor - wip

* update tests, refactor Placeholder component

* remove redundant data

* update unit test - wip

* move tests to placeholder, refactor placeholder

* refactor tests

* refactor placeholdermetadata component, update tests

* refactor placeholder tests, update jsdoc

* update nextjs types

* remove keys from code

* fix nextjs placeholder props

* change deprecated statement

* update deprecated 2

---------

Co-authored-by: Illia Kovalenko <[email protected]>
Co-authored-by: yavorsk <[email protected]>

* [templates/nextjs-sxa] SXA components has been reworked with supporting metadata (#1788)

* [templates/nextjs-sxa] SXA components has been reworked with supporting metadata approach

---------

Co-authored-by: Ruslan Matkovsky <[email protected]>

* [Editing Integration] Placeholder chromes hydration (Update - Part 2) (#1792)

* fix unit tests, update upgrade guide

* update changelog

* update missing component test

* remove only

* update changelog, upgrade guide

* remove only

* Update CHANGELOG.md

Co-authored-by: Illia Kovalenko <[email protected]>

* Update CHANGELOG.md

Co-authored-by: Illia Kovalenko <[email protected]>

* Update CHANGELOG.md

Co-authored-by: Illia Kovalenko <[email protected]>

* refactor placeholder tests

* update changelog

---------

Co-authored-by: Illia Kovalenko <[email protected]>

* [sitecore-jss-react] [sitecore-jss-nextjs] Reconciled withSitecoreContext and Placeholder types (#1797)

* [sitecore-jss-react] [sitecore-jss-nextjs] Reconciled withSitecoreContext and Placeholder types

* Reuse default _editable_ value in withFieldMetadata component

* Updated CHANGELOG

* Updated props param

* Remove extra type

* Remove generic in Styleguide-Layout-Tabs

* [Editing Integration] Render clientScripts / clientData (#1800)

* [Editing Integration] Render clientScripts / clientData

* Updated upgrade guide, changelog

---------

Co-authored-by: Illia Kovalenko <[email protected]>
Co-authored-by: illiakovalenko <[email protected]>
Co-authored-by: Addy Pathania <[email protected]>
Co-authored-by: Ruslan Matkovskyi <[email protected]>
Co-authored-by: Ruslan Matkovsky <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@art-alexeyenko art-alexeyenko art-alexeyenko 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 @art-alexeyenko