[NextJS] Fix for bug in RichText component - links not reinitialised when content changes #1503
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Subsequent pages don't have client side routing Added a useEffect dependancy to the NextJs RichText component. This fixes an issue where where the useEffect() hook doesn't run when navigating to a page that contains a RichText component after clicking an internal link in a RichText component on another page to cause the navigation. Description: 1. Open a page that contains a RichText component that has an <a> tag that links to a page within the same site (i.e. clicking the link fires the routeHandler() function within the richText component that performs a client side navigation) 2. When the RichText component on the new page renders, it doesn't run the useEffect hook so the event handler doesn't get attached to any internal links within the RichText component 3. Clicking on any of the embedded links causes a whole page refresh as the event handler isn't attached, so no client side routing occurs.
… the embedded links.
pschofield
changed the title
pschofield
changed the title
pschofield
changed the title
|
Hi @pschofield ! Thanks for the report, and big thanks for the PR. I've added this to our backlog, we'll triage and see what to do with this next. |
art-alexeyenko
requested changes
Jun 6, 2023
There was a problem hiding this comment.
@pschofield This looks good.
Can I ask you to add a Changelog entry, and then we'll merge it?
art-alexeyenko
approved these changes
Jun 6, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description / Motivation
When the RichText component content changes, the useEffect() hook is not re-run as it has no dependancies. This means the initializeLinks() function is not run so any embedded internal links within the component will not have the appropriate click event handler. This causes any link within the component to cause a full page load rather than client side routing occuring. This change adds the "hasText" property (essentially the rich text) as a dependancy for the useEffect() hook, ensuring the initializeLinks() function is run when the content changes, enabling client side routing to occur.
Testing Details
Unit test added.
Manual testing involved applying the proposed changes to a local dev instance of Sitecore 10.2 and 10.3 and observing the appropriate event handler is applied to internal links within the rich text component, both on first render but also after the content changes
Types of changes
This PR fixes Bug 1502:
NextJs RichText doesn't allow client side routing when content changes #1502