hashicorp / nomad

Nomad is an easy-to-use, flexible, and performant workload orchestrator that can deploy a mix of microservice, batch, containerized, and non-containerized applications. Nomad is easy to operate and scale and has native Consul and Vault integrations.
https://www.nomadproject.io/
Other
14.8k stars 1.94k forks source link

UI: Document the PopoverMenu component in Storybook #7101

Open DingoEatingFuzz opened 4 years ago

DingoEatingFuzz commented 4 years ago

Node drain introduced a new PopoverMenu component that hasn't found its way into our living styleguide yet.

Styleguide: https://nomad-storybook.netlify.com/ Files:

  1. Template
  2. Logic
  3. Styles

States to Capture:

Screenshots: image image

CITguy commented 4 years ago

Please excuse my ignorance of pre-existing component specifications/behavior.

Questions

1. Where can I view redline specifications for each state?

NOTE: many of the following sections might be answered, if provided with full redline specifications.

2. What is the intended behavior if the user clicks outside of an open popover?

3. What is the intended behavior if an open popover is scrolled out of view?

4. What are the accessibility requirements of this component?

5. Disabled w/ Tooltip

(assuming that <dd.Trigger> renders a <button>)

[WAI ARIA best practices]() mention the following in regards to Tooltip widgets:

A tooltip is a popup that displays information related to an element when the element receives keyboard focus or the mouse hovers over it. It typically appears after a small delay and disappears when Escape is pressed or on mouse out.

Tooltip widgets do not receive focus. A hover that contains focusable elements can be made using a non-modal dialog.

An HTML element needs to be focusable, when navigating via keyboard (to trigger visibility of the tooltip as well as to announce tooltip text via screen reader). A disabled button (<button disabled>) is impossible to focus via keyboard navigation (non-configurable, browser-default behavior), so there's no way to trigger visibility of a tooltip with keyboard navigation.

What should be done in this scenario?

Other Issues/Comments

Anchor vs Link

Regarding https://github.com/hashicorp/nomad/blob/master/ui/app/components/popover-menu.js#L7

Links (<a href="...">) and Anchors (<a>) may share the same HTML element, but they have very different behavior.

An Anchor (<a>) is not natively focusable. The selector a:not([disabled]) would still match an anchor, yet it would not be focusable, resulting in a false positive for firstFocusableElement. A better CSS selector to match focusable links would be either a[href] OR :link.

A Link (<a href="...">) cannot be disabled semantically via a disabled boolean attribute (a[disabled]). Browsers still allow "click" events on a[href][disabled], even if it's styled to appear disabled. You could disable mouse interaction via pointer-events: none; in CSS. However, in order to disable keyboard interaction, you'd also have to add "click" event listeners to each <a href="..." disabled> node and prevent the event behavior. A better solution to "disable" links is to remove the href attribute (turning the Link into a non-focusable Anchor).

[disabled] vs :disabled

The use of the boolean attribute selector [disabled] is a symptom of using elements in a non-semantic way. Recommend using :disabled instead of [disabled] to promote correct HTML semantics.

DingoEatingFuzz commented 4 years ago

Hey @CITguy,

I could answer all these (or at least most of these) here, but I encourage you to do some tire kicking of the component instead.

There is no formal documentation for these components, since that's what we use Storybook for. However, you can see the component in action here: https://nomad-ui.netlify.com/ui/clients

The drain popover on the Client Detail page will show you the disabled state, and you can set an ACL token to enable it. The mock ACL tokens are console logged. You'll want to use the first Secret Token since that's the management token that gets all permissions. image

Aside from the questions, but I want to make sure it's clear what's expected of you for this issue. We're just looking for a story that documents the component in its current state. I would be surprised to see a PR that makes any changes to the three files I linked above.

This isn't to say auditing the component for accessibility or missing states isn't valuable! I just don't want to see this work balloon. If you come across ways to improve the component, please open issues in this repo to state those ideas.