Select Menu component

[simurai]

This is an alternative to #801. It introduces a responsive .SelectMenu component to Primer.

Markup

Below a simple example, more options in the documentation.

<details class="details-reset details-overlay" open>
  <summary class="btn" type="button">Choose an item</summary>
  
  <!-- SelectMenu component starts here -->
  <div class="SelectMenu">
    <div class="SelectMenu-modal">
      <header class="SelectMenu-header">
        <h3 class="SelectMenu-title">Title</h3>
        <button class="SelectMenu-closeButton" type="button">X</button>
      </header>
      <menu class="SelectMenu-list">
        <button class="SelectMenu-item">Item 1</button>
        <button class="SelectMenu-item">Item 2</button>
        <button class="SelectMenu-item">Item 3</button>
      </menu>
    </div>
  </div>

</details>

Modifiers

By default, when the viewport is smaller than sm, the Select Menu has an auto-height and is vertically centered. If the Select Menu has a filter, the .SelectMenu--hasFilter modifier can be added to make the Select Menu stick to the top and have a fixed height. This allows the list to be filtered without the input jumping around.

Modifier	📱 xs	🖥 > sm
default
.SelectMenu--hasFilter

Must have

• Right aligned
• Loading view
• Blankslate
• Tabs for multiple lists
• Animation
• Hover/focus styles
• Fix a few issues on mobile.
• Add aria/role attributes

Nice to have

• Fix border-radius/overflow: auto issue in Safari iOS. -> It works by adding overflow: hidden on a parent (.SelectMenu-modal) element.
• It pretty hard to reliably add bottom border radius. The :last-child pseudo selector can't really be used because sometimes the last element gets hidden but remains in the DOM. And the .last-visible class is deprecated. Or an <div class="sr-only" data-filterable-notice="" aria-live="polite">10 results found.</div> element gets added as last element. -> ☝️ Above fix also fixed this. Now all child elements should get "masked" with rounded corners. Downside: Overflowing content is no possible anymore. Let's see if we need it.
• Add the backdrop animation back. It got removed because of a render glitch in Safari iOS.
• Currently an id is needed to make the data-toggle-for work for the close button. Should be fine in some cases, but could also become more complex when a Select Menu gets used multiple times on the same page and a unique ID needs to be passed from a parent of the partial. Would be great if <details-menu> would have an equivalent to data-close-dialog that doesn't require a unique ID.
• Since both, the new .SelectMenu AND the current .select-menu will be around for a while forever 🙃, both select menus should look and behave similar. They are currently pretty close, but when taking a closer look, there are still some details that are different.

.com tests

• User profiles. This was pretty straightforward and seems to work ok.
• Blob editor. Needs JS changes or adding old selectors.
• Dashboard. Still some "nice to have", but shouldn't be a blocker.
• Issues/PR -> wontfix (for now). These are a lot more complex and probably need custom CSS, changes to JS and ERB partials.

---

Ref. https://github.com/github/design-systems/issues/601#issuecomment-489551559

[vercel]

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

[shawnbot]

The CSS doesn't seem to support native checkbox inputs out of the box. Could we support something like:

<label class="SelectMenu-item">
  <input type="checkbox">
  <%= octicon "check", class: "SelectMenu-icon" %>
  Check this out
</label>

and then style it with:

.SelectMenu-item {
  input[type=checkbox] { display: none; }

  .SelectMenu-icon ~ input[type=checkbox] { visibility: hidden; }
  .SelectMenu-icon ~ input[type=checkbox]:checked { visibility: visible; }
}

Or is that too clever? 🤔

[simurai]

The CSS doesn't seem to support native checkbox inputs out of the box.
Or is that too clever?

Yeah, that would be clever. 🤓 On .com we sometimes do use the details-menu element with checkbox/radio inputs. In a test for the profile repo SelectMenu the input gets hidden with d-none and toggling of the "selected" state is done by using the aria-checked="true" selector:

<div class="SelectMenu-list">
  <label class="SelectMenu-item" role="menuitemradio" aria-checked="<%= type_filter.to_s == value %>" tabindex="0">
    <%= radio_button_tag "type", value, type_filter.to_s == value, class: "d-none", "data-autosubmit": "true" %>
    <%= octicon("check", class: "SelectMenu-icon") %>
    <span class="text-normal" data-menu-button-text><%= name %></span>
  </label>
</div>

Is that ok too? It would leave us the option to also style the SelectMenu-item with a selected state and not just anything that comes after the <input type="checkbox">.

[muan]

the input gets hidden with d-none and toggling of the "selected" state is done by using the aria-checked="true" selector:

FWIW in <details-menu>, aria-checked attribute value is toggled on its containing label > input's change event. So in those cases @shawnbot's suggestion would work too.

However, I think it isn't necessary to support the input:checked ~ * selector because the wrapping element (.SelectMenu-item) will always need to know about the checked state too, which the descendent can't inform.

But I do think it'd be good to support a CSS class checked state alternative. It'd be useful when aria-checked does not make sense, since it only works on elements with specific role values.

.SelectMenu-item[aria-checked="true"],
.SelectMenu-item.SelectMenu-item--checked {
  ...
}

[shawnbot]

But I do think it'd be good to support a CSS class checked state alternative. It'd be useful when aria-checked does not make sense, since it only works on elements with specific role values.

That makes sense to me, but I wonder if we can do it with other ARIA attributes, e.g. aria-selected and/or aria-current? Otherwise we're adding a presentational class that we'd probably want to lint for to ensure that people are using the component properly.

In other words: Would we only use SelectMenu-item--checked for documentation? If so, I think we should stick with ARIA attributes if possible and avoid introducing the equivalent of the (purely presentational) selected class, which has been copied and pasted from tons of other component examples all over the web. ☹️

[muan]

@shawnbot That makes sense to me. We can revisit if/when a pressing use case comes up.

[simurai]

We can revisit if/when a pressing use case comes up.

Sounds good. An additional aria-selected="true" maybe makes sense in some cases? Although currently the .SelectMenu is kinda dependant on <details-menu> to become functional, so aria-checked="true" might be enough for now.

[muan]

Yeah I think that is also a case to revisit if/when we need them.

[shawnbot]

Okay, let's ship this! I have some thoughts on layout spacing, but I think those would be best addressed in follow-ups. Thanks for all your hard work on this, @simurai !