Skip to content
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

Navigation: OffCanvasEditor: Accessibility review #46939

Closed
10 of 15 tasks
scruffian opened this issue Jan 6, 2023 · 26 comments
Closed
10 of 15 tasks

Navigation: OffCanvasEditor: Accessibility review #46939

scruffian opened this issue Jan 6, 2023 · 26 comments
Labels
[Block] Navigation Affects the Navigation Block [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes).

Comments

@scruffian
Copy link
Contributor

scruffian commented Jan 6, 2023

What problem does this address?

If we make the OffCanvasEditor the default experience for the navigation block remove the "experiment" wrapper from the Nav block offcanvas editor, we need to ensure that any accessibility concerns are addressed.

What is your proposed solution?

Address all the Todos below.

We have this issue which has one proposal, but it would be good to raise more issues so we can address them ASAP.

Todos

Update: additional issues have been identified as part of a follow up review.

The following Issues should be developed and tested using a screen reader:

Context

The offcanvas editor repurposes the standard list view and displays it within the inspector controls sidebar for the Navigation block. Users are then able to create/edit nav menus using a customised list view tree interface.

The two calls for testing from Make Blog are available at:

cc @alexstine @tellthemachines @ndiego @afercia

@scruffian scruffian added the [Block] Navigation Affects the Navigation Block label Jan 6, 2023
@ndiego
Copy link
Member

ndiego commented Jan 6, 2023

cc @colorful-tones

@colorful-tones
Copy link
Member

I’m lacking a good deal of context here. I would be happy to test and provide any feedback but I’m not sure where to look to test.

Are we talking about a newer navigation editorial flow and doing an audit of the component(s) that allow for it?

@joedolson
Copy link
Contributor

@colorful-tones I'm assuming that this is about assessing the accessibility of the off canvas Navigation editor as enabled at Gutenberg > Experiments, which moves the editing of navigation menus into the sidebar, rather than within the design canvas.

Based on my initial look, there's a bit more to be done than accessibility work - are there relevant PRs that complete the work more significantly? I only looked in Gutenberg 14.9 for my initial look, but it seemed pretty incomplete.

@alexstine
Copy link
Contributor

alexstine commented Jan 7, 2023

Here is a live audit. Please do not take my comments personally, I just wish this had been thought out better before all this work. I am strongly of the opinion that this should not go in to Core.

Thanks.

video1552526668.mp4

Notes from video (transcribed by @getdave):

  • on block insertion Nav block doesn't announce anything.
  • Open Navigaiton List View button in block toolbar
    • wording isn't clear enough to describe the feature/utility.
    • activation doesn't transfer focus to the offcanvas panel which is the intent.
    • should be a toggle. Must convey current open/closed state (see primary list view feature implementation in editor for example of how this should behave).
    • Must be possible to hide the list view for screen readers.
  • Initially focusing the list view doesn't provide suitable context about the purpose of the feature. Not sure what to do.
  • Add block isn't descriptive enough without context. What does this do? Instead Add block to Navigation menu would be preferable.
  • Block inserter:
    • Order of block items in inserter doesn't always prioritise Custom Link. This is confusing as an initial experience.
    • Block inserter dialog has random tab stop. This needs to be removed.
    • Remove Browse All
    • Lacking a close button.
  • Inserting block transfers focused back to canvas - this is a regression. The focus should (and was) remain in the List View so context is retained 😞
  • Moving from auto (Page List menu) to having a Nav Menu created when you insert a block is confusing without context. We need to provide context about the state change and why this happened.
  • When entering the block there is no context as to the menu were are editing. It should explain whether it's auto menu or one of the menus we've created. Also perhaps it should announce whether the menu is "saved" or in "draft"?
  • Open List View button should be a toggle and announce whether the List View is currently open (or not).
  • List View menu selector (top right 3 dots)
    • needs more context - "Choose a Navigation menu".
    • needs to announce which menu is currently selected. "You are currently editing: %MENU NAME%".
  • The accessible Name of the List View should be Navigation Menu: %%NAV MENU NAME%%. Currently it's something about "Application" which is confusing.

@tellthemachines
Copy link
Contributor

I'm afraid I haven't been following the development work on this experiment very closely, and not sure what the intentions behind some of these changes are, so I'm going to start with some questions 😅

  • To clarify, does "off canvas editor" mean both the "Menu" sidebar section and the separate "Navigation" sidebar in the site editor, or are there different names for each of these? They both seem to have the same functionality.
  • Does "make the OffCanvasEditor the default experience" just mean moving this work out of experimental status, or will there be further changes to how the Nav block currently works? As far as I can tell it's still possible to edit the block directly from the canvas as before, so what does it mean for the off canvas experience to be "default"?

I enabled the experiment and had a quick play with it. As in Alex's video above, I though the "Open navigation list view" button didn't do anything. It was only on checking the code that I realised it's meant to open the sidebar. My sidebar was already open when I clicked it, so it seemed broken.

What could be done to improve that?

  • It would be good if at least clicking the button transferred focus to the relevant area of the sidebar.
  • Ideally, the button would say something like "edit navigation from sidebar" or something like that. It should avoid the word "open" because it's not an open/close toggle. If the sidebar is already visible, it doesn't do anything.

Similarly, and also mentioned in Alex's video, when clicking "edit" on a Nav item from the sidebar, it would be good to keep focus in the sidebar (maybe direct it to the top section where the block name together with the "got to parent" button?).

In the site editor, the Navigation sidebar has the same problem described in #15322: essentially, it's inaccessible to keyboard and screen readers. This is a wider problem for the site editor though, because it also affects global styles.

@talldan
Copy link
Contributor

talldan commented Jan 10, 2023

The issue should probably have some testing instructions.

Here are some things that I've spotted:

  • Each item in the off canvas is announced as a link, but those links don't do anything when selected (the functionality was intentionally turned off, but the element doesn't indicate this). Not sure what the best solutions to this is, possibly the href shouldn't be defined, but it might be weird that they're links.
  • The 'Edit block' (pencil) button doesn't seem to receive focus when pressing the right arrow while the block name is focused, only when pressing the left arrow from the block settings menu. (fixed by Fix TreeGrid keyboard nav skipping Edit Block button in off canvas editor #46998)
  • The appender doesn't have the correct aria-posinset value (it's a minor thing as this isn't announced, but it'd be nice to fix it). As Alex mention it would be good if the appender had some indication of context, blocks in List View have an aria-description like this - "Block 2 of 3, Level 1", which help give screenreader users an idea of order and nesting level, some extra information along these lines might be good. There might also be more that can be done, like announcing what the parent block is and that this appends to the end.
  • On a similar note there's no way to add links to submenus because the appender only shows at root level. That might be a problem if this is to become the primary way to edit menus.
  • There are some missing options for some blocks (Page List children don't have an "Edit block" button), screen reader users may wonder why this is. It also makes the grid style keyboard navigation confusing, if I press the up arrow from from the block settings of the first page list item, I land on the edit button for the parent page list block, which means I didn't traverse the structure as a grid. I think it'd be better to show an aria-disabled version of the "edit block" button with an appropriate description.

It'd be good to make a list of the things Alex mentioned too. There are some bigger ones, like the way focus is transferred to the canvas, that might be a challenging one to solve.

@getdave
Copy link
Contributor

getdave commented Jan 10, 2023

For additional context I've updated the Issue description with links to the x2 calls for testing that were made on Make Blog for this feature over the past few months.

@getdave
Copy link
Contributor

getdave commented Jan 10, 2023

@alexstine Thank you for taking the time to provide an overview here. We really appreciate it.

We will dive into all aspects of accessibility and have been planning to address this from the get go of the project.

I just wish this had been thought out better before all this work

For context, as we were unsure whether the feature would provide a good experience for users at all we committed to building a working experiment first. This involved prioritising features over fidelity at all levels (including accessibility).

Now the x2 calls for testing have provided us with confidence the feature has value we are turning our attention to all aspects of fidelity including:

  • code quality
  • accessibility
  • visual aesthetics
  • usability

Whilst we haven't ignored any of these aspects, we recognise that all of them will need significant attention prior to inclusion of the feature in WP Core.

We have now paused all work on new features for the offcanvas and are focused solely on ensure the offcanvas is polished for all users.

I am strongly of the opinion that this should not go in to Core.

Could I respectfully ask you to explain why you hold this opinion? What specifically makes you feel this is unsuitable? Or is it that just in its current state this is unsuitable?

Bear in mind that offcanvas is largely based upon the existing <ListView> component which is already in core so any a11y fixes we find for offcanvas will ultimately end up improving the <ListView> (indeed we are refactoring towards as single implementation code wise).

Contributors working on this feature and more than happy to discuss/address any concerns fully.


Update: i've watched your video in full and taken notes as best I could (I trust you won't mind that I updated your original comment with the notes?). Having watched this (as far as it is possible) I fully appreciate how frustrating this is. Contributors working here are fully committed to making this better.

All the notes I took will be converted into individual Issues and I will cross link as Todos on this Issue. We will then work through them all.

@getdave
Copy link
Contributor

getdave commented Jan 10, 2023

@talldan I went through all your points and created Issues for those which don't overlap with @alexstine's review.

  • On a similar note there's no way to add links to submenus because the appender only shows at root level. That might be a problem if this is to become the primary way to edit menus.

You can achieve this for now using the options menu ("3 dots") for each individual node.

Whilst it might be ideal to have a roving appender at some point (you and I have discussed in the past and I agree in principle), we have to deprioritise new features in favour of fidelity of existing so this is unlikely to be added.

@getdave
Copy link
Contributor

getdave commented Jan 10, 2023

@tellthemachines Thank you for your feedback.

To clarify, does "off canvas editor" mean both the "Menu" sidebar section and the separate "Navigation" sidebar in the site editor, or are there different names for each of these? They both seem to have the same functionality.

The "Offcanvas editor" is the current name is use to describe the experience of creatuing/editing a navigation menu using a list-view type interface in the inspector controls of the Navigaiton block (only).

That experience is also being considered for the wider Site Editor sidebar but that's a separate effort by @jorgefilipecosta and others.

Would love your input on a better name for this 🙏

Does "make the OffCanvasEditor the default experience" just mean moving this work out of experimental status, or will there be further changes to how the Nav block currently works? As far as I can tell it's still possible to edit the block directly from the canvas as before, so what does it mean for the off canvas experience to be "default"?

I updated the description to clarity that this means removing the "experimental" gating around the feature and making it default in the Gutenberg Plugin. That does not mean we will merge into core. Instead the idea is that the feature will be subject to review (as all Gutenberg features are at feature freeze) and if not acceptable it will be gated behind a "is Gutenberg Plugin" flag and not shipped to WP Core.

@draganescu
Copy link
Contributor

Hey there friends. First, thanks everybody for the in depth testing - it surfaced quite a list of issues.

Second, I want to mention that the name "off canvas editor" imbues the whole feature with some extra importance that now acts as a blocker. So, to clarify:

  1. The name is nothing but a sort of way to differentiate this from the editor list view. "Off canvas editor" sounds impressive but it only means a customized list view in the navigation block's inspector, just that.
  2. It is not a replacement of "in canvas" editing. It will never be. It is a complementary feature to in canvas editing for deeply nested contexts such as the navigation block or complex block templates with locked content (maybe, at some point, in the future).
  3. "Making it a default" meant removing the experiment flag and enabling it in the navigation block by default. The primary way to edit menus with the navigation block remains the same. This feature only adds, subtracts nothing.

From the above (2) is most important as editing nested content in the canvas is extremely hard - with or without assistive technology. It is unlikely we'll land on a perfect feature in one go, so offering a way for users to manage nested content in a localized tree, with specific features to that context, starting this via the navigation block, is a great ramp to figuring it out for all the cases.

Third, @alexstine I think the "too little and chaotic context" problem of the editor is terrible and your concern is 100% valid. I also think that not shipping something indirectly affected by a problem will not fix the problem. We need to address the problem specifically.

Editing content via the inspector is not only problematic for lack of context for users of assistive technology, it's also muddying the waters as to what does each part of the editor do. However, there's only so much space and so many affordances to use. I am sure contributors from all parts (design, docs, code, trainign) will come back to this many times and improve it based on feedback to a state incomparably better, that we can't foresee today - just look at how many improvements the navigation block got once people started actually using it.

Fourth, and last 🙇🏻 , while there are still steps to be taken to make it ready for a WordPress release (fixing blocking accessibility problems, making sure the implementation architecture is sound) the improvement it produces needs to be accounted for: the purpose of the experiment was to validate the idea. The two calls for testing of the experiment showed that in many cases menus are easier to be edited in a sort of isolated tree, and this UX greatly improved the experience - far from being perfect. Removing the experimental flag will only allow more people to see and use the improvement and provide more feedback. Removing the experimental flag makes the feature available by default for Gutenberg plugin users.

@alexstine
Copy link
Contributor

@getdave

Could I respectfully ask you to explain why you hold this opinion? What specifically makes you feel this is unsuitable? Or is it that just in its current state this is unsuitable?

The biggest complication we have here is not managing focus. It is not even labeling buttons properly. List view does not complicate this much since it is 99% accessible at this point. The issue is, how do you communicate to blind users who have no point of reference on a page that some blocks are edited in the canvas and others are edited in the inspector sidebar? My goal for Gutenberg is to be seamless for keyboard and screen reader users. However, it is not seamless when you have to wait for announcement hints on how to use every block. This could be because the block has buttons or just a simple text field. It could be the fact that different blocks have different keyboard event listeners. It could be the fact that Gutenberg designers are really happy with no buttons and users just magically knowing pressing enter saves changes. It does not follow standard flow. I have said for a long time that if I require a list of keyboard shortcuts to use your accessible web app, you did something very wrong. That is kind of where Gutenberg is right now.

Adding this feature will further add to the endless confusion for blind users specifically who have to deal with never ending changes in context and having no idea what one block experience will be like from the next. It may have a placeholder, it may just be a blank block wrapper, etc.

If you can somehow convince me that you can eliminate this context problem, I will be more onboard for your idea. My doubts are heavy because this would be, in my opinion, a stretch if not impossible. What worries me further is where we go from here. If this block is a major success, are we going to end up with 5 more blocks like this? You see the problem? We cannot keep users guessing.

@draganescu

However, there's only so much space and so many affordances to use. I am sure contributors from all parts (design, docs, code, trainign) will come back to this many times and improve it based on feedback to a state incomparably better, that we can't foresee today - just look at how many improvements the navigation block got once people started actually using it.

That has been my point though. This is purely a design issue, not one that required a massive re-design and stuff that could have lasting damage on the editor as a whole if this expands to other blocks. The thing is, it never ends with 1. It may start with 1, never ends with 1. Also, I have not even addressed third-party developers yet... Sigh...

Thanks for opening all those issues. I am interested to see how this evolves. I just want my concerns very well noted that I think this could open up tons of possibilities for this to be a disaster for users with assistive tech.

@getdave
Copy link
Contributor

getdave commented Jan 11, 2023

@alexstine Thanks for taking the time to respond here. Your concerns have been noted by all contributors.

I'm hearing that you are concerned that the new Navigation list view will introduce an additional context problem for users of assistive tech.

some blocks are edited in the canvas and others are edited in the inspector sidebar

There is no compunction for users to use the list view. In canvas editing will remain the primary means of interacting with the block (and indeed all blocks).

We should consider that for other users who don't rely on keyboard or assistive tech, having a tree-based means of editing to complement in-canvas editing seems to be a big improvement. This is evidenced by the responses to the call for testing.

Of course we should ensure that the list view feature is available and accessible for all users (we're working on that), but as no user is required to use it and it is a complementary feature to the in-canvas editing perhaps the problem is less pronounced? Maybe I'm wrong?

If you can somehow convince me that you can eliminate this context problem, I will be more onboard for your idea

I don't for a minute want to downplay the wider concerns you (and others) have with Gutenberg around context switching. Having watched your video I can appreciate it's an issue but not one we can solve as part of this feature.

If we were introducing a mandatory new means of editing then I'd agree that the Nav list view could be problematic. However, as it is complementary and entirely optional I wonder if by not introducing it we are doing a disservice to those users who value it as a complementary editing mechanic.

My feeling is once we solve the focus problems in the Nav list view, the context issues you experienced will improve. The Issues I've created off the back of your video will go a long way towards that.

However, in terms of the wider problem of context switching in the editor - that's outside the scope of this new feature. It's something that needs to be solved at a fundamental level and can then propagate out across the editor.

This is purely a design issue, not one that required a massive re-design

With respect, contributors have spent years attempting to produce a solid in-canvas editing experience for nested menus and it's proven to have shortcomings at every turn. That is a major problem for the editor and thus for WordPress in general because (as you know) navigation is a key facet of any website.

The primary feedback about the Nav block for some time has been that it is extremely difficult to interact with in-canvas once levels of nesting are introduced. As I said, folks have tried to address this numerous times with little success. Indeed, I'm not even convinced it's a problem that it's possible to solve satisfactorily.

Therefore, contributors felt the time had come to explore different options to provide an improved editing UX where necessary to complement the in-canvas editing experience.

That doesn't mean it's now impossible to achieve the same tasks using the in-canvas editing. Indeed, for many users it might well be preferable to perform edits in canvas. If there are bugs around that experience (and I'm sure there are) then lets address those too.

Thanks for opening all those issues. I am interested to see how this evolves.

You're welcome. Your feedback was invaluable here. I'm hopeful that the next few weeks will see a marked improvement in the core editing experience for assistive tech (at least on this particular feature).

@joedolson
Copy link
Contributor

As long as this is an alternate and equal set of options (equal meaning that all the same features exist in both sets of controls), I think this is fine. I do think that we need to be very cautious about allowing blocks to have editing environments in the sidebar, however.

I will definitely say that when attempting to use the in-canvas navigation block as a sighted mouse user, it is extremely difficult to use. Accessibility is about ensuring that all users can create content - as it stands, neither mechanism is really usable yet, but the tree view is much more similar to past views, so it's well worth exploring.

@alexstine
Copy link
Contributor

I agree with @joedolson . As long as the exact same functionality will be implemented in both places and this is not a replacement, there is no reason to not try. With all this talk about making it available to Gutenberg plugin users, I guess I lost sight of the fact that this will simply compliment the experience. If keyboard users want to keep editing in the canvas, they are free to do so. They would never have to interact with the off canvas editing at all if they chose not to do so.

If this is correct, I think we should work through the related issues and see where we end up.

Sorry @getdave for confusing this issue so much. Most of my feedback was around showing you how many context issues currently exist in Gutenberg. While this issue is not to fix those, I wanted to prevent adding another pain point. What I now understand is off canvas editing is not replacing editing in the canvas. It is just a secondary option for users. I think that makes a context problem much less of a big deal. Although it will still be up to us to keep track of focus and what not which will likely be no easy task.

I guess I just do not understand the visual aspect of this. I was not trying to downplay it if it was a huge issue. It just kind of read to me at the time like visuals come first, off canvas editing for navigation block, no more canvas editing, we'll figure out accessibility one of these days. It makes a big difference with the understanding I have with intent. Maybe it was there all along and I just missed it.

Thanks for putting up with my negative and sometimes, slightly positive views around accessibility.

@getdave
Copy link
Contributor

getdave commented Jan 12, 2023

@alexstine @joedolson Thank you for your considered feedback. Very much appreciated.

I agree with @joedolson . As long as the exact same functionality will be implemented in both places and this is not a replacement, there is no reason to not try.

That's exactly it. A complementary interface which will hopefully make it much easier for users to edit nested menus which can be very challenging to edit "in canvas" - especially for sighted users using a mouse.

Sorry @getdave for confusing this issue so much. Most of my feedback was around showing you how many context issues currently exist in Gutenberg.

No worries. I can see we have a lot of context issues that need to be addressed in the wider project. I for one would be happy to join you in to advocating for improvements in that area.

For what it's worth I think sometimes contributors want to implement the correct UX but - even if they develop/test with a screen reader - struggle to know what that should be. If anyone who has first hand experience with that setup can help guide then changes will likely happen more quickly (in this feature at least).

Although it will still be up to us to keep track of focus and what not which will likely be no easy task.

Our basic understanding is focus should not leave the list view until the user actively choses to leave it. There will be micro decisions about how focus should be managed within the list view but we should iron those out.

I guess I just do not understand the visual aspect of this. I was not trying to downplay it if it was a huge issue.

Understood and thanks. It's a really big pain point so I'm glad we agreed it's important that it's addressed so long as it's not to the detriment of the experience for users of assistive tech.

Thanks for putting up with my negative and sometimes, slightly positive views around accessibility.

No worries and you are welcome. We're all committed to improving the editor and your right to hold us to account if the experience we've produced is not up to scratch. In hindsight I feel that as contributors we should have conducted our own accessibility audit prior to requesting your input. We'll take learnings from that and try to do better next time.

As you've probably seen we've got a load of PRs open (see PRs linked to the Issues in the description) many of which seem to have stalled due to contributors being unsure of the correct UX to implement for assistive tech. Any advice you are able to offer would be much appreciated.

@alexstine
Copy link
Contributor

@getdave I have tried to reply and leave some feedback. I am not sure how to test right now because I am on business travel until January 29th. I generally build these branches on my Mac and my Windows environment takes forever with Node.JS. Really wish @youknowriad had that CodeSandbox fully working. 👍 I will do what I can with answering questions and reviewing code diffs. When I return, happy to record another video and give everyone clear visibility in to what can be improved or what looks perfect.

Probably not the news you wanted but I recorded that video the night before I left. Hope I do not hold things up too much.

Thanks.

@getdave
Copy link
Contributor

getdave commented Jan 13, 2023

@alexstine No problem. I think the main help at this point would be answering questions on implementation details and UX. Hopefully they won't all require building branches on Node.

I'll try and get contributors to leave a list of ongoing questions in a comment here to make it easier (cc @scruffian)

@alexstine
Copy link
Contributor

Okay, I finally figured out an easier way for testing Gutenberg with WSL. I worked through a few PRs today and really enjoying the UX now. Once all outstanding PRs/issues are merged/closed, I would be happy to do another audit. I think we're in a much better position now.

@getdave
Copy link
Contributor

getdave commented Feb 2, 2023

👋 @alexstine. Glad to hear you've found an easier way. That's great news!

We would of course be grateful for any audit you could provide.

The Issues I created from your previous audit have either been fixed or they have been identified to be a wider problem requiring work outside the immediate scope of the Nav block list view project (e.g. the "Close" button for popovers).

Some items are still being worked on but it does no harm to get a fresh look at the current state and whether things have indeed improved as you suspect.

@alexstine
Copy link
Contributor

Awesome. I will try to circle back on this during the weekend. Returned to ice and snow, trying to catch up on a lot here lately.

Thanks.

@alexstine
Copy link
Contributor

Okay, accessibility audit done. Overall, there are some great improvements but I still found some concerning bugs. Really happy with progress, lets keep it moving.

Sorry about the fact that my screen reader sounds like I am in an echo chamber. Not sure what is up there.

Thanks.

video1723862502.mp4

@getdave
Copy link
Contributor

getdave commented Feb 6, 2023

Okay, accessibility audit done. Overall, there are some great improvements but I still found some concerning bugs. Really happy with progress, lets keep it moving.

Sorry about the fact that my screen reader sounds like I am in an echo chamber. Not sure what is up there.

Thanks.

video1723862502.mp4

Thank you @alexstine. We'll keep working at this. I'll dive in here shortly and check against existing bugs vs new bugs.

@getdave
Copy link
Contributor

getdave commented Feb 13, 2023

Going through this now.

@getdave
Copy link
Contributor

getdave commented Feb 13, 2023

Items from a11y review no.2

Thanks to @alexstine for again taking time to review this work.

Note: some of the issues Alex ran into have subsequently become known bugs and were making the experience unnecessarily confusing.

TL;DR

Managing focus is the major problem to work on. Most of the role and aria affordances are good but focus is very poor and is hampering the ability of users of assistive tech to use the feature

Issues

The following will be converted into Issues:

@scruffian
Copy link
Contributor Author

Now that the OffCanvasEditor is no longer used, we don't need this issue. Future improvements should happen to ListView.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Navigation Affects the Navigation Block [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes).
Projects
None yet
Development

No branches or pull requests

10 participants