Migrate Menu to tailwind

Update a few components to account for style adjustments

src/sidebar/components/Annotation/AnnotationPublishControl.js

@@ -46,8 +46,11 @@ function AnnotationPublishControl({
   );
 
   const menuLabel = (
-    <div className="p-2.5 text-color-text-inverted" style={buttonStyle}>
-      <Icon name="expand-menu" />
+    <div
+      className="w-9 h-9 flex items-center justify-center text-color-text-inverted"
+      style={buttonStyle}
+    >
+      <Icon name="expand-menu" classes="w-4 h-4" />
     </div>
   );
 

src/sidebar/components/Menu.js

 import classnames from 'classnames';
 import {
-  SvgIcon,
+  Icon,
   normalizeKeyName,
   useElementShouldClose,
 } from '@hypothesis/frontend-shared';
 import { useCallback, useEffect, useRef, useState } from 'preact/hooks';
 
+import MenuArrow from './MenuArrow';
 import MenuKeyboardNavigation from './MenuKeyboardNavigation';
 
-/**
- * The triangular indicator below the menu toggle button that visually links it
- * to the menu content.
- *
- * @param {object} props
- *   @param {string} [props.className]
- */
-function MenuArrow({ className }) {
-  return (
-    <svg className={classnames('Menu__arrow', className)} width={15} height={8}>
-      <path d="M0 8 L7 0 L15 8" stroke="currentColor" strokeWidth="2" />
-    </svg>
-  );
-}
-
 /**
  * Flag indicating whether the next click event on the menu's toggle button
  * should be ignored, because the action it would trigger has already been
@@ -171,7 +157,8 @@ export default function Menu({
     // See https://github.com/evcohen/eslint-plugin-jsx-a11y/blob/master/docs/rules/no-static-element-interactions.md#case-the-event-handler-is-only-being-used-to-capture-bubbled-events
     // eslint-disable-next-line jsx-a11y/no-static-element-interactions, jsx-a11y/click-events-have-key-events
     <div
-      className="Menu"
+      className="relative"
+      data-testid="menu-container"
       ref={menuRef}
       // Add inline styles for positioning
       style={containerStyle}
@@ -186,7 +173,15 @@ export default function Menu({
       <button
         aria-expanded={isOpen ? 'true' : 'false'}
         aria-haspopup={true}
-        className="Menu__toggle"
+        className={classnames(
+          'hyp-u-outline-on-keyboard-focus',
+          'flex items-center justify-center rounded-sm transition-colors',
+          {
+            'text-grey-7 hover:text-grey-9': !isOpen,
+            'text-brand': isOpen,
+          }
+        )}
+        data-testid="menu-toggle-button"
         onMouseDown={toggleMenu}
         onClick={toggleMenu}
         aria-label={title}
@@ -194,27 +189,43 @@ export default function Menu({
       >
         <span
           // wrapper is needed to serve as the flex layout for the label and indicator content.
-          className="Menu__toggle-wrapper"
+          className="flex items-center gap-x-1"
         >
           {label}
           {menuIndicator && (
             <span
-              className={classnames('Menu__toggle-arrow', isOpen && 'is-open')}
+              className={classnames({
+                'rotate-180 text-color-text': isOpen,
+              })}
             >
-              <SvgIcon name="expand-menu" className="Menu__toggle-icon" />
+              <Icon name="expand-menu" classes="w-2.5 h-2.5" />
             </span>
           )}
         </span>
       </button>
       {isOpen && (
         <>
-          <MenuArrow className={arrowClass} />
+          <MenuArrow
+            direction="up"
+            classes={classnames(
+              // Position menu-arrow caret near bottom right of menu label/toggle control
+              'right-0 top-[calc(100%-6px)] w-[15px]',
+              arrowClass
+            )}
+          />
           <div
             className={classnames(
-              'Menu__content',
-              `Menu__content--align-${align}`,
+              'hyp-u-outline-on-keyboard-focus',
+              // Position menu content near bottom of menu label/toggle control
+              'absolute top-[calc(100%+5px)] z-1 border shadow',
+              'bg-white text-lg',
+              {
+                'left-0': align === 'left',
+                'right-0': align === 'right',
+              },
               contentClass
             )}
+            data-testid="menu-content"
             role="menu"
             tabIndex={-1}
             onClick={closeMenu}

src/sidebar/components/test/Menu-test.js

@@ -10,6 +10,10 @@ import { checkAccessibility } from '../../../test-util/accessibility';
 describe('Menu', () => {
   let container;
 
+  const menuSelector = '[data-testid="menu-container"]';
+  const contentSelector = '[data-testid="menu-content"]';
+  const toggleSelector = 'button[data-testid="menu-toggle-button"]';
+
   const TestLabel = () => 'Test label';
   const TestMenuItem = () => 'Test item';
 
@@ -25,7 +29,7 @@ describe('Menu', () => {
   };
 
   function isOpen(wrapper) {
-    return wrapper.exists('.Menu__content');
+    return wrapper.exists(contentSelector);
   }
 
   beforeEach(() => {
@@ -43,9 +47,9 @@ describe('Menu', () => {
   it('opens and closes when the toggle button is clicked', () => {
     const wrapper = createMenu();
     assert.isFalse(isOpen(wrapper));
-    wrapper.find('button').simulate('click');
+    wrapper.find(toggleSelector).simulate('click');
     assert.isTrue(isOpen(wrapper));
-    wrapper.find('button').simulate('click');
+    wrapper.find(toggleSelector).simulate('click');
     assert.isFalse(isOpen(wrapper));
   });
 
@@ -56,16 +60,16 @@ describe('Menu', () => {
 
     assert.isTrue(isOpen(wrapper));
 
-    wrapper.find('button').simulate('click');
+    wrapper.find(toggleSelector).simulate('click');
     assert.isTrue(isOpen(wrapper));
   });
 
   it('calls `onOpenChanged` prop when menu is opened or closed', () => {
     const onOpenChanged = sinon.stub();
     const wrapper = createMenu({ onOpenChanged });
-    wrapper.find('button').simulate('click');
+    wrapper.find(toggleSelector).simulate('click');
     assert.calledWith(onOpenChanged, true);
-    wrapper.find('button').simulate('click');
+    wrapper.find(toggleSelector).simulate('click');
     assert.calledWith(onOpenChanged, false);
   });
 
@@ -73,9 +77,9 @@ describe('Menu', () => {
     const wrapper = createMenu();
     assert.isFalse(isOpen(wrapper));
 
-    wrapper.find('button').simulate('mousedown');
+    wrapper.find(toggleSelector).simulate('mousedown');
     // Make sure the follow-up click doesn't close the menu.
-    wrapper.find('button').simulate('click');
+    wrapper.find(toggleSelector).simulate('click');
 
     assert.isTrue(isOpen(wrapper));
   });
@@ -96,12 +100,6 @@ describe('Menu', () => {
     assert.isTrue(wrapper.exists(TestMenuItem));
   });
 
-  it('flips toggle arrow when open', () => {
-    const wrapper = createMenu({ defaultOpen: true });
-    const toggle = wrapper.find('.Menu__toggle-arrow');
-    assert.isTrue(toggle.hasClass('is-open'));
-  });
-
   let e;
   [
     new Event('mousedown'),
@@ -136,13 +134,13 @@ describe('Menu', () => {
 
   it('does not close menu if user presses mouse on menu content', () => {
     const wrapper = createMenu({ defaultOpen: true });
-    let content = wrapper.find('.Menu__content');
+    let content = wrapper.find(contentSelector);
     act(() => {
       content
         .getDOMNode()
         .dispatchEvent(new Event('mousedown', { bubbles: true }));
       wrapper.update();
-      content = wrapper.find('.Menu__content');
+      content = wrapper.find(contentSelector);
     });
     assert.isTrue(isOpen(wrapper));
   });
@@ -180,7 +178,7 @@ describe('Menu', () => {
       const clock = sinon.useFakeTimers();
       try {
         const wrapper = createMenu({ defaultOpen: true });
-        wrapper.find('.Menu__content').simulate(eventType, { key });
+        wrapper.find(contentSelector).simulate(eventType, { key });
         // The close event is delayed by a minimal amount of time in
         // order to allow links to say in the DOM long enough to be
         // followed on a click. Therefore, this test must simulate
@@ -199,19 +197,20 @@ describe('Menu', () => {
 
     // The event may be received either by the top `<div>` or the arrow element
     // itself.
-    wrapper.find('.Menu').simulate('mousedown');
-    wrapper.find('.Menu__arrow').simulate('mousedown');
+    wrapper.find(menuSelector).simulate('mousedown');
+
+    assert.isTrue(isOpen(wrapper));
   });
 
   it('aligns menu content depending on `align` prop', () => {
     const wrapper = createMenu({ defaultOpen: true });
-    assert.isTrue(wrapper.exists('.Menu__content--align-left'));
+    assert.isTrue(wrapper.find(contentSelector).hasClass('left-0'));
 
     wrapper.setProps({ align: 'left' });
-    assert.isTrue(wrapper.exists('.Menu__content--align-left'));
+    assert.isTrue(wrapper.find(contentSelector).hasClass('left-0'));
 
     wrapper.setProps({ align: 'right' });
-    assert.isTrue(wrapper.exists('.Menu__content--align-right'));
+    assert.isTrue(wrapper.find(contentSelector).hasClass('right-0'));
   });
 
   it('applies custom content class', () => {
@@ -219,7 +218,7 @@ describe('Menu', () => {
       defaultOpen: true,
       contentClass: 'special-menu',
     });
-    const content = wrapper.find('.Menu__content');
+    const content = wrapper.find(contentSelector);
     assert.isTrue(content.hasClass('special-menu'));
   });
 
@@ -228,16 +227,16 @@ describe('Menu', () => {
       arrowClass: 'my-arrow-class',
       defaultOpen: true,
     });
-    const arrow = wrapper.find('.Menu__arrow');
+    const arrow = wrapper.find('MenuArrow');
 
-    assert.isTrue(arrow.hasClass('my-arrow-class'));
+    assert.include(arrow.props().classes, 'my-arrow-class');
   });
 
   it('has relative positioning if `containerPositioned` is `true`', () => {
     const wrapper = createMenu({
       containerPositioned: true, // default
     });
-    const menuContainer = wrapper.find('.Menu');
+    const menuContainer = wrapper.find(menuSelector);
 
     assert.include({ position: 'relative' }, menuContainer.prop('style'));
   });
@@ -246,7 +245,7 @@ describe('Menu', () => {
     const wrapper = createMenu({
       containerPositioned: false,
     });
-    const menuContainer = wrapper.find('.Menu');
+    const menuContainer = wrapper.find(menuSelector);
 
     assert.include({ position: 'static' }, menuContainer.prop('style'));
   });

src/styles/sidebar/components/Menu.scss

-@use '@hypothesis/frontend-shared/styles/mixins/focus';
-
-@use '../../mixins/buttons';
-@use '../../mixins/layout';
-@use '../../mixins/molecules';
-@use '../../mixins/utils';
-@use '../../variables' as var;
-
-.Menu {
-  position: relative;
-}
-
-// Toggle button that opens the menu.
-.Menu__toggle {
-  // Override $coarse-min-size so the button's size won't change in mobile view.
-  @include buttons.button--icon-only($coarse-min-size: inherit);
-  appearance: none;
-  background: none;
-  padding: 0;
-  // "block" display is needed so it can take up the
-  //  full height of its parent container
-  display: block;
-  height: 100%;
-  align-items: center;
-
-  // nb. This selector is nested to ensure it has higher specificity than the
-  // default icon size set by the `buttons.button--icon-only` mixin above.
-  .Menu__toggle-icon {
-    @include utils.icon--xsmall;
-  }
-}
-
-.Menu__toggle-wrapper {
-  @include layout.row($align: center);
-  height: 100%;
-}
-
-// Triangular indicator next to the toggle button indicating that there is
-// an associated drop-down menu.
-.Menu__toggle-arrow {
-  width: 10px;
-  height: 10px;
-  margin-left: 5px;
-
-  &.is-open {
-    // Flip the indicator when the menu is open.
-    transform: rotateX(180deg);
-    color: var.$color-text;
-  }
-}
-
-// Triangular indicator at the top of the menu that associates it with the
-// toggle button.
-.Menu__arrow {
-  @include molecules.menu-arrow;
-
-  // Position the arrow so that it appears flush with the right edge of the
-  // content when the menu is right-aligned, and the bottom of the arrow just
-  // overlaps the content's border. The effect is that the menu's border is a
-  // rounded rect with a notch at the top.
-  top: calc(100% - 2px); // nb. Adjust this if changing the <svg> size.
-  right: 0;
-}
-
-// Content area of the menu.
-.Menu__content {
-  @include utils.font--large;
-  @include utils.border;
-  @include utils.shadow;
-  @include focus.outline-on-keyboard-focus;
-  background-color: white;
-  position: absolute;
-  top: calc(100% + 5px);
-  z-index: 1;
-
-  &--align-left {
-    left: 0;
-  }
-
-  &--align-right {
-    right: 0;
-  }
-}

src/styles/sidebar/components/_index.scss

@@ -12,7 +12,6 @@
 @use './FilterSelect';
 @use './GroupList';
 @use './GroupListItem';
-@use './Menu';
 @use './StyledText';
 
 // TODO: Evaluate all classes below after components have been converted to