-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Add header title to list view sidebar #61553
Conversation
Size Change: +89 B (+0.01%) Total Size: 1.74 MB
ℹ️ View Unchanged
|
cd4e0c2
to
5c2e7cb
Compare
.block-editor-inserter-sidebar__header-title { | ||
font-size: 12px; | ||
font-weight: 500; | ||
color: #6b6b6b; |
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.
@richtabor @SaxonF Had to change this from $gray-600 to meet AA contrast requirement of 4.5:1. Feel free to take this over and apply the background and text color styles you want.
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.
@jameskoster - I meant to ping you on this about the colors. I quite like your design, and we only need to tweak the colors to meet AA contrast requirements.
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.
A white background would make $gray-700 adequate, this might be a better approach?
Added before & after screenshots to the description. |
Let's not do this, it diverges from every other panel we have. To clarify: the way to solve this is to explore the space of available options, apply solutions to every panel we have, and then implement it from there. |
Yes, the intention was for it to be applied to every panel.
@jasmussen Agreed on that plan! I would love to help with this. I'm available to pair on this in realtime. I can assist on dev work and accessibility feedback and pair on design calls. How would you like to move forwards on this? |
#61837 seems like it might thread the needle, though I'll let others chime in there first. |
To me, this doesn't seem a good argument. To me, all the other panels should be changed as well, as @jeryj clearly mentioned in this PR description. Instead, let's do this. For all panels. To me, any panel is a section of content. Any section of content should be clearly identified by a heading that explains what the panel is about. For example, see the discussion about the 'block inserter' panel at #61483. With the addition of Patterns and Media, the 'Block Inserter' is not only a block inserter any longer. The current labeling and names are out of date. That panel is now more a 'Library' with content types that can be added: Blocks, Patterns, Media. On top of that, the panel header would only contain the panel name and the Close button, which makes perfectly sense and would solve the order problem for panels with tabs. #61837 wouldn't be a real solution. |
Exploring the Styles panel: Reserving the panel header to: Title and Close button. That would solve many issues including providing enough space for the 'Show button text labels' preference. See #61761 Rough screenshots, from top to bottom:
Specifically to the Styles panel header, worth reminding it uses also a 'small' version of the header that is meant to be rendered on small screens. The small version would likely be no longer necessary, allowing for some code simplification. |
Agree with @afercia. Globally applied, this would be extremely valuable for clarifying the UI. I think it would also be beneficial for development; having a visible heading that provides a name for the visible panel is extremely valuable for making reference to locations and components. |
Hi there! As I said here, I think #62343 could be a great base for this change, since it unifies sidebars. I think this is a positive change, and I would like to see it land. Question for the folks in this thread: What's blocking this, currently? How can I help unblock it? If we get consensus around this, we can then take it into account when modeling the a11y story in #62343, and then add this as a fast-follow. Or even add it directly as part of that effort, since it's not a big change. Thanks @jeryj for bringing this to my attention. |
I don't think this helps clarify the UI and instead makes a worse overall design. The close buttons are creating redundancy, interfering with the tabs, and increasing the weight of the panels in a considerable way. Can't we look at how to improve the toggle relationship instead without having to place things contiguously at the DOM? |
@mtias after doing a deeper dive into this, I think I agree with you re: close buttons. However, there are some critical a11y concerns we need to address.
|
Can you please add more context? In what way a visible title for all panels wouldn't help clarify?
I'd agree. I'd just like to remind everyone that the Close buttons between the tabs and tab panels break the Tabs ARIA pattern and we need to remove them. See #59013. Regarding the 'weight' of the UI, I'd just like to remind that many of these X close buttons are already there in the current UI. For example, I would say thie UI for the Styles panel beader brings in pretty much 'weigh' and it's not the best design in the first place: A header that only contains the panel title and the close button would add more white space, making the UI cleaner, less confusing, and also more accessible. To me, additional controls should go separately in a 'subheader'. |
@mtias - Improving the relationship to of the open/close button to the panel it opens would need the toggle button and panel to be placed contiguously in the DOM. That would allow us to remove the close button, but may create other problems. I'm happy to explore this if it's something that has support as a valid way forwards. This issue discusses the idea in more detail. |
It is important to keep in mind that, while React provides an abstraction on top of the DOM, what actually matters for all users, their browsers, and the assistive technologies some users may use is still the DOM. Anyways, I'd still strongly suggest to make all panels have a header with a visible title (a heading with an appropriate level). Any section of the UI should be identified by a meaningful heading. |
Let me see if I understood correctly: the purpose of having these be contiguous in the DOM is so that the sidebar tab stops are placed right between the disclosure button and the next button in the toolbar, correct? If so, I'd be happy to attempt a simple programmatic solution that achieves this. That way, there's no need to alter the DOM but focus is managed appropriately. |
Would a visually hidden heading work? That way it'd cover screen reader users without altering the current design, which as I argued here seems good enough as-is. |
I think moving it in the DOM would be the better long-term solution for this. I'd be concerned any programatic solution for tab keypresses wouldn't also work with a screen reader virtual cursor. |
@DaniGuardiola thanks for your thoughts and feedback. I don't think the current design helps building an application that is accessible and usable for everyone. WordPress is a pretty large web application that serves millions of users. Visually hidden headings would help only screen reader users. There's a multitute of other users needs that need to be taken into account. Some of the arguments in the last point of #61553 (comment) are interesting but seems to me they are based on a visual approach rather than a functional one. I would need to write down a book to explain why, to me, most of those arguments aren't valid and I'm not sure a pull request comment is the right place for that book 😅
The main objection I feel like making is about the 'clutter' argument. It's 20 years I've been hearing designers and developers using the 'clutter' argument but that's an extremely subjective perception. An UI may be perceived as 'cluttered' by some users and may not by others. It's really about personal perception. Instead, content structure and information architecture are objective facts. I'm not sure I understand how adding visible headings may ever add cognitive load. I'd argue the opposite case, i.e. panels without headings, do add cognitive load because each time users open a panel they miss a visible confirmation they're in the right place. Rather than basing this conversation on presonal opinions, I'd encourage everyone to do some research and have a look at the largest, most poopular web applications around and observe what they do with their panels. All the applications that are designed to serve millions of users I observed around do use visible headings for their panels. I could point to Google Docs, Google Drive, Microsoft Team, etx. but there's plenty of examples around. Panels with headings are so common in those web applications that I'd even call this pattern an industry standard. I'm not sure I understand why WordPress wants do reinvent the wheel, again, and go in the opposite direction by hiding or removing headings. |
A few years ago, something similar was discussed regarding the purpose of the 'Spotlight mode' option and how to extend it. Currently, it is only meant to 'Focus on one block at a time'. At some point it was proposed to make it a switch to a 'lighter UI mode' instead. This isn't uncommon, for example Gmail does have layout options like 'Default', 'Comfortable', and 'Compact'. However, such kind of alternative UI modes would likely introduce complexity amd maintenance cost. It could be done but there should be some more rigorous design and development process as in:
That would be a huge effort. I'm not sure this project has the resources to achieve such a goal. Instead, making the UI accessible for everyone would be much simpler, as long as the design is made accessible since the beginning. |
My understanding is that we are going to try this approach: #61837 so I'll close this PR for now. |
Even if #61837 would solve the Close button issue, it won't address other issues emerged in this PR discussion. For example:
I'll try to summarize all the considerations from this PR in a new issue. |
Thanks, I think that's a good way forward. |
Alternative to #61514.
Fixes #59013.
This code is not ready, but should give an idea of what the experience would be like. It is only implemented for the list view and inserter sidebars. The Styles and Settings sidebars have the old style so you can compare the behaviors more easily. Before merging a change like this, we would need to address all the sidebars.
What?
Fixes accessibility issues where focus order must match visual order, and also not be between the tabs and tab panel.
Why?
Accessibility.
How?
Moves Close button to before the tabs and add header title.
Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast