Merge pull request #922 from primer/more-select-menu-improvements

More SelectMenu improvements

Author: simurai

deprecations.js

@@ -5,6 +5,16 @@
  */
 const versionDeprecations = {
   '14.0.0': [
+    {
+      selectors: [
+        '.SelectMenu-item+.SelectMenu-item',
+        '.SelectMenu-divider:first-child',
+        '.SelectMenu-divider:last-child',
+        '.SelectMenu--hasFilter .SelectMenu-item:last-child',
+        '.SelectMenu-item[aria-checked="true"] .SelectMenu-icon'
+      ],
+      message: `These selectors are deprecated and not used anymore.`
+    },
     {
       selectors: [
         '.flex-item-equal',

docs/content/components/select-menu.md

@@ -85,7 +85,7 @@ In case the Select Menu should be aligned to the right, use `SelectMenu right-0`
 
 ## Selected state
 
-Add a `.SelectMenu-icon .octicon-check` icon and it will show up when `aria-checked="true"` is set.
+If the `SelectMenu` should show a check icon for selected items, use the `SelectMenu-icon SelectMenu-icon--check` classes. It will make the check icon show when `aria-checked="true"` is set.
 
 ```html live
 <details class="details-reset details-overlay" open>
@@ -108,68 +108,28 @@ Add a `.SelectMenu-icon .octicon-check` icon and it will show up when `aria-chec
       </header>
       <div class="SelectMenu-list">
         <button class="SelectMenu-item" role="menuitemcheckbox" aria-checked="true">
-          <!-- <%= octicon "check", class: "SelectMenu-icon" %> -->
-          <svg
-            width="12"
-            height="16"
-            viewBox="0 0 12 16"
-            class="octicon octicon-check SelectMenu-icon"
-            aria-hidden="true"
-          >
-            <path fill-rule="evenodd" d="M12 5l-8 8-4-4 1.5-1.5L4 10l6.5-6.5L12 5z" />
-          </svg>
+          <!-- <%= octicon "check", class: "SelectMenu-icon SelectMenu-icon--check" %> -->
+          <svg class="SelectMenu-icon SelectMenu-icon--check octicon octicon-check" width="12" height="16" viewBox="0 0 12 16" aria-hidden="true" ><path fill-rule="evenodd" d="M12 5l-8 8-4-4 1.5-1.5L4 10l6.5-6.5L12 5z" /></svg>
           Selected state
         </button>
         <button class="SelectMenu-item" role="menuitemcheckbox">
-          <!-- <%= octicon "check", class: "SelectMenu-icon" %> -->
-          <svg
-            width="12"
-            height="16"
-            viewBox="0 0 12 16"
-            class="octicon octicon-check SelectMenu-icon"
-            aria-hidden="true"
-          >
-            <path fill-rule="evenodd" d="M12 5l-8 8-4-4 1.5-1.5L4 10l6.5-6.5L12 5z" />
-          </svg>
+          <!-- <%= octicon "check", class: "SelectMenu-icon SelectMenu-icon--check" %> -->
+          <svg class="SelectMenu-icon SelectMenu-icon--check octicon octicon-check" width="12" height="16" viewBox="0 0 12 16" aria-hidden="true" ><path fill-rule="evenodd" d="M12 5l-8 8-4-4 1.5-1.5L4 10l6.5-6.5L12 5z" /></svg>
           Default state
         </button>
         <button class="SelectMenu-item" role="menuitemcheckbox" aria-checked="true">
-          <!-- <%= octicon "check", class: "SelectMenu-icon" %> -->
-          <svg
-            width="12"
-            height="16"
-            viewBox="0 0 12 16"
-            class="octicon octicon-check SelectMenu-icon"
-            aria-hidden="true"
-          >
-            <path fill-rule="evenodd" d="M12 5l-8 8-4-4 1.5-1.5L4 10l6.5-6.5L12 5z" />
-          </svg>
+          <!-- <%= octicon "check", class: "SelectMenu-icon SelectMenu-icon--check" %> -->
+          <svg class="SelectMenu-icon SelectMenu-icon--check octicon octicon-check" width="12" height="16" viewBox="0 0 12 16" aria-hidden="true" ><path fill-rule="evenodd" d="M12 5l-8 8-4-4 1.5-1.5L4 10l6.5-6.5L12 5z" /></svg>
           Selected state
         </button>
         <button class="SelectMenu-item" role="menuitemcheckbox">
-          <!-- <%= octicon "check", class: "SelectMenu-icon" %> -->
-          <svg
-            width="12"
-            height="16"
-            viewBox="0 0 12 16"
-            class="octicon octicon-check SelectMenu-icon"
-            aria-hidden="true"
-          >
-            <path fill-rule="evenodd" d="M12 5l-8 8-4-4 1.5-1.5L4 10l6.5-6.5L12 5z" />
-          </svg>
+          <!-- <%= octicon "check", class: "SelectMenu-icon SelectMenu-icon--check" %> -->
+          <svg class="SelectMenu-icon SelectMenu-icon--check octicon octicon-check" width="12" height="16" viewBox="0 0 12 16" aria-hidden="true" ><path fill-rule="evenodd" d="M12 5l-8 8-4-4 1.5-1.5L4 10l6.5-6.5L12 5z" /></svg>
           Default state
         </button>
         <button class="SelectMenu-item" role="menuitemcheckbox">
-          <!-- <%= octicon "check", class: "SelectMenu-icon" %> -->
-          <svg
-            width="12"
-            height="16"
-            viewBox="0 0 12 16"
-            class="octicon octicon-check SelectMenu-icon"
-            aria-hidden="true"
-          >
-            <path fill-rule="evenodd" d="M12 5l-8 8-4-4 1.5-1.5L4 10l6.5-6.5L12 5z" />
-          </svg>
+          <!-- <%= octicon "check", class: "SelectMenu-icon SelectMenu-icon--check" %> -->
+          <svg class="SelectMenu-icon SelectMenu-icon--check octicon octicon-check" width="12" height="16" viewBox="0 0 12 16" aria-hidden="true" ><path fill-rule="evenodd" d="M12 5l-8 8-4-4 1.5-1.5L4 10l6.5-6.5L12 5z" /></svg>
           Default state
         </button>
       </div>
@@ -183,7 +143,7 @@ Add a `.SelectMenu-icon .octicon-check` icon and it will show up when `aria-chec
 
 ## List items
 
-The list of items is arguably the most important subcomponent within the menu. Build them out of anchors, buttons, or just about any [interactive content](http://w3c.github.io/html/dom.html#interactive-content). List items are also customizable with options for avatars, additional icons, and multiple lines of text. Use utility classes like `mr-2`, `d-flex` or `float-right` in case more custom styling is needed.
+The list of items is arguably the most important subcomponent within the menu. Build them out of anchors, buttons, or just about any [interactive content](http://w3c.github.io/html/dom.html#interactive-content). List items are also customizable with options for avatars, additional icons, and multiple lines of text. Use utility classes in case more custom styling is needed.
 
 ```html live
 <details class="details-reset details-overlay" open>
@@ -208,6 +168,11 @@ The list of items is arguably the most important subcomponent within the menu. B
         <button class="SelectMenu-item" role="menuitem">
           Text only
         </button>
+        <button class="SelectMenu-item" role="menuitem">
+          <!-- <%= octicon "pin", class: "SelectMenu-icon" %> -->
+          <svg class="SelectMenu-icon octicon octicon-pin" viewBox="0 0 16 16" version="1.1" width="16" height="16" aria-hidden="true"><path fill-rule="evenodd" d="M10 1.2V2l.5 1L6 6H2.2c-.44 0-.67.53-.34.86L5 10l-4 5 5-4 3.14 3.14a.5.5 0 00.86-.34V10l3-4.5 1 .5h.8c.44 0 .67-.53.34-.86L10.86.86a.5.5 0 00-.86.34z"></path></svg>
+          With an icon
+        </button>
         <button class="SelectMenu-item" role="menuitem">
           <img
             class="avatar avatar-small mr-2"
@@ -247,7 +212,7 @@ The list of items is arguably the most important subcomponent within the menu. B
 </details>
 
 <div class="d-sm-none" style="height: 600px"><!-- min height for < sm --></div>
-<div class="d-none d-sm-block" style="height: 300px"><!-- min height for > sm --></div>
+<div class="d-none d-sm-block" style="height: 320px"><!-- min height for > sm --></div>
 ```
 
 ## Divider
@@ -350,7 +315,7 @@ Also consider adding a `.SelectMenu-footer` at the bottom. It can be used for ad
 </details>
 
 <div class="d-sm-none"         style="height: 600px"> <!-- min height for < sm --> </div>
-<div class="d-none d-sm-block" style="height: 380px"> <!-- min height for > sm --> </div>
+<div class="d-none d-sm-block" style="height: 500px"> <!-- min height for > sm --> </div>
 ```
 
 ## Tabs
@@ -430,7 +395,7 @@ A `SelectMenu-message` can be used to show different kind of messages to a user.
       <div class="SelectMenu-list">
         <button class="SelectMenu-item" role="menuitem">Item 1</button>
         <button class="SelectMenu-item" role="menuitem">Item 2</button>
-        <div class="SelectMenu-message border-bottom border-top bg-red-0 text-red p-2">Message goes here</div>
+        <div class="SelectMenu-message bg-red-0 text-red p-2">Message goes here</div>
         <button class="SelectMenu-item" role="menuitem">Item 3</button>
       </div>
     </div>

src/select-menu/select-menu.scss

@@ -3,6 +3,8 @@
 // selector-max-type is needed for body:not(.intent-mouse) to target keyboard only styles.
 // TODO: Introduce an additional .intent-keyboard class
 
+$SelectMenu-max-height: 480px !default;
+
 // Select Menu
 //
 // A more advanced menu with support for navigation, filtering, and more.
@@ -85,7 +87,7 @@
   @include breakpoint(sm) {
     width: 300px;
     height: auto;
-    max-height: 350px;
+    max-height: $SelectMenu-max-height;
     margin: $spacer-1 0 $spacer-3 0;
     font-size: $font-size-small;
     border: $border-width $border-style $border-gray-dark;
@@ -163,6 +165,8 @@
   position: relative;
   padding: 0;
   margin: 0;
+  // stylelint-disable-next-line primer/spacing
+  margin-bottom: -$border-width; // Hides the last border in the list
   flex: auto;
   overflow-x: hidden;
   overflow-y: auto;
@@ -186,11 +190,7 @@
   cursor: pointer;
   background-color: $bg-white;
   border: 0;
-
-  & + & {
-    // Add a top border only if the above element also is a list item
-    border-top: $border-width $border-style $border-gray-light;
-  }
+  border-bottom: $border-width $border-style $border-gray-light;
 
   @include breakpoint(sm) {
     padding-top: $spacer-2;
@@ -200,11 +200,16 @@
 
 // Icon
 //
-// The icon shown on the left of a list item. Typically a check icon.
+// Icon shown on the left of a list item.
 
 .SelectMenu-icon {
   width: $spacer-3; // fixed width to make sure following content aligns
   margin-right: $spacer-2;
+  flex-shrink: 0;
+}
+
+// Check icon
+.SelectMenu-icon--check {
   visibility: hidden;
   transition: transform 0.12s cubic-bezier(0.5, 0.1, 1, 0.5), visibility 0s 0.12s linear;
   transform: scale(0);
@@ -274,6 +279,10 @@
 //
 // A container used to show different kinds of content. Like a message, a blankslate or the loading animation.
 
+.SelectMenu-message {
+  border-bottom: $border-width $border-style $border-gray-light;
+}
+
 .SelectMenu-message,
 .SelectMenu-blankslate,
 .SelectMenu-loading {
@@ -293,23 +302,15 @@
   font-weight: $font-weight-bold;
   color: $text-gray-light;
   background-color: $bg-gray;
-  border-top: $border;
-  border-bottom: $border;
-
-  &:first-child {
-    border-top: 0;
-  }
-
-  &:last-child {
-    border-bottom: 0;
-  }
+  border-bottom: $border-width $border-style $border-gray-light;
 }
 
 // Footer
 //
 // A container at the bottom.
 
 .SelectMenu-footer {
+  z-index: 0; // Avoid top border from getting covered by the negative margin of the list
   padding: $spacer-2 $spacer-3;
   font-size: $font-size-small;
   color: $text-gray-light;
@@ -333,16 +334,10 @@
 
     @include breakpoint(sm) {
       height: auto;
-      max-height: 350px;
+      max-height: $SelectMenu-max-height;
       margin-top: $spacer-1;
     }
   }
-
-  // Show bottom "border" for the last item when the list doesn't take up the whole height
-  .SelectMenu-item:last-child {
-    // stylelint-disable-next-line primer/box-shadow
-    box-shadow: 0 $border-width 0 lighten($gray-200, 3%);
-  }
 }
 
 // States
@@ -368,7 +363,7 @@
   font-weight: $font-weight-semibold;
   color: $text-gray-dark;
 
-  .SelectMenu-icon {
+  .SelectMenu-icon--check {
     visibility: visible;
     transition: transform 0.12s cubic-bezier(0, 0, 0.2, 1), visibility 0s linear;
     transform: scale(1);