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

Publishing Flow accessibility #4187

Closed
afercia opened this issue Dec 27, 2017 · 28 comments · Fixed by #11543
Closed

Publishing Flow accessibility #4187

afercia opened this issue Dec 27, 2017 · 28 comments · Fixed by #11543
Assignees
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Priority] High Used to indicate top priority items that need quick attention

Comments

@afercia
Copy link
Contributor

afercia commented Dec 27, 2017

Just seen the publish dropdown has been replaced by a sidebar panel #4119, as first step towards the updated publish flow #3496 (comment)

This is a very good example of why accessibility specialists recommend to design with accessibility in mind since the beginning. I see this new feature as an UI improvement compared to the previous dropdown but I also see many accessibility concerns haven't been addressed at all, not even the most basic ones. At this point of the project, I'd expect to see new features merged with some basic accessibility already built in.

A few things noticed so far:

Placement:
The publish panel is placed outside of any of the 3 landmark regions, and also can't be navigated to with the Ctrl+backtick shortcut. Since the introduction of landmarks to indicate 3 main editor regions (toolbar, content, sidebar), any UI component must be placed within one of these regions. Landmark regions can be reviewed and adjusted, of course.

Keyboard navigation:
Activating Publish/Update in the toolbar makes the Publish panel slide in but there are several tab stops between the toggle button and the panel. As mentioned several times on other issues, see for example #469 (April 20th!), controls that expand a panel should be placed immediately before the panel itself, or focus should be handled programmatically. The first option is always preferable. Now, when the publish panel opens, the underlying default sidebar controls are still focusable, though visibly hidden. This produces various tab stops to hidden controls and must be avoided.

Focus loss:
Closing the panel by publishing/updating/switching to draft, makes the panel disappear and there's a complete focus loss: press Tab again and keyboard navigation starts again from scratch from the document root.

Also, disabling focused buttons causes a focus loss UI controls should never get disabled while they're focused. In this kind of scenario, either focus should be moved programmatically to a logical, predictable, expected, place, or the control shouldn't get disabled in the first place. Worth reminding in at least one other place, we're keeping a button focusable and just make it noop.

@afercia afercia added the [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). label Dec 27, 2017
@youknowriad
Copy link
Contributor

As always I don't agree that we should work on accessibiity from the start since the UI is changing. For example, the Panel won't be closed right after clicking publish or update, it won't even open for updates after all the iterations. So most of the accessibility work would have been lost time.

That said, yes, accessibility is important and once the iteration completed, I'll make sure to address all the accessibility issues as another iteration.

@afercia
Copy link
Contributor Author

afercia commented Dec 28, 2017

I agree we disagree (kindly) 🙂
As I see it, accessibility is a requirement in this project and as such, it should be implemented since the beginning, at least in a basic form. Not implementing accessibility at all when releasing a new feature leaves so many things unfinished and unpolished. At some point, all these unfinished things will need to be fixed.

What I'd really like to avoid is to be blamed that "accessibility is slowing down development" when the time to fix all these things will arrive. In my experience, accessibility as an afterthought, in the erroneous belief "it can be added later" leads always to non-optimal results. Some accessibility features (not all of them) are really part of the design process and should be addressed during the design process. That would actually save time because some, as already happened, some iterations can't be made accessible and avoiding to spend time on not-accessible implementation saves time.

@afercia
Copy link
Contributor Author

afercia commented Jan 5, 2018

Update, see also the new "Post Published" panel added in #4158

screen shot 2017-12-25 at 10 45 09

  • need to check for keyboard accessibility
  • the input field with the post URL needs an associated label, even it it's a readonly field it does need to be labeled

@afercia
Copy link
Contributor Author

afercia commented Mar 19, 2018

Report from one of the accessibility testers. Sorry for the image, she sent a PDF:

screen shot 2018-03-20 at 00 33 33

Ah well actually I can copy and paste text:

Opening the publish panel:
• Focus is on the “Publish” button but it won’t activate with control+option+space bar or Enter key, so I’m not able to publish the post.
• I got trapped in the panel, I’m not able to return to main content or the actions at the top.
• Also, I’m not able to select the “Close publish panel” button, it does not appear to be in the same tab order. To select it, I will need to pass through all the main content until focus is on the panel.

@karmatosed karmatosed modified the milestones: Merge Proposal, Merge Proposal: Accessibility Apr 12, 2018
@karmatosed karmatosed added the [Priority] High Used to indicate top priority items that need quick attention label Apr 13, 2018
@nic-bertino
Copy link

@karmatosed I think you were on to something with #4920, and I'd like to continue that conversation here while we're also considering the post accessibility.

Most importantly, you wrote:

There are visual changes but if the above isn't being noticed, what can we do to improve it? I'd love to get some ideas and sketches around this from people.

In my opinion, the publishing is a major user milestone. As such, I think the publishing process should be intentional and decoupled from the content. Mapping to what we currently have, that means having the publishing options dominate the visual space (with a clear and easy way to get back to the draft). In a best-case scenario, anything related to publishing could be accomplished there too (taxonomy, excerpts, featured image). It's your chance to review all of this document information before going live.

Here's a wireframe of what I mean:
Wireframe of a proposed publishing flow that uses a publishing page interstitial

This is essentially what happens on mobile, and I think it could be easily extrapolated to a desktop screen. Also, plugins could extend this pre-publishing screen for sharing to social, SEO check, etc in a way that isn't confined by a small slideover. Just a thought.

@afercia
Copy link
Contributor Author

afercia commented Apr 18, 2018

From an a11y perspective, an accessible modal or a "screen takeover" that behaves like a modal would be better than what we have now, where the publishing flow "slideover":

  • is placed outside the main 3 landmark regions (all content should be placed within one of these 3 regions)
  • the tab order is completely confusing with many unexpected tabs tops required before being able to get to the publishing flow box

@rianrietveld rianrietveld added the Needs Design Feedback Needs general design feedback. label Apr 23, 2018
@karmatosed karmatosed added Needs Design Needs design efforts. and removed Needs Design Feedback Needs general design feedback. labels Apr 23, 2018
@karmatosed
Copy link
Member

I'm marking as needs design as seems to me it's not feedback as much as a design here? I totally could be wrong. With regards to that, I have one point to ask, does it have to be full screen?

@afercia
Copy link
Contributor Author

afercia commented Apr 24, 2018

does it have to be full screen?

Not necessarily. From an a11y perspective the most important thing is the modal has all the a11y feature that react-modal has.

@karmatosed
Copy link
Member

karmatosed commented Apr 24, 2018

In that case if it doesn't have to be full screen I would suggest we don't change the design and look at ways to if possible add those features. I do not believe adding more content to the publishing flow like this is a good idea. Design wise, at the end I don't feel we need more functionality or a split in functionality, we just need to know it gets there and celebrate it in an appropriate manner, close the story.

@karmatosed karmatosed added Design and removed Needs Design Needs design efforts. labels Apr 24, 2018
@samikeijonen
Copy link
Contributor

Can we start with proper keyboard interactions so that users can publish the post with keyboard only?

@afercia
Copy link
Contributor Author

afercia commented May 9, 2018

Worth reminding one of the a11y features needed here is "constraining tabbing" within the publishing panel. A dedicated HoC is going to land in a pending PR about modals, see withFocusContain in #6261

@rianrietveld
Copy link
Member

@nic-bertino It seems we need to work with the existing design that is there. We should probably start trying to fix keyboard interaction, tab order, focus management, etc.

@paaljoachim
Copy link
Contributor

paaljoachim commented May 28, 2018

Hey

In regards to saving and publishing a post.

screen shot 2018-05-28 at 14 40 06

Clicking Save Draft it will for a moment show Saving and then Saved. Then go back to the words Save Draft. It really should show Saved until I do something in the document. As I might click it a few times just to be sure. When the word Saved remains then I really know that the post is saved.

Example: For instance Photoshop will show a star * next to the document title when a change has been made after the last save. Saving will remove the star until I do something in the document in which the star comes back again.

I would think the same concept should imply here. One clicks Save Draft. The words Saved are seen until the user has made a change in the post.


Save/publish status.

Another matter. At the top of the Publish panel there should be a Status check. Perhaps something similar to this (I added Status: Document saved as Draft.):

gutenberg-publish-panel-status

@karmatosed karmatosed removed the Design label May 29, 2018
@jorgefilipecosta jorgefilipecosta added the [Status] In Progress Tracking issues with work in progress label Jul 2, 2018
jorgefilipecosta added a commit that referenced this issue Jul 3, 2018
…#7552)

## Description
This commit makes a series of changes to make publish panel more accessible. The PR tries to follow the ideas discussed during WCEU contributors day and Accessibility chats on Wordpress slack.
Tries to address most parts of #4187.

## Types of changes
Adds a new Publish landmark. When publish panel is visible the publish landmark contains the Publish panel. When the panel is hidden nothing is visible, unless we focus the landmark using Gutenberg landmark navigation (control + < or control + `) or we focus the button using tab, in this cases a button to open publish panel appears.

We automatically focus the contents of the publish panel when we open it. The first element of the Panel is the publish button, so it gets focused.

We now don't render other sidebars if the publish panel is visible. The other sidebars were not visible, but they were tabbable making keyboard users experience sub-optimal.

Added aria-expanded true to the close button on publishing panel. This button allows collapsing the panel so I feel it should have this aria property.
@jorgefilipecosta
Copy link
Member

Hi @paaljoachim, thank you for sharing your thoughts. I did some tests and I think the behavior described by you of "one clicks Save Draft. The words Saved are seen until the user has made a change in the post." is implemented, we show saved until a change is made. Could you confirm if that is the case?

@jorgefilipecosta jorgefilipecosta removed their assignment Jul 16, 2018
@jorgefilipecosta jorgefilipecosta removed the [Status] In Progress Tracking issues with work in progress label Jul 16, 2018
@jorgefilipecosta
Copy link
Member

It looks like the remaining point to address is having a status in a pre-publish panel. Is that correct?

@paaljoachim
Copy link
Contributor

paaljoachim commented Jul 16, 2018

Hey @jorgefilipecosta
Right now one clicks the Save Draft. It says Saving for a second then says Saved and then goes back to the default Save Draft. It should say Saved until the user makes a change.
Instead of: Save Draft -> Saving -> Saved -> Save Draft.
It should say: Save Draft -> Saving -> Saved.
The user makes a change then the word Saved goes back to the words Save Draft.

save-draft-gutenberg


Regarding pre-publish panel.
It would be good with a Status of how the document is saved. Be it draft, published, private etc. At a quick glance the user can see how a document is saved in the Status area.

--

So those are my two issues I believe would make it clear that the document is saved and how it is saved.

@jorgefilipecosta
Copy link
Member

Hi @paaljoachim could you confirm if you have some meta-boxes in the tests you are doing?
Right now we show "Saved" until a change is made like you are describing the exception is if meta boxes exist. In that case, we stop showing saved after a small amount of time, that happens because when meta boxes are present we don't know if they were changed or they require a new save.

@paaljoachim
Copy link
Contributor

Hello @jorgefilipecosta
Yes I can confirm that I have meta boxes present that cause the above effect:
Save Draft -> Saving -> Saved -> then goes back to Save Draft.

save-draft-gutenberg-meta-boxes-present

When meta boxes are not present it shows up like this:
Save Draft -> Saving -> Saved.

save-draft-without-metaboxes-present

Many themes and plugins will add meta boxes to the screen.
So it is tricky that Gutenberg does not discover when someone has done something in the meta boxes. Perhaps we need to think a bit different about this. I believe there is an auto save function that after a specific amount of time the document is automatically saved. It could then just save autosaved. To set a revision point the user could then also click the autosaved button to just create another save point. Then go to the save/revision/action points to go back in time.

@karmatosed
Copy link
Member

Noting that the publishing flow is being worked on in #7602. I don't think the answer here is to add a new status to the document settings. I feel we can iterate the flow over add that.

@tofumatt tofumatt self-assigned this Oct 3, 2018
@tofumatt
Copy link
Member

tofumatt commented Oct 4, 2018

I think the first step here is to write some E2E tests that ensure a post can be written and published entirely using the keyboard–that'll identify any issues with the keyboard-only publishing workflow. I'll try to get on this soon.

I'll have a look at some of the focus issues mentioned in the original comment as well, as they're what I consider in the scope of this issue. The panel has changed a bit but it should be checked to see if any of the issues still remain.

@oandregal
Copy link
Member

oandregal commented Oct 5, 2018

I've been reviewing this issue and related ones to digest what's left to do into actionable tasks. I know there are some changes coming in that would affect this, but wanted to have a go-to place to keep the state of things and communicate progress. Here are my on-going notes (will update them as I continue with the audit):

Pre-publish panel

  • landmark area
  • keyboard navigation:
    • on opening the panel, focus on the first tabbable component of the panel (or something else that makes sense).
    • constrained tabbing: we already have this for the modals and popovers, and my understanding is that this slide-in panel would also fall in the constrained-tabbing category.
    • upon closing the panel, the "publish" button should get the focus back.

Post-publish panel

  • landmark area
  • keyboard navigation:
    • on opening the panel, focus on the first tabbable component of the panel (or something else that makes sense). Focus goes to the non-interactive "published" div at the moment.
    • constrained tabbing: we already have this for the modals and popovers, and my understanding is that this slide-in panel would also fall in the constrained-tabbing category.
    • upon closing the panel, return focus to the opener. Problem is that at this point the post will be published and the "publish" button disabled, so we need an alternate component to return the focus to.

Is there any outstanding issue missing?

@tofumatt tofumatt assigned oandregal and unassigned tofumatt Nov 1, 2018
@tofumatt
Copy link
Member

tofumatt commented Nov 1, 2018

@nosolosw Has there been any work on the issues you mentioned in
#4187 (comment)?

I don't think there's anything off the top of my head to add to that list, it seems a good first pass.

@oandregal
Copy link
Member

Not that I know of. I'm wrapping one task and then will start implementing these changes.

@oandregal
Copy link
Member

#11403 addresses the constrained tabbing. Managing the focus is going to take me more thinking, so we may want to merge that and iterate on focus in a separate PR.

@oandregal
Copy link
Member

#11489 addresses focusing a tabbable element when the post-publish panel is opened.

@tofumatt
Copy link
Member

tofumatt commented Nov 7, 2018

upon closing the panel, the "publish" button should get the focus back.

Sounds good.

Problem is that at this point the post will be published and the "publish" button disabled, so we need an alternate component to return the focus to.

This could be another case where we use an aria-disabled prop and remove the onClick event handler like we do for Undo/Redo when "Publish" is disabled (see: #11379). Returning focus to that button but saying it's disabled means the user's focused is at least returned to the area that opened the panel. I think that'd be fine—or at least better than what we have now.

@oandregal
Copy link
Member

Thanks for the pointer, Matt. Returning focus to the opener it's been more involved than I originally thought: we render different components for the publish button in the header settings that mount/unmount at different stages. I've started to explore a solution at #11543 but I'm still not convinced that's the right way.

@oandregal
Copy link
Member

#11543 is ready for review. It looks like we can close this issue as soon as that lands in master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Priority] High Used to indicate top priority items that need quick attention
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants