-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
[WIP] dynamic titles and URLs for Nav block items #46891
base: trunk
Are you sure you want to change the base?
Conversation
Size Change: +74 B (0%) Total Size: 1.51 MB
ℹ️ View Unchanged
|
The problem here is now that if you enter custom label text for your link it will be overwritten again by the data returned from the API. We need to
To do this we probably never actually want to set either Then if there is an existing |
We should probably also disallowed custom editing of the Link if the link is dynamically generated from the ID. Rather we should simply force LinkControl to show the value of the link but not allow you to edit it. This would be consistent because it will be very confusing to be able to edit a link which references an entity whilst also having that link dynamically kept in sync with the underlying entity. |
We will also have to make the same dynamic look up for other types of referenced entities such as terms and media (basically anything supported by LinkControl). |
// If the new link value has an ID then it's an internal link so do not set the URL. | ||
// This will be dynamically generated from the ID. | ||
// Passed `url` may already be encoded. To prevent double encoding, decodeURI is executed to revert to the original string. | ||
const updatedUrl = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This causes the LinkControl to display the Link to empty
message when you first pick a link and the post hasn't yet resolved.
The experience I get with this is that when I edit the link name and then save, the nav is refreshed and my changes are lost: Screen.Recording.2023-01-06.at.17.00.03.mov |
Pinging @jasmussen @jameskoster and @SaxonF for some input! |
Just a note to say I think the design problem that needs consideration is how the Link Control interface can display the state of a selected "entity". For example, if you search and then select a Page from the list of results then how does the edit interface look for that? Should it differ from the example where you have manually entered a URL (e.g. The objective is that if a link is to an internal WP entity (e.g. a Post) then that URL of that link should be dynamically retrieved and displayed. Currently the code and UX of the Link UI relies on a URL and will fail if one isn't provided. Therefore whilst we look up the URL the Link Control interface looks broken. We need a means of communicating "this link is to an entity" vs "this link is a manually entered URL". I advise testing this PR as it will showcase some of the issues which can be difficult to describe in text form. |
My instinct is yes, they should be different. Internal content should probably have a more token-like apperance and not be editable at all (except to swap entries). Whereas a custom link should just be inputs. At the moment both are mixed which I agree feels a bit flaky. |
It seems like the issue at hand would be improved by this design: That is, each item is more of a "token" you link to, rather than just a loose association between title and URL. I think I shared this design also on a link dialog issue, but I forgot which one. In case one of you remember, I'd appreciate it 🙏 |
I agree with this 👍 The designs are a good step. They focus on the link creation UI. However, from an implementation perspective what I'd really find useful to see are the states for when
🙇 |
I can look at this. @jasmussen any chance you can find the Figma for the above? |
Quick pass at the edit link UI. This just adjusts so url field value acts more like a token rather than input, and after clicking edit we use the existing UI for entering page/url value. OnBlur reverts back to current page. edit-link-demo.mp4 |
397d7a9
to
51fbbaf
Compare
Yes, this is the Figma file I worked in, but feel free to extract it somewhere more ergonomic. |
Your quick pass looks good! It reminds me we could benefit from a new edit icon that works a bit better in these utilitarian contexts. I made this one for a separate conversation, but I'm not sure it still threads the needle: Something to look at separately. Do we need to show the URL in the "resting" state? If there's an affordance to edit it, perhaps just Page icon plus "Home" label would make it feel more like a token as discussed? Can always show the URL as a tooltip if need be. We probably shouldn't change the popover appearance (border, shadow, radius) as part of this effort, can discuss that separately if need be. Nice work! |
Here's an updated concept considering some of the other items listed in #35073:
How do we feel about this? Does the cog work for the settings toggle button, or should we use the |
I now centralised all design discussion in #47310 |
This comment was marked as off-topic.
This comment was marked as off-topic.
Coming back to this. Here are the requirements to allow for these dynamic updates. The UI needs to reflect the fact that if you select a link to a This UX would mean that if the URL of the referenced post changes then it's simple for us to dynamically update the Now if the user opts to edit the |
51fbbaf
to
f8a1cfb
Compare
Flaky tests detected in 9128d72. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5977073360
|
@SaxonF Are you still happy with the design route from #46891 (comment)? cc @richtabor |
Would it be possible to add an anchor to a link while keeping it dynamic? |
@getdave Im not completely up to date with the Link UI but #49091 (comment) is the more recent interpretation of that concept. ie input becomes a dynamic token where appropriate. @richtabor is probably the better person to make a decision on whether that approach still makes sense or not. (sorry Rich :D ) |
This is a very good point and one which would require careful consideration. Thank you. |
The UX infrastructure we put in place in this PR will also allow this to be rolled out to the rich text implementation of links. |
The current solution is mainly focused on the Therefore I am wondering about two things:
|
Next steps here are to continue to enhance the Link UI with support for links which represent an entity. In such circumstances the The main blocker is how to allow authors to add a internal anchor link to a link of this type without severing the connection to the entity. We also need to consider how this might apply to other scenarios outside of Navigation such as inline links in Rich Text which also save an |
Noting that an alternative approach might be to take advantage of the proposed attribute sources in the Blocks API to connect the various block attributes with the equivalent Post fields |
What?
Currently a WIP
Closes #18345
Why?
Menus in classic themes will automatically sync entity (post/pages...etc) based links with the underlying entity.
For example if I create a link to the Page "Dave's Page" then no URL is stored an the information about the Page is dynamically populated.
If I then go to the "Pages" area in WP Admin, update "Dave's Page" to "Bobs Page", and then return to the Menus page, my menu item will have updated to reflect that change. That is its Label and URL will now be the same as the updated page.
Currently the Nav block does not exhibit this behaviour.
How?
Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast
Screen.Capture.on.2023-01-04.at.20-58-32.mp4