Conversation
|
Size Change: +972 B (+0.05%) Total Size: 1.96 MB
ℹ️ View Unchanged
|
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as outdated.
This comment was marked as outdated.
edb4727 to
0661e98
Compare
This comment was marked as resolved.
This comment was marked as resolved.
0661e98 to
9d781a2
Compare
|
Update help text to
Text should probably auto update to match the entity title (follow up) |
|
Important PR; nice work! I wanted to note, that I think this PR experience wise is more or less good to go, as the value UI is fairly legible and can always be improved later. So no blockers on the visuals. That said, two design options, I believe they are based past work by @jameskoster. I don’t have a preference:
|
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
|
Flaky tests detected in 3eb61ea. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/18225510631
|
|
@scruffian thanks for the review 👍
I have pushed what I hope is a fix for this. Could you pull and re-test 🙏
I think I came across this too, but I can no longer replicate it. Can you?
Agreed. Once @cbravobernal has reviewed the implementation for Nav Link and we've stablised an approved method then we should duplicate this to Submenus as well. |
Interesting but not unexpected given the scope of these changes. Nothing flagged during dev but I guess flaky tests are...well... flaky. @jeryj was working on quite a few "fix" PRs which may inadvertently resolve these. But let's keep a close eye. I don't think we need to revert this PR for flaky tests if we can wrap them up in the next few days. It's an important change so we will get these issues ironed out. |
…n the editor and on front end (#71630) * Initial commit * Post rebase fixes * Improve help text as per https://github.com/WordPress/gutenberg/pull/71630\#discussion_r2397328702 * Extract simple suffic component * Remove erroneous isSelected condition for url binding condition * Initial e2e test coverage * Test unlinking entity * Improve scope of test * Improve error handling in binding * Implement a11y fixes as per https://github.com/WordPress/gutenberg/pull/71630/files\#r2398999057 * Improve tests to provide coverage for a11y relationship between unsync button and input * Extract simple component for help text and add tests * LinkControl: Add accessibility association for unlink button - Add aria-describedby association between unlink button and help text - Generate unique helpTextId using useInstanceId hook for entity use cases - Create custom help text with proper semantic HTML (p tag) - Add tooltip support using showTooltip and label props - Position help text inside search-input-wrapper with custom styling - Use standard design system variables for consistent styling - Update button label to 'Unsync and edit' for better UX - Add comprehensive unit tests for accessibility association - Fix existing test regression for button name change Addresses a11y feedback: 'The Synced with the selected page help text underneath this input's aria-describedby isn't attached to the button, it's attached to an unfocusable input, so screen reader users will likely skip it. If we're in this synced state, the aria-described by text should get assigned to the button so it's accessible.' * LinkControl: Clean up URLInput and SearchInput components - Remove unused help prop from LinkControlSearchInput - Add aria-describedby support to URLInput component - Remove unused helpText variable and sprintf import - Complete the accessibility association implementation * URLInput: Remove unused ariaDescribedBy prop - Remove ariaDescribedBy from props destructuring - Remove ariaDescribedBy from inputProps - This prop was added but never actually used in LinkControl - The accessibility association is handled directly in LinkControl * Implement bding in Submenu via shared code paths * Register Submenu bindings * Revert bindings docs * Lint and fix * Remove TODOs in code * Add backport PR info * Fix autoformatting breaking changelog Unlinked contributors: warudin. Co-authored-by: getdave <[email protected]> Co-authored-by: talldan <[email protected]> Co-authored-by: scruffian <[email protected]> Co-authored-by: cbravobernal <[email protected]> Co-authored-by: jeryj <[email protected]> Co-authored-by: jasmussen <[email protected]> Co-authored-by: obenland <[email protected]> Co-authored-by: youknowriad <[email protected]> Co-authored-by: noisysocks <[email protected]> Co-authored-by: mrfoxtalbot <[email protected]> Co-authored-by: draganescu <[email protected]> Co-authored-by: retrofox <[email protected]>
|
Cherry-picked in 7a93fd5 for Gutenberg 21.8 |
|
@cbravobernal Unfortunately @jeryj noticed that this PR broke the ability to create custom links without block bindings being created. I have a fix incoming. |
|
Flaky Test fixes are being worked on here: #72132 |
|
Tracking #72059 |
|
We should have a follow up to bring this behavior to the Button block if it doesn't exist already. |


What
This PR improves the Navigation Link block in two key ways:
Closes #18345
Why
The Core Problem
The Navigation Link block has never dynamically fetched URLs for linked entities (e.g. Pages, Posts):
id: the ID of the entity.url: a static copy of the permalink at that time.This means:
Why This Is a Major Usability Issue
WYSIWYG Mismatch
Invisible Link Types
Poor Content Resilience
How
Using the Block Bindings API, we bind the
urlattribute of the block to the entity represented by theidattribute of the block through a newcore/entitybinding source.Key implementation details:
core/entitybinding source that usessource_args['key']to determine which property to return (e.g.,url)LinkControlUI to clearly indicate when a link is bound to an entityThis creates a unified approach between editor and frontend, with clear visual indicators for dynamic vs static links.
Note: After discussion with subject matter experts (@ockham and @cbravobernal), we agreed to implement a generic
core/entitybinding source for now. @ockham will investigate the possibility of normalizing this with existing data sources in the future.Proposed Followups
The following items are being pushed to immediate follow ups:
Textcontent of the Nav Link item to match the chosen entity. This is the most likely workflow.entityblock binding and utilise existing post-data and term-data bindings once they have been made more extensible (see this comment)