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

Update LinkControl docs with advice to memoize value prop #54659

Merged
merged 4 commits into from
Oct 5, 2023

Conversation

getdave
Copy link
Contributor

@getdave getdave commented Sep 20, 2023

What?

Documents the best practice to memoize the value prop passed to <LinkControl>.

Addresses #54603 (comment)

Props to @Mamaduka for the inspiration and the great work to fix this Issue in Core.

Why?

LinkControl is likely to be inside other compomnents which may re-render. If value is not-memozied then LinkControl will then also have to re-render.

As LinkControl maintains it's own internal state of any changes made by the user interacting with the control prior to submitting their changes. This is synchronised with the value prop passed to the component.

By memoizing the value we avoid this.

This pattern is established in Core PRs:

How?

Add documentation.

Why don't you just remove the internal state?

If you ask this question please carefully read and evaluate the discussion in #51387 🙇

Testing Instructions

  • Check that documentation is accurate.
  • Check that the wording makes sense.
  • Check that it marries with approach in the referenced PRs

Testing Instructions for Keyboard

Screenshots or screencast

@getdave getdave added [Type] Developer Documentation Documentation for developers [Feature] Link Editing Link components (LinkControl, URLInput) and integrations (RichText link formatting) labels Sep 20, 2023
@getdave getdave requested a review from Mamaduka September 20, 2023 10:13
@getdave getdave self-assigned this Sep 20, 2023
@getdave getdave changed the title Update docs Update LinkControl docs with advice to memoize value prop Sep 20, 2023
@getdave getdave added [Type] Code Quality Issues or PRs that relate to code quality and removed [Type] Code Quality Issues or PRs that relate to code quality labels Sep 20, 2023
@getdave getdave requested a review from Mamaduka October 4, 2023 12:34
@github-actions
Copy link

github-actions bot commented Oct 4, 2023

Flaky tests detected in 6fb2d75.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6406030158
📝 Reported issues:

Copy link
Member

@Mamaduka Mamaduka left a comment

Choose a reason for hiding this comment

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

Thank you, @getdave!

@getdave getdave enabled auto-merge (squash) October 5, 2023 10:44
@getdave getdave merged commit 5735a49 into trunk Oct 5, 2023
49 checks passed
@getdave getdave deleted the add/document-link-control-value-memo branch October 5, 2023 11:26
@github-actions github-actions bot added this to the Gutenberg 16.9 milestone Oct 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Link Editing Link components (LinkControl, URLInput) and integrations (RichText link formatting) [Type] Developer Documentation Documentation for developers
Projects
Development

Successfully merging this pull request may close these issues.

2 participants