Merge pull request #346 from marcellamaki/update-kdropdown-menu-api

Update kdropdown menu api
learningequality · Sep 6, 2022 · e27e430 · e27e430
2 parents 77c5d4b + 3ab6d61
commit e27e430
Show file tree

Hide file tree

Showing 10 changed files with 90 additions and 97 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,13 +5,12 @@ Releases are recorded as git tags in the [Github releases](https://github.com/le
 ## Develop (version not yet known)
 - [#351] - Wrap `KCheckbox` default slot's content in <label>
 - [#355] - Add `KSelect` to KDS
-
+- [#346] - Update KDropdownMenu API: `KDropdownMenu` no longer contains a button. `KDropdownMenu` can be used in `KButton` or `KIconButton` `slots`. Both `slots` and `text` can now be used in `KButton` - `slots` do not take precedence over `text`.
 
 ## Version 1.4.x
 - [#185] - Handle arrow key navigation and improve focusOutline
 - [#338] - Allow for new 'nav' slot inline in the toolbar
 
-
 ## Version 1.3.1
 
 - [#309] - Add jest testing environment to KDS

diff --git a/docs/pages/buttons.vue b/docs/pages/buttons.vue
@@ -134,44 +134,48 @@
 
     <DocsPageSection title="Dropdowns" anchor="#dropdowns">
       <p>
-        Buttons can also have drop-down menus. Currently this is implemented using the poorly-named <code>KDropdownMenu</code> component, but this is <DocsGithubLink text="subject to change" issue="164" />.
+        Buttons can also have drop-down menus. 
       </p>
-      <!-- TODO: implement 3 examples: primary button, primary flat button, and secondary icon button that function as dropdowns. See Google doc for example -->
+      <p> 
+        The API for this has been updated, and now the <code>KDropdownMenu</code> component can be added using <code>template</code> with <code>#menu</code> to a <code>slot</code> in either a <code>KButton</code> or a <code>KIconButton</code>. 
+        Previously, the <code>KDropdownMenu</code> component was responsible for the button rendering as well as the menu, but now, it only manages the menu itself.
+      </p>
+
       <DocsShow>
-        <KDropdownMenu
-          style="margin-right: 16px;"
-          text="Primary"
-          :primary="true"
-          :options="['Option 1', 'Option 2']"
-          appearance="raised-button"
-        />
-        <KDropdownMenu
-          style="margin-right: 16px;"
-          text="Secondary"
-          :primary="false"
-          :options="['Option 1', 'Option 2']"
-          appearance="raised-button"
-        />
-        <KDropdownMenu
-          text="Icon dropdown"
-          :primary="false"
-          :options="['Option 1', 'Option 2']"
-          appearance="raised-button"
-        >
-          <template #button>
-            <KIconButton
-              tooltip="Dropdown options"
-              icon="optionsHorizontal"
-              appearance="flat-button"
-              :primary="false"
+        <KButtonGroup>
+          <KButton
+            text="Options"
+            :primary="true"
+            iconAfter="chevronDown"
+          >
+            <KDropdownMenu
+              style="margin-right: 16px;"
+              text="Primary"
+              :primary="true"
+              :options="['Option 1', 'Option 2']"
+              appearance="raised-button"
             />
-          </template>
-        </KDropdownMenu>
-      </DocsShow>
-      <p>
-        Caution: <code>KDropdownMenu</code> does not work with the <DocsLibraryLink component="KButtonGroup" /> component.
-      </p>
+          </KButton>
+          <KIconButton
+            tooltip="Dropdown options"
+            icon="optionsHorizontal"
+            appearance="flat-button"
+            :primary="false"
+          >
+            <template #menu>
+              <KDropdownMenu
+                style="margin-right: 16px;"
+                text="Primary"
+                :primary="true"
+                :options="['Option 1', 'Option 2']"
+                appearance="raised-button"
+              />
+            </template>
+          </KIconButton>
+        </KButtonGroup>
 
+      </DocsShow>
+      <p>For more guidance, see the <code>KDropdownMenu</code> component.</p>
     </DocsPageSection>
 
     <DocsPageSection title="Visual specs" anchor="#specs" />
@@ -193,7 +197,4 @@
 
   export default {};
 
-</script>
-
-
-<style lang="scss" scoped></style>
+</script>
diff --git a/docs/pages/kbutton.vue b/docs/pages/kbutton.vue
@@ -37,6 +37,7 @@
           <DocsLibraryLink component="KIconButton" /> has a tooltip instead of a text label
         </li>
         <li><DocsLibraryLink component="KButtonGroup" /> is used for button layout</li>
+        <li><DocsLibraryLink component="KDropdownMenu" /> and <DocsInternalLink text="Buttons and links" href="/buttons" /> have more detail about adding a menu of options to a button component</li>
       </ul>
     </DocsPageSection>
 

diff --git a/docs/pages/kdropdownmenu.vue b/docs/pages/kdropdownmenu.vue
@@ -4,10 +4,14 @@
     <DocsPageSection title="Overview" anchor="#overview">
 
       <p>
-        Implements a button with a dropdown set of options, based on <DocsExternalLink text="Keen's UI Menu" href="https://josephuspaye.github.io/Keen-UI/#/ui-menu" />. See these docs to understand the current implementation of the options object array.
+        Implements a dropdown set of options, based on <DocsExternalLink text="Keen's UI Menu" href="https://josephuspaye.github.io/Keen-UI/#/ui-menu" />. See these docs to understand the current implementation of the options object array.
       </p>
       <p>
-        Note that this component may be deprecated in the future: <DocsGithubLink issue="164" />
+        Notable possible for configuring the menu include: icons, text, secondary text, and dividers.
+      </p>
+      <p>
+        Prior to 09/2022, this component also managed the button associated with the dropdown menu, but the API has changed for a clearer division of responsibilities. 
+        Please see the <DocsInternalLink href="/buttons#dropdowns" text="Dropdown section of the Buttons and links page" /> on the buttons page for more details about how to use with a button, and a code example.
       </p>
     </DocsPageSection>
   </DocsPageTemplate>

diff --git a/docs/pages/kiconbutton.vue b/docs/pages/kiconbutton.vue
@@ -25,6 +25,7 @@
         </li>
         <li><DocsLibraryLink component="KButton" />, <DocsLibraryLink component="KExternalLink" />, and <DocsLibraryLink component="KRouterLink" /> are all similar but handle events differently</li>
         <li><DocsLibraryLink component="KButtonGroup" /> is used for button layout</li>
+        <li><DocsLibraryLink component="KDropdownMenu" /> and <DocsInternalLink text="Buttons and links" href="/buttons" /> have more detail about adding a menu of options to a button component</li>
       </ul>
     </DocsPageSection>
 

diff --git a/lib/KDropdownMenu.vue b/lib/KDropdownMenu.vue
@@ -1,29 +1,21 @@
 <template>
 
-  <KButton
-    ref="button"
-    :appearance="appearance"
-    :disabled="disabled"
-    :hasDropdown="true"
-    :primary="$attrs.primary"
+  <UiPopover
+    v-if="!disabled"
+    ref="popover"
+    :z-index="12"
+    :containFocus="true"
+    :dropdownPosition="position"
+    @close="handleClose"
+    @open="handleOpen"
   >
-    <span>{{ text }}</span>
-    <UiPopover
-      v-if="!disabled"
-      ref="popover"
-      :z-index="12"
-      :containFocus="true"
-      :dropdownPosition="position"
-      @close="handleClose"
-      @open="handleOpen"
-    >
-      <UiMenu
-        ref="menu"
-        :options="options"
-        @select="handleSelection"
-      />
-    </UiPopover>
-  </KButton>
+    <UiMenu 
+      ref="menu" 
+      :options="options" 
+      :hasIcons="hasIcons" 
+      @select="handleSelection" 
+    />
+  </UiPopover>
 
 </template>
 
@@ -32,7 +24,6 @@
 
   import UiPopover from './keen/UiPopover';
   import UiMenu from './keen/UiMenu';
-  import { validator } from './buttons-and-links/appearances';
 
   /**
    * The KDropdownMenu component is used to contain multiple actions
@@ -44,13 +35,6 @@
       UiMenu,
     },
     props: {
-      /**
-       * Button label text
-       */
-      text: {
-        type: String,
-        required: true,
-      },
       /**
        * An array of options objects, with one object per dropdown item
        */
@@ -59,17 +43,9 @@
         required: true,
       },
       /**
-       * Button appearance: `'raised-button'` or `'flat-button'`
-       */
-      appearance: {
-        type: String,
-        default: 'raised-button',
-        validator,
-      },
-      /**
-       * Whether or not the button is disabled
+       * Whether or not the options display an icon
        */
-      disabled: {
+      hasIcons: {
         type: Boolean,
         default: false,
       },

diff --git a/lib/buttons-and-links/KButton.vue b/lib/buttons-and-links/KButton.vue
@@ -22,11 +22,12 @@
       data-test="iconBefore"
       class="prop-icon"
     />
-
-    <!-- @slot Pass sub-components into the button, which provides more flexibility than and takes precedence over the `text` prop -->
+    <!-- @slot Pass sub-components into the button, typically `KDropdownMenu` -->
+    <slot name="menu"></slot>
+    <!-- @slot Slot alternative to the `text` prop -->
     <slot v-if="$slots.default"></slot>
 
-    <template v-else>
+    <template>
       <span class="link-text" :style="textStyle">{{ text }}</span>
     </template>
 

diff --git a/lib/buttons-and-links/KIconButton.vue b/lib/buttons-and-links/KIconButton.vue
@@ -10,16 +10,19 @@
     text=""
     v-on="$listeners"
   >
+    <!-- if no "position" is passed as a prop, defaults to bottom, as previously -->
     <UiTooltip
       v-if="tooltip"
       :zIndex="24"
       open-on="hover"
-      position="bottom"
+      :position="tooltipPosition" 
     >
       {{ tooltip }}
     </UiTooltip>
     <!-- UiIconButton used flexbox - 7px is the magic centering number -->
     <KIcon :icon="icon" :color="color" :style="iconStyles" />
+    <!-- @slot Pass sub-components into the button, typically `KDropdownMenu` -->
+    <slot name="menu"></slot>
   </KButton>
 
 </template>
@@ -89,6 +92,13 @@
         type: String,
         default: null, // https://github.com/learningequality/kolibri-design-system/issues/168
       },
+      /**
+       * Tooltip position: `'top', 'right', 'bottom', 'left'`
+       */
+      tooltipPosition: {
+        type: String,
+        default: 'bottom',
+      },
     },
     computed: {
       appearanceOverrides() {

diff --git a/lib/buttons-and-links/__tests__/KButton.spec.js b/lib/buttons-and-links/__tests__/KButton.spec.js
@@ -39,7 +39,7 @@ describe('KButton', () => {
       expect(wrapper.text()).toContain('test');
     });
 
-    it('should render the slot and not the text prop when the slot has content', () => {
+    it('should render the slot when the slot has content', () => {
       const wrapper = shallowMount(KButton, {
         propsData: {
           text: 'test',
@@ -49,7 +49,7 @@ describe('KButton', () => {
         },
       });
       expect(wrapper.text()).toContain('slot');
-      expect(wrapper.text()).not.toContain('test');
+      expect(wrapper.text()).toContain('test');
     });
   });
 

diff --git a/lib/keen/UiMenuOption.vue b/lib/keen/UiMenuOption.vue
@@ -15,17 +15,12 @@
   >
     <slot v-if="!isDivider">
       <div class="ui-menu-option-content">
-        <UiIcon
+        <KIcon
+        class="k-icon"
           v-if="icon"
-
-          class="ui-menu-option-icon"
-          :icon-set="iconProps.iconSet"
           :icon="icon"
-          :remove-text="iconProps.removeText"
-
-          :use-svg="iconProps.useSvg"
         />
-
+ 
         <div class="ui-menu-option-text">
           {{ label }}
         </div>
@@ -180,4 +175,9 @@
     color: $hint-text-color;
   }
 
+  .k-icon {
+    top: 0px !important;
+    margin: 0px 8px;
+  }
+
 </style>