Dynamically change number of formatter buttons in md toolbar overflow menu (#18224)

* dynamically change number of formatter buttons in the overflow menu

* add specs

app/assets/stylesheets/views/article-form.scss

@@ -158,15 +158,19 @@
     top: 0;
     background: var(--base-0);
     padding: var(--su-2) var(--content-padding-x);
-    padding-right: var(--toolbar-padding-right, 0);
-    overflow-x: auto;
+    padding-right: var(--content-padding-x, 0);
     flex-shrink: 0;
     margin: calc(var(--content-padding-y) * -1)
       calc(var(--content-padding-x) * -1) var(--su-6)
       calc(var(--content-padding-x) * -1);
 
     .editor-toolbar {
-      position: relative;
+      overflow-x: auto;
+
+      @media (min-width: $breakpoint-xs) {
+        overflow-x: unset;
+        position: relative;
+      }
     }
 
     > :first-child {
@@ -174,16 +178,6 @@
         margin-left: calc(var(--su-2) * -1);
       }
     }
-
-    &::-webkit-scrollbar {
-      background: transparent;
-      height: 0;
-    }
-
-    @media (min-width: $breakpoint-m) {
-      --toolbar-padding-right: var(--content-padding-x);
-      overflow: visible;
-    }
   }
 
   &__cover {

app/assets/stylesheets/views/comments.scss

@@ -28,14 +28,9 @@
     .editor-toolbar {
       border-top: 1px solid var(--form-border);
       overflow-x: auto;
-
-      &::-webkit-scrollbar {
-        background: transparent;
-        height: 0;
-      }
     }
 
-    @media (min-width: $breakpoint-m) {
+    @media (min-width: $breakpoint-xs) {
       .editor-toolbar {
         overflow-x: unset;
       }

app/javascript/crayons/MarkdownToolbar/MarkdownToolbar.jsx

@@ -2,8 +2,7 @@ import { h, cloneElement } from 'preact';
 import { useState, useLayoutEffect, useRef } from 'preact/hooks';
 import { ImageUploader } from '../../article-form/components/ImageUploader';
 import {
-  coreSyntaxFormatters,
-  secondarySyntaxFormatters,
+  markdownSyntaxFormatters,
   getNewTextAreaValueWithEdits,
 } from './markdownSyntaxFormatters';
 import OverflowIcon from '@images/overflow-vertical.svg';
@@ -15,6 +14,25 @@ import { getSelectionData } from '@utilities/textAreaUtils';
 // Placeholder text displayed while an image is uploading
 const UPLOADING_IMAGE_PLACEHOLDER = '![Uploading image](...)';
 
+const MAX_CORE_FORMATTERS_BY_SCREEN_SIZE = {
+  small: 5,
+  large: 7,
+  extraLarge: 10,
+};
+
+const getNumberOfIconsToDisplayInToolbar = ({
+  isSmallScreen,
+  isLargeScreen,
+}) => {
+  if (isSmallScreen) {
+    return MAX_CORE_FORMATTERS_BY_SCREEN_SIZE.small;
+  }
+  if (isLargeScreen) {
+    return MAX_CORE_FORMATTERS_BY_SCREEN_SIZE.large;
+  }
+  return MAX_CORE_FORMATTERS_BY_SCREEN_SIZE.extraLarge;
+};
+
 /**
  * Returns the next sibling in the DOM which matches the given CSS selector.
  * This makes sure that only toolbar buttons are cycled through on Arrow key press,
@@ -66,6 +84,13 @@ export const MarkdownToolbar = ({
   const [overflowMenuOpen, setOverflowMenuOpen] = useState(false);
   const [storedCursorPosition, setStoredCursorPosition] = useState({});
   const smallScreen = useMediaQuery(`(max-width: ${BREAKPOINTS.Medium - 1}px)`);
+  const largeScreen = useMediaQuery(
+    `(min-width: ${BREAKPOINTS.Large}px) and (max-width: ${
+      BREAKPOINTS.ExtraLarge - 1
+    }px)`,
+  );
+
+  const overflowMenuRows = smallScreen ? 2 : 1;
 
   // Enhance any additional toolbar elements with the appropriate roles & listeners
   const additionalSecondaryItems = additionalSecondaryToolbarElements.map(
@@ -78,11 +103,6 @@ export const MarkdownToolbar = ({
       }),
   );
 
-  const markdownSyntaxFormatters = {
-    ...coreSyntaxFormatters,
-    ...secondarySyntaxFormatters,
-  };
-
   const keyboardShortcuts = Object.fromEntries(
     Object.keys(markdownSyntaxFormatters)
       .filter(
@@ -116,7 +136,7 @@ export const MarkdownToolbar = ({
     if (!focusableToolbarButton) {
       document.querySelector('.toolbar-btn').setAttribute('tabindex', '0');
     }
-  }, [smallScreen]);
+  }, [smallScreen, largeScreen]);
 
   useLayoutEffect(() => {
     const clickOutsideHandler = ({ target }) => {
@@ -298,29 +318,32 @@ export const MarkdownToolbar = ({
     );
   };
 
-  const getSecondaryFormatterButtons = (isOverflow) =>
-    Object.keys(secondarySyntaxFormatters).map((controlName, index) => {
+  const numberOfCoreFormatters = getNumberOfIconsToDisplayInToolbar({
+    isSmallScreen: smallScreen,
+    isLargeScreen: largeScreen,
+  });
+
+  const coreSyntaxFormatters = Object.fromEntries(
+    Object.entries(markdownSyntaxFormatters).slice(0, numberOfCoreFormatters),
+  );
+  const secondarySyntaxFormatters = Object.fromEntries(
+    Object.entries(markdownSyntaxFormatters).slice(numberOfCoreFormatters),
+  );
+
+  const secondaryFormatterButtons = Object.keys(secondarySyntaxFormatters).map(
+    (controlName, index) => {
       const { icon, label, getKeyboardShortcut } =
         secondarySyntaxFormatters[controlName];
 
       return (
         <Button
           key={`${controlName}-btn`}
-          role={isOverflow ? 'menuitem' : 'button'}
+          role="menuitem"
           icon={icon}
-          className={
-            isOverflow
-              ? 'overflow-menu-btn hidden m:block mr-1'
-              : 'toolbar-btn m:hidden mr-1'
-          }
-          tabindex={isOverflow && index === 0 ? '0' : '-1'}
+          className="overflow-menu-btn mr-1"
+          tabindex={index === 0 ? '0' : '-1'}
           onClick={() => insertSyntax(controlName)}
-          onKeyUp={(e) =>
-            handleToolbarButtonKeyPress(
-              e,
-              isOverflow ? 'overflow-menu-btn' : 'toolbar-btn',
-            )
-          }
+          onKeyUp={(e) => handleToolbarButtonKeyPress(e, 'overflow-menu-btn')}
           aria-label={label}
           tooltip={
             smallScreen ? null : (
@@ -336,7 +359,8 @@ export const MarkdownToolbar = ({
           }
         />
       );
-    });
+    },
+  );
 
   return (
     <div
@@ -395,31 +419,25 @@ export const MarkdownToolbar = ({
         }}
       />
 
-      {smallScreen ? getSecondaryFormatterButtons(false) : null}
-
-      {smallScreen ? (
-        additionalSecondaryItems
-      ) : (
-        <Button
-          id="overflow-menu-button"
-          onClick={() => setOverflowMenuOpen(!overflowMenuOpen)}
-          onKeyUp={(e) => handleToolbarButtonKeyPress(e, 'toolbar-btn')}
-          aria-expanded={overflowMenuOpen ? 'true' : 'false'}
-          aria-haspopup="true"
-          icon={OverflowIcon}
-          className="toolbar-btn ml-auto hidden m:block"
-          tabindex="-1"
-          aria-label="More options"
-        />
-      )}
+      <Button
+        id="overflow-menu-button"
+        onClick={() => setOverflowMenuOpen(!overflowMenuOpen)}
+        onKeyUp={(e) => handleToolbarButtonKeyPress(e, 'toolbar-btn')}
+        aria-expanded={overflowMenuOpen ? 'true' : 'false'}
+        aria-haspopup="true"
+        icon={OverflowIcon}
+        className="toolbar-btn ml-auto"
+        tabindex="-1"
+        aria-label="More options"
+      />
 
       {overflowMenuOpen && (
         <div
           id="overflow-menu"
           role="menu"
-          className="crayons-dropdown flex p-2 min-w-unset right-0 top-100"
+          className={`crayons-dropdown grid grid-rows-${overflowMenuRows} grid-flow-col p-2 min-w-unset right-0 top-100`}
         >
-          {getSecondaryFormatterButtons(true)}
+          {secondaryFormatterButtons}
           {additionalSecondaryItems}
         </div>
       )}

app/javascript/crayons/MarkdownToolbar/__tests__/MarkdownToolbar.test.jsx

@@ -3,6 +3,7 @@ import { render, waitFor } from '@testing-library/preact';
 import { axe } from 'jest-axe';
 import '@testing-library/jest-dom';
 import { MarkdownToolbar } from '../MarkdownToolbar';
+import { BREAKPOINTS } from '@components/useMediaQuery';
 
 describe('<MarkdownToolbar />', () => {
   beforeEach(() => {
@@ -30,30 +31,149 @@ describe('<MarkdownToolbar />', () => {
     expect(results).toHaveNoViolations();
   });
 
-  it('should render core syntax formatters in main toolbar', () => {
-    const { getByLabelText } = render(<MarkdownToolbar />);
+  describe('small screen layout', () => {
+    const smallScreenMediaQuery = `(max-width: ${BREAKPOINTS.Medium - 1}px)`;
+    beforeEach(() => {
+      global.window.matchMedia = jest.fn((query) => {
+        return {
+          matches: query === smallScreenMediaQuery,
+          media: query,
+          addListener: jest.fn(),
+          removeListener: jest.fn(),
+        };
+      });
+    });
 
-    expect(getByLabelText('Bold')).toBeInTheDocument();
-    expect(getByLabelText('Italic')).toBeInTheDocument();
-    expect(getByLabelText('Ordered list')).toBeInTheDocument();
-    expect(getByLabelText('Unordered list')).toBeInTheDocument();
-    expect(getByLabelText('Heading')).toBeInTheDocument();
-    expect(getByLabelText('Quote')).toBeInTheDocument();
-    expect(getByLabelText('Code')).toBeInTheDocument();
-    expect(getByLabelText('Code block')).toBeInTheDocument();
-    expect(getByLabelText('Embed')).toBeInTheDocument();
+    it('should render only 5 formatters in main toolbar', () => {
+      const { getByLabelText, queryByLabelText } = render(<MarkdownToolbar />);
+
+      expect(getByLabelText('Bold')).toBeInTheDocument();
+      expect(getByLabelText('Italic')).toBeInTheDocument();
+      expect(getByLabelText('Ordered list')).toBeInTheDocument();
+      expect(getByLabelText('Unordered list')).toBeInTheDocument();
+      expect(getByLabelText('Link')).toBeInTheDocument();
+
+      expect(queryByLabelText('Heading')).not.toBeInTheDocument();
+      expect(queryByLabelText('Quote')).not.toBeInTheDocument();
+      expect(queryByLabelText('Code')).not.toBeInTheDocument();
+      expect(queryByLabelText('Code block')).not.toBeInTheDocument();
+      expect(queryByLabelText('Embed')).not.toBeInTheDocument();
+      expect(queryByLabelText('Underline')).not.toBeInTheDocument();
+      expect(queryByLabelText('Strikethrough')).not.toBeInTheDocument();
+      expect(queryByLabelText('Line divider')).not.toBeInTheDocument();
+    });
+
+    it('should render remaining formatters in overflow menu', async () => {
+      const { getByLabelText } = render(<MarkdownToolbar />);
+
+      getByLabelText('More options').click();
+
+      await waitFor(() =>
+        expect(getByLabelText('Heading')).toBeInTheDocument(),
+      );
+
+      expect(getByLabelText('Quote')).toBeInTheDocument();
+      expect(getByLabelText('Code')).toBeInTheDocument();
+      expect(getByLabelText('Code block')).toBeInTheDocument();
+      expect(getByLabelText('Embed')).toBeInTheDocument();
+      expect(getByLabelText('Underline')).toBeInTheDocument();
+      expect(getByLabelText('Strikethrough')).toBeInTheDocument();
+      expect(getByLabelText('Line divider')).toBeInTheDocument();
+    });
   });
 
-  it('should render an overflow menu with secondary formatters', async () => {
-    const { getByLabelText } = render(<MarkdownToolbar />);
+  describe('large screen layout', () => {
+    const largeScreenMediaQuery = `(min-width: ${
+      BREAKPOINTS.Large
+    }px) and (max-width: ${BREAKPOINTS.ExtraLarge - 1}px)`;
+    beforeEach(() => {
+      global.window.matchMedia = jest.fn((query) => {
+        return {
+          matches: query === largeScreenMediaQuery,
+          media: query,
+          addListener: jest.fn(),
+          removeListener: jest.fn(),
+        };
+      });
+    });
 
-    getByLabelText('More options').click();
+    it('should render only 7 formatters in the main toolbar', () => {
+      const { getByLabelText, queryByLabelText } = render(<MarkdownToolbar />);
 
-    await waitFor(() =>
-      expect(getByLabelText('Underline')).toBeInTheDocument(),
-    );
-    expect(getByLabelText('Strikethrough')).toBeInTheDocument();
-    expect(getByLabelText('Line divider')).toBeInTheDocument();
+      expect(getByLabelText('Bold')).toBeInTheDocument();
+      expect(getByLabelText('Italic')).toBeInTheDocument();
+      expect(getByLabelText('Ordered list')).toBeInTheDocument();
+      expect(getByLabelText('Unordered list')).toBeInTheDocument();
+      expect(getByLabelText('Link')).toBeInTheDocument();
+      expect(getByLabelText('Heading')).toBeInTheDocument();
+      expect(getByLabelText('Quote')).toBeInTheDocument();
+
+      expect(queryByLabelText('Code')).not.toBeInTheDocument();
+      expect(queryByLabelText('Code block')).not.toBeInTheDocument();
+      expect(queryByLabelText('Embed')).not.toBeInTheDocument();
+      expect(queryByLabelText('Underline')).not.toBeInTheDocument();
+      expect(queryByLabelText('Strikethrough')).not.toBeInTheDocument();
+      expect(queryByLabelText('Line divider')).not.toBeInTheDocument();
+    });
+
+    it('should render remaining formatters in overflow menu', async () => {
+      const { getByLabelText } = render(<MarkdownToolbar />);
+
+      getByLabelText('More options').click();
+
+      await waitFor(() => expect(getByLabelText('Code')).toBeInTheDocument());
+
+      expect(getByLabelText('Code block')).toBeInTheDocument();
+      expect(getByLabelText('Embed')).toBeInTheDocument();
+      expect(getByLabelText('Underline')).toBeInTheDocument();
+      expect(getByLabelText('Strikethrough')).toBeInTheDocument();
+      expect(getByLabelText('Line divider')).toBeInTheDocument();
+    });
+  });
+
+  describe('extra large screen layout', () => {
+    beforeEach(() => {
+      global.window.matchMedia = jest.fn((query) => {
+        return {
+          matches: false,
+          media: query,
+          addListener: jest.fn(),
+          removeListener: jest.fn(),
+        };
+      });
+    });
+
+    it('should render 10 formatters in the main toolbar', () => {
+      const { getByLabelText, queryByLabelText } = render(<MarkdownToolbar />);
+
+      expect(getByLabelText('Bold')).toBeInTheDocument();
+      expect(getByLabelText('Italic')).toBeInTheDocument();
+      expect(getByLabelText('Ordered list')).toBeInTheDocument();
+      expect(getByLabelText('Unordered list')).toBeInTheDocument();
+      expect(getByLabelText('Link')).toBeInTheDocument();
+      expect(getByLabelText('Heading')).toBeInTheDocument();
+      expect(getByLabelText('Quote')).toBeInTheDocument();
+      expect(getByLabelText('Code')).toBeInTheDocument();
+      expect(getByLabelText('Code block')).toBeInTheDocument();
+      expect(getByLabelText('Embed')).toBeInTheDocument();
+
+      expect(queryByLabelText('Underline')).not.toBeInTheDocument();
+      expect(queryByLabelText('Strikethrough')).not.toBeInTheDocument();
+      expect(queryByLabelText('Line divider')).not.toBeInTheDocument();
+    });
+
+    it('should render remaining formatters in overflow menu', async () => {
+      const { getByLabelText } = render(<MarkdownToolbar />);
+
+      getByLabelText('More options').click();
+
+      await waitFor(() =>
+        expect(getByLabelText('Underline')).toBeInTheDocument(),
+      );
+
+      expect(getByLabelText('Strikethrough')).toBeInTheDocument();
+      expect(getByLabelText('Line divider')).toBeInTheDocument();
+    });
   });
 
   it('should render any custom secondary toolbar elements', async () => {

app/javascript/crayons/MarkdownToolbar/markdownSyntaxFormatters.jsx

@@ -404,7 +404,7 @@ export const getNewTextAreaValueWithEdits = ({
     editSelectionStart,
   )}${replaceSelectionWith}${textAreaValue.substring(editSelectionEnd)}`;
 
-export const coreSyntaxFormatters = {
+export const markdownSyntaxFormatters = {
   bold: {
     icon: () => <Icon src={BoldIcon} />,
     label: 'Bold',
@@ -734,9 +734,6 @@ export const coreSyntaxFormatters = {
         suffix: ' %}',
       }),
   },
-};
-
-export const secondarySyntaxFormatters = {
   underline: {
     icon: () => <Icon src={UnderlineIcon} />,
     label: 'Underline',