Try/add page name to document settings [IN PROGRESS] #25386
Conversation
jeyip
force-pushed
the
try/add_page_name_to_document_settings
branch
from
147371b
to
9e52497
Compare
|
Size Change: +338 B (0%) Total Size: 1.2 MB
ℹ️ View Unchanged
|
ZebulanStanphill
added
the
Needs Accessibility Feedback
There was a problem hiding this comment.
It's worth noting that I'm working on #25353, which serves a similar purpose and would probably conflict with this PR.
| <Button | ||
| onClick={ onToggle } | ||
| className={ classnames( | ||
| 'edit-site-document-actions__label', | ||
| 'edit-site-document-actions__title', | ||
| { | ||
| 'is-active': isTitleActive, | ||
| } | ||
| ) } | ||
| aria-haspopup="true" | ||
| aria-expanded={ isOpen } | ||
| onKeyDown={ openOnArrowDown } | ||
| label={ __( 'Change document settings.' ) } | ||
| showTooltip | ||
| > | ||
| { documentTitle } | ||
| </Button> |
There was a problem hiding this comment.
I don't think the label attribute works if you also have a nested text label. Additionally, it is an accessibility anti-pattern (i.e. something you should avoid) to use current state to label a button. The visible label should just be what's in the label attribute right now.
Also, you might be able to shorten the label to just "Document settings".
There was a problem hiding this comment.
Additionally, it is an accessibility anti-pattern to use current state to label a button. The visible label should just be what's in the label attribute right now.
An aria-label should override any button text regarding accessibility. The visible text should be fine as a basic name as long as we aria-label with the action?
There was a problem hiding this comment.
ARIA attributes are supposed to be a last resort, not a band-aid to let one give any visible label to a button and call it accessible. A <button> should, if at all possible, use a visible text label that actually functions as a label. Accessibility doesn't just apply to screen readers; it applies to all methods of interaction.
While the efforts towards accessibility have been significant, there has been a strong dependence on methods we consider to be anti-patterns. First among these is a heavy use of ARIA attributes. ARIA, while sometimes necessary, should be considered only as a final option when no native HTML interactions are available.
This reflects a problem in the design process: beginning with custom controls (such as the “switch toggle”), then attempting to make them accessible rather than beginning with standard controls and making a functional interface.
https://make.wordpress.org/accessibility/2018/10/29/report-on-the-accessibility-status-of-gutenberg/
The accessibility team has wanted issues like #470 to be fixed for over 3 years now. Only now is it looking like that particular issue will be resolved. (See #25170 and #24024.) We shouldn't be introducing more buttons that have the same problem but even less of a reason to not use a proper label.
There was a problem hiding this comment.
An
aria-labelshould override any button text regarding accessibility. The visible text should be fine as a basic name as long as wearia-labelwith the action?
@Addison-Stavlo as I just explained on #25085 (comment) a mismatch between the visible text and the aria-label is a huge barrier for some users and violates WCAG Label in Name.
Also, totally agree with @ZebulanStanphill that ARIA should be used as last resort and also in that case should be used appropriately. Always prefer meaningful visible text, please.
Also, since the underlying issue in #470 is being discussed since more than 3 years now, I'd expect designers and developers in this project to be aware of that and to share information when new designers / developers join the project, otherwise repeating over and over the same recommendation is going to be unsustainable for the accessibility team. I do realize this part is more related to process but, alas, it appears the process has big room for improvements.
There was a problem hiding this comment.
Thanks for the insight, and thank you for the explanation there on that issue @afercia. It is definitely helpful.
I think a large barrier we run into that makes naming buttons after their actions difficult is the available real estate in the areas they need to go. For example in the available room in this top bar can be extremely limited in tablet/mobile views as well as with the top-toolbar mode activated. So much so we can't afford to put Template / Template Part names side by side and need to stack them vertically, and something likeOpen Template: ${template_name}'s settings or something similar would not be an option here either. With limited room, how do we go about adding more interactions while still making them accessible to all accessibility flows? 🤔 (I don't expect an explicit answer, this is just something I am trying to ponder as I learn more about accessibility)
There was a problem hiding this comment.
I'd expect designers and developers in this project to be aware of that and to share information when new designers / developers join the project, otherwise repeating over and over the same recommendation is going to be unsustainable for the accessibility team.
I totally get the frustration here. :) I think this probably applies to so many things across the block editor. For example, a specific data store might cache data in a very specific way for performance reasons, but a new developer to the project won't understand that context. So they could easily make a change which circumvents that caching, causing a performance regression. (That's me, I made a change like that!)
I don't think there is really any sort of onboarding process at all for new devs, other than reading the documentation and code (which is fairly poor, unfortunately.) I wonder if there is a sustainable way to share this information, since I imagine there are a lot of similar examples. 🤔 Because that would definitely be great for both you and me in this situation! I think at least partially, some of these issues are just an ongoing learning process for everyone.
Part of that is Gutenberg contributions are really accessible to a broad range of folks: it doesn't require much of anything to start trying to contribute. So there are lots of contributors with varying ranges of experience on different topics, which means we don't really have that shared base of common skills or knowledge. :/ Which goes back into what you said: would it be helpful to create a way to generate a baseline of knowledge for new contributors? ANYWays, we're getting pretty meta here. :p
I think the space issue @Addison-Stavlo mentions is going back to the root designs we are working with here. I know that y'all have brought up this topic tons of times in the discussions about these designs. Point being, it'd be really nice to all get on the same page about how exactly this is going to work, rather than splitting this discussion across several issues.
And part of that being, we're exploring and creating some prototypes for what these might look like, so I don't think we have to treat this PR as if we are trying to do something outside of the discussion so far. We're very happy to incorporate improvements that we reach consensus on! I think in this case, it is difficult as a dev to incorporate improvements if we aren't completely sure what this interaction should look like after all of the suggestions are in place. 🤔 I basically don't want to get into a place where we implement X, and which you disagree on, so we implement Y, which design disagrees on, etc.
There was a problem hiding this comment.
I think part of the problem here is that you're trying to conflate two things that should be separate: an indicator of the current template title, and a button to change the template's title/url/etc. Trying to have one UI element serve as both probably won't work.
The purpose of the template settings button is to open an interface to edit the template settings. So it should be labeled accordingly, and it's position in the UI would logically be somewhere in the top bar.
The indicator of the current template's title, however, is something different. And I don't think it belongs in the top bar... at least not in the same row as everything else. It's already weird that there's a left half and a right half to the top bar. I think adding a middle section would just confuse the layout even more.
I think what we're looking for is a sort of "status bar". Something that is designed to present information to the user first and foremost, with any interactivity being a "nice to have" that isn't treated as the primary way to do anything.
Maybe this bar could look something like this:
Inside template part: Header | Selected block: Paragraph | Visibility: Public | Publish date: Dec 12, 2020
Perhaps this status bar could be placed below the top bar or maybe even above it. Maybe it could take the place of the current footer bar? I'm not sure.
I also had another idea for presenting "status" info, which I'll quote from #25353 (comment):
Maybe we ought to consider adding a "document status/overview" popover/modal/something that lists out important details of the document (e.g. visibility, publish state, SEO status), with "Edit" links to jump to the parts of the UI dedicated to changing those details. Maybe we could even combine it with the "Content structure" tool (the one that shows word count and stuff like that)? Just throwing ideas out there.
There was a problem hiding this comment.
And I don't think it belongs in the top bar...
I think you have some good ideas, but I will note that #25085 specifically for exposing these names in the top bar has been generally agreed upon and added to the next phase of the Site Editors infrastructure development #24818 . We are just moving forward with that infrastructure and UI development agenda.
I do welcome your concerns and ideas, however this PR may not be the best place to raise concerns with whether or not we should show the names here or in a new status bar, as most of the eyes involved in that consensus and decision may not see them. Nothing is final and even if we (together as core) move forward to put the labels in this region, we are still able to discuss better solutions and reiterate. But for now, there needs to be a starting point to assess and start iterating from and putting them here seems to be the current consensus for that starting point.
There was a problem hiding this comment.
Thanks @Addison-Stavlo !
I think a large barrier we run into that makes naming buttons after their actions difficult is the available real estate in the areas they need to go.
Well, generally any UI should be designed around the content, not the other way around 🙂 Trying to "squeeze" controls and text into a limited space is a clear sign the UI has some problems in the first place. I do realize this feedback doesn't propose an alternative solution. Maybe, some lateral thinking could help and i fit was up to me I'd ask myself whether the top bar is really the right place for these control. Giving all the problems we're facing, I'd say it is not.
| const { page } = useSelect( ( select ) => { | ||
| const { getPage } = select( 'core/edit-site' ); | ||
| return { page: getPage() }; | ||
| } ); |
There was a problem hiding this comment.
If you're just going to get one thing from useSelect, you can simplify this and avoid any destructuring.
| const { page } = useSelect( ( select ) => { | |
| const { getPage } = select( 'core/edit-site' ); | |
| return { page: getPage() }; | |
| } ); | |
| const page = useSelect( ( select ) => select( 'core/edit-site' ).getPage() ); |
Given that there are no document settings in the site editor yet, I think we won't conflict for now. 😅 One could see a future where this item in the header opens a document settings dialogue modal, if that is indeed the interaction we move towards for document settings. For the time being, in the site editor, we are exploring the specific items which ought to make up document settings in the context of multiple entities (starting with the basics). I believe it has been mentioned in some other issues that with the site editor, we get an opportunity to improve the hierarchy and design of these settings, so we currently plan to move towards that incrementally from the FSE side of things. |
| ); | ||
| } } | ||
| renderContent={ () => ( | ||
| <div |
There was a problem hiding this comment.
I think it would be helpful to split out the actual settings area into a separate component, even at this early stage :) (including the page selector above)
jeyip
force-pushed
the
try/add_page_name_to_document_settings
branch
from
e5bb192
to
be81066
Compare
noahtallen
force-pushed
the
try/add-document-settings-in-header
branch
from
202d122
to
f4927d4
Compare
| <span style={ { marginRight: '12px' } }> | ||
| Name | ||
| </span> |
There was a problem hiding this comment.
This should be a <label> associated via the for prop with the <input> that follows it. You can use useInstanceId to help make a unique id for the <input>.
There was a problem hiding this comment.
Thanks @ZebulanStanphill I'm still working on the main functionality of the PR, but I'll be sure to circle back and address this too before the next cycle of code review 👍
|
Thanks for the staying involved in this PR everyone! We will not be making page name and page slug editable in the site editor for now. We can return to this whenever "template editing" is better developed. |
Description
Addresses #25586
Document settings are being added to the site editor in the center of the block editor topbar. This PR adds content to the document settings dropdown, whereby the user can change the page title (if the page context exists).
How has this been tested?
Screenshots
Types of changes
New feature
Checklist: