More SelectMenu improvements

[simurai]

This adds a few more improvements to the .SelectMenu .

TODO:

• Fix top border
• Increase max height
• Introduce .SelectMenu-icon--check
• Prevent shrinking of the icon
• Deprecate the removed selectors

TODO on dotcom:

• Add SelectMenu-icon--check to all check icons.
• The hack added in https://github.com/github/github/pull/127223/files#r336699321 could probably be removed again.

---

While testing the SelectMenu for the "branch picker" more issues came up:

1. When filtering, a top border is visible

When using fuzzy-list to filter, the DOM nodes get removed, but for the branch picker display: none is used. This has the effect that :first/:last-child can't be used anymore.

Fix: Use a bottom-border and then margin-bottom: -1px on the list to hide the "last" border fb3a10a

Using box-shadow would be an alternative because it gets cut off within overflow-y: auto;. But it seems scrolling performance suffers.

Before	After

2. The branch list is not as tall

Fix: Increase max-height a bit. 279c87e

Before	After

3. Introduce .SelectMenu-icon--check

Currently we use the SelectMenu-icon class specifically for the check icon to show if an item is selected or not. But there have been cases where a SelectMenu uses different icons unrelated to the selected/checked state. This PR:

• Makes the .SelectMenu-icon more generic. It only cares about the size and margin and can be used for any icon.
• Adds an additional .SelectMenu-icon--check modifier class that toggles based on the aria-checked="true" attribute.

<button class="SelectMenu-item" role="menuitemcheckbox" aria-checked="true">
  <%= octicon "check", class: "SelectMenu-icon SelectMenu-icon--check" %>
  Selectable item with a check icon
</button>
<button class="SelectMenu-item" role="menuitem">
  <%= octicon "pin", class: "SelectMenu-icon" %>
  Item with a pin icon
</button>

Above markup renders as:

It allows to mix check and other icons in the same list and have them aligned without fiddling with margin utilities.

Alternatives

Some alternative names instead of SelectMenu-icon--check:

• .SelectMenu-icon--checked might be confused for adding a state
• .SelectMenu-icon--selected same
• .SelectMenu--showWhenChecked could be used for anything, not just icons. People might expect it to be display: none by default, instead of visibility: hidden. But that would make the text jump.

4. Prevent shrinking of the icon

Happens when the text starts to wrap, see #961. A flex-shrink-0 can be used, but might be convenient to "bake that in".

[vercel]

This pull request is being automatically deployed with ZEIT Now (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://zeit.co/primer/primer-css/1uyech6h7
🌍 Preview: https://primer-css-git-more-select-menu-improvements.primer.now.sh

[simurai]

FYI: While resolving merge conflicts, I also added the changes from #965 into this branch.

I think this should now be ready.

[shawnbot]

This looks great. Thanks for working through all these issues!