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

Dialog box improvements #2263

Closed
1 of 4 tasks
PVince81 opened this issue Oct 15, 2019 · 9 comments
Closed
1 of 4 tasks

Dialog box improvements #2263

PVince81 opened this issue Oct 15, 2019 · 9 comments

Comments

@PVince81
Copy link
Contributor

PVince81 commented Oct 15, 2019

Dialogs:

  • create file

  • create folder

  • rename file/folder

  • delete file/folder confirmation when deleting inside of trashbin

  • remove confirmation for deletion when trashbin is enabled ? @pmaier1

  • design principle: always rename "ok" button to whatever action

  • always use placeholder

  • validation must not be jumpy: Jumpy error message in folder creation dialog #1906

image

@marcus-herrmann @wuenschedesign

@PVince81 PVince81 added this to the Milestone 1: Phoenix for users milestone Oct 15, 2019
@LukasHirt
Copy link
Collaborator

LukasHirt commented Oct 16, 2019

remove confirmation for deletion when trashbin is enabled

I wouldn't go in this direction. Delete confirmation should be there even if the trashbin is enabled IMHO. Given more thought to this and if we'd have also some undo then it'd make sense

@marcus-herrmann
Copy link
Contributor

marcus-herrmann commented Oct 18, 2019

Modal vs. "normal" dialog?

What discerns dialog from modal dialog is that modal ones have an interface blocking characteristic. They force the user to make a decision. So in the current use cases of Phoenix's interface, namely for renaming or even deleting a file using a modal dialog is apt. But since this issue is named "Dialog box improvements" I wanted to make sure that the difference is mentioned.

Read more about this in my blog post about (modal) dialogs.

Rendering the content behind the modal inert

If a dialog is (a) modal (the word is also an adjective) that means only the modal dialog's content should be accessible to users.

Visual (and mouse/touch) users

This distinction is 99% of the time made with using an overlay, darkening the interface behind the modal.

Keyboard users

Keyboard users should expect that focus is kept within the modal ('s tabable elements). Focus mustn't leave the modal when it is open, the darkened/inactive interface behind it should not be reachable via keyboard. This is the case in the current implementation, and a thing we should fix given there's an UX decision in favor of the modal dialog. This could be solved with setting the inert attribute (and its polyfill) during open modal state (inerts addition is part of my Pull Request #2101).

Screen reader users

The inert attribute does not prevent screen readers from reaching the content behind the modal, though. But this could be solved with aria-hidden="true" on everything but the open modal. A word of warning: Once you add aria-hidden="true" on an element, it and all of its children will be rendered inaccessible to screen readers. Even when a child node of said element has aria-hidden="false" this does not override its parent's true, it's still inaccessible. Think of it like #foo {display: none} in CSS (even with children of #foo set to display: block they won't be visible).

Therefore, until aria-modal could be used as an alternative (as of now it can't because of Apple, background) the correct DOM order when working with modals is important:

<div aria-hidden="true" inert>
  <main role="main">
    <p>content</p>
  </main>
</div>
<div role="dialog" aria-labelledby="myModal-title" tabindex="-1">
  <h1 id="myModal-title">Supermodal!</h1>
  ...
</div>
<!-- In short: Inert content and modals are siblings, not nested! -->

Luckily, in phoenix's current implementation is okay regarding that. But should it ever change, please keep the DOM order issue in mind.

Proper role & label

As you can see in the code example above, dialogs both need a proper role and an accessible name. Since there is currently a visible headline within the modal ("Ordner Documents umbenennen", "Datei oder Ordner löschen") this headline can be referenced with aria-labelledby (but at first it needs an ID).

CSS Tricks' latest article on <dialog>

Some days ago "Some hand-on" with the HTML dialog element" was published. When reading this, using <dialog> seems to be the drop-in solution for many things (darkened background, focus management, semantics). Unfortunately people who do thorough screen reader testing do not recommend to use it in production.

tldr; I’m just going to say right now that the dialog element and its polyfill are not suitable for use in production. And it’s been that way since the dialog’s earliest implementation in Chrome, six-ish years ago. [Scott O'Hara]

So common recommendation among accessibility pundits is to use role="dialog".

One other thing: although there is the best practice of only using one h1 in a document it is also best practice to make the modal's title an h1 (since it's creating a new context and, if implemented correctly, will still be the only active h1 (because the other one is hidden away with aria-hidden=true).

Focus management

It is best practice to manage focus in the following way

  • Given you open the modal through a button (and you should) focus should be sent from said button to the modal itself (I have to update my blog post mentioned above regarding this – not the first interactive element inside the modal but the modal itself)
  • Then, focus should be kept inside the modal, the user mustn't reach the interactive elements of the "darkened" user interface.
  • On modal closing, focus should be sent back to the triggering button.

So it is best to save a reference to the triggering button in memory. Unfortunately because of browser inconsistencies (not in every user agent and every OS a button also receives focus on click), we should not rely on document.activeElement for this (alone).

Enough theory: How to implement this in phoenix?

Like mentioned above: Given that the UX phase results in deciding to keep the modal dialog interface pattern there are these options:

  • Contribute to UIKit and its modal implementation by fixing the accessibility issues in there
  • Fork UIKit and implement the accessibility improvements in said fork
  • Not using UIKit for modals at all and instead implement accessible modals ourselves (we would have to start from scratch, there a battle tested "Vanilla JS" scripts around).

@marcus-herrmann
Copy link
Contributor

Setting this issue's state in "to review" because its dependent on the UX workshop, on whether the text above is helpful to you, and on a decision how to proceed if the modal UX pattern stays in phoenix.

@PVince81
Copy link
Contributor Author

@marcus-herrmann I agree with your suggestion regarding accessibility for dialogs.

As far as I heard, we are already not using UIKit's modals. We have an "oc-dialog" element in the design system. Can you check whether it can be extended to include the accessibility requirements from above ?

Then we need to check how we implement modal triggering in Vue and see how we can switch "aria-hidden" for the matching components in a convenient way. As far as I remember we are flipping a flag to display the rename or delete dialog. See: https://github.com/owncloud/phoenix/blob/master/apps/files/src/components/FilesAppBar.vue#L77

I suspect that the latter dialogs are not rendered aside the app container so we might to do some changes.

@marcus-herrmann
Copy link
Contributor

@PVince81 Should I stall any work on oc-dialog before your UX meetings are over – or do I overestimate the possibility of removal of the modal pattern in these worksjops (actually I guess this concern is only based on an accessory sentence of @michaelstingl when we met in person)?

@PVince81
Copy link
Contributor Author

To me the above text is mostly research results to confirm that modals are not a blocker for accessibility as there are ways to make it work. This way we can then continue the discussion about modals, so let's not invest time yet for implementing accessibility until we decide what to do with them.

@marcus-herrmann
Copy link
Contributor

Gotcha.

@PVince81 PVince81 modified the milestones: Milestone 1: Phoenix for users, backlog Nov 11, 2019
@PVince81
Copy link
Contributor Author

some part of visual improvements done in owncloud/owncloud-design-system#723

@pascalwengerter
Copy link
Contributor

Closing this since the new modal has been introduced a while ago now #3378

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants