-
Notifications
You must be signed in to change notification settings - Fork 639
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
Advanced link fields #16268
Merged
Merged
Advanced link fields #16268
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
gcamacho079
requested changes
Dec 6, 2024
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.
- The “Advanced” toggle button can’t be operated via keyboard. This should use a
button
element with the following attributes:aria-expanded
that toggles betweentrue
/false
on expand/close, and anaria-controls
value that references the ID of the advanced field container - URL type and value inputs should have explicit visible labels
- The
aria-describedby
reference on the inputs with the “More info” doesn’t exist. For example, the following issue is found on the URL Suffix input: 'ARIA attribute element ID does not exist on the page: aria-describedby="fields-link1-url-suffix-instructions"'.
In this case, I recommend adding anaria-hidden
element in the heading or input container with the referenced ID and duplicating the instruction text inside. While it may cause some confusion that the instructions are hidden behind the tooltip and read as part of the input description, it’s a better experience than requiring users to navigate back to the toggle tip to access the instructions. - The
input
/heading
containers require some extra padding when viewing at 320px - I noticed when I add a value to the URL Suffix input and save the entry, then return to the entry, the class, ID, and rel inputs end up with random values (see screenshot below).
@gcamacho079 Thanks! Addressed everything but the individual labels – that’s not a regression by this PR so we’ll address it separately. |
[ci skip]
Very cool! Thanks @brandonkelly |
@Michael-Paragonn Feel free to post that as a new discussion! |
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
Adds a new “Advanced Fields” setting to Link fields, where developers can choose which “advanced” fields they want to be available to authors. (Target and URL Suffix have both been moved into there.)
When any of them are selected, inputs will get an “Advanced” toggle, and each of the advanced fields will be hidden under it. (Collapsed by default.)
Related issues