Browse Source

Sidebar animation fixes (#8423). Fix #8311

Co-authored-by: Thibaud Colas <thibaudcolas@gmail.com>

- Animations – The close animation for sub-menus doesn't seem to play when the menu is expanded - Made it so sub menu's stay open when the menu is expanded and collapsed
- Animations – The account menu seems to have a different tween animation to the rest of the menu. Causing it to do a weird thing when you collapse the menu while the account menu is open
- Animations – The avatar suddenly jumps to the right when you collapse the menu
- Animations – The Bird seems to have two hover states (try slowly moving your mouse cursor from top to bottom and you'll see it's wing appears before the hover animation is triggered). Not sure if this is intended behaviour.
- Accessibility – Focus order is incorrect on the mobile version (it should be possible to move to the sidebar after having toggled it)
- Try and fit more letters in to the sidebar menu items by reduce the padding / margin on the right side of the arrow, and reduce the gap between the icon and the text a tiny bit
- Make it so when you have a menu open (e.g. Bakery misc) and you click the slim sidebar icon, the menu stays open as the menu gets slim.
- Add a label to the sidebar’s `<aside>`
Steve Stein 2 years ago
parent
commit
db5f4106db

+ 1 - 2
client/scss/components/_grid.legacy.scss

@@ -1,10 +1,9 @@
 .wrapper {
   @include clearfix();
+  @apply w-transition-sidebar;
   height: 100vh;
-  transition: transform 0.2s ease;
 
   @include media-breakpoint-up(sm) {
-    @include transition(padding-inline-start $menu-transition-duration ease);
     transform: none;
     padding-inline-start: $menu-width;
 

+ 12 - 10
client/src/components/Sidebar/Sidebar.scss

@@ -41,22 +41,15 @@
   }
 }
 
-.sidebar {
-  @apply w-fixed w-flex w-flex-col w-h-full w-bg-primary w-z-[300];
+.sidebar,
+.sidebar-loading {
+  @apply w-fixed w-flex w-flex-col w-h-full w-bg-primary w-z-[300] w-transition-sidebar;
   width: $menu-width;
   // Remove once we drop support for Safari 13.
   // stylelint-disable-next-line property-disallowed-list
   left: 0;
   inset-inline-start: 0;
 
-  @include transition(
-    width $menu-transition-duration ease,
-    // Remove once we drop support for Safari 13.
-    // stylelint-disable-next-line property-disallowed-list
-    left $menu-transition-duration ease,
-    inset-inline-start $menu-transition-duration ease
-  );
-
   @media (forced-colors: $media-forced-colours) {
     border-inline-end: 1px solid transparent;
   }
@@ -79,6 +72,11 @@
     inset-inline-start: -$menu-width;
   }
 
+  // When sidebar is completely closed and animations have finished
+  &--closed {
+    display: none;
+  }
+
   &__inner {
     // On medium, make it possible for the nav links to scroll.
     @apply w-h-full w-bg-primary w-flex w-flex-col w-flex-nowrap;
@@ -95,6 +93,10 @@
   }
 }
 
+.sidebar-collapsed .sidebar-loading {
+  width: $menu-width-slim;
+}
+
 // This is a separate component as it needs to display in the header
 .sidebar-nav-toggle {
   @include sidebar-toggle;

+ 57 - 24
client/src/components/Sidebar/Sidebar.tsx

@@ -10,7 +10,7 @@ export interface ModuleRenderContext {
   key: number;
   slim: boolean;
   expandingOrCollapsing: boolean;
-  onAccountExpand: () => void;
+  onHideMobile: () => void;
   onSearchClick: () => void;
   currentPath: string;
   navigate(url: string): Promise<void>;
@@ -39,6 +39,7 @@ export const Sidebar: React.FunctionComponent<SidebarProps> = ({
   // It records the user's general preference for a collapsed/uncollapsed menu
   // This is just a hint though, and we may still collapse the menu if the screen is too small
   const [collapsed, setCollapsed] = React.useState(collapsedOnLoad);
+  const mobileNavToggleRef = React.useRef<HTMLButtonElement>(null);
 
   // Call onExpandCollapse(true) if menu is initialised in collapsed state
   React.useEffect(() => {
@@ -50,6 +51,8 @@ export const Sidebar: React.FunctionComponent<SidebarProps> = ({
   // 'visibleOnMobile' indicates whether the sidebar is currently visible on mobile
   // On mobile, the sidebar is completely hidden by default and must be opened manually
   const [visibleOnMobile, setVisibleOnMobile] = React.useState(false);
+  // 'closedOnMobile' is used to set the menu to display none so it can no longer be interacted with by keyboard when its hidden
+  const [closedOnMobile, setClosedOnMobile] = React.useState(true);
 
   // Tracks whether the screen is below 800 pixels. In this state, the menu is completely hidden.
   // State is used here in case the user changes their browser size
@@ -59,27 +62,36 @@ export const Sidebar: React.FunctionComponent<SidebarProps> = ({
     function handleResize() {
       if (checkWindowSizeIsMobile()) {
         setIsMobile(true);
+        return null;
       } else {
         setIsMobile(false);
 
-        // Close the menu as this state is not used in desktop
+        // Close the menu and animate out as this state is not used in desktop
         setVisibleOnMobile(false);
+        // wait for animation to finish then hide menu from screen readers as well.
+        return setTimeout(() => {
+          setClosedOnMobile(true);
+        }, SIDEBAR_TRANSITION_DURATION);
       }
     }
 
     window.addEventListener('resize', handleResize);
-    handleResize();
-    return () => window.removeEventListener('resize', handleResize);
+    const closeTimeout = handleResize();
+    return () => {
+      window.removeEventListener('resize', handleResize);
+      if (closeTimeout) {
+        clearTimeout(closeTimeout);
+      }
+    };
   }, []);
 
   // Whether or not to display the menu with slim layout.
-  // Separate from 'collapsed' as the menu can still be displayed with an expanded
-  // layout while in 'collapsed' mode if the user is 'peeking' into it (see above)
   const slim = collapsed && !isMobile;
 
   // 'expandingOrCollapsing' is set to true whilst the the menu is transitioning between slim and expanded layouts
   const [expandingOrCollapsing, setExpandingOrCollapsing] =
     React.useState(false);
+
   React.useEffect(() => {
     setExpandingOrCollapsing(true);
     const finishTimeout = setTimeout(() => {
@@ -105,6 +117,7 @@ export const Sidebar: React.FunctionComponent<SidebarProps> = ({
 
     const finishTimeout = setTimeout(() => {
       setExpandingOrCollapsing(false);
+      setClosedOnMobile(!closedOnMobile);
     }, SIDEBAR_TRANSITION_DURATION);
     return () => {
       clearTimeout(finishTimeout);
@@ -133,9 +146,25 @@ export const Sidebar: React.FunctionComponent<SidebarProps> = ({
     }
   };
 
-  const onAccountExpand = () => {
-    if (slim) {
-      onClickCollapseToggle();
+  React.useEffect(() => {
+    // wait for animation to finish then hide menu from screen readers as well.
+    const finishHidingMenu = setTimeout(() => {
+      if (!visibleOnMobile) {
+        setClosedOnMobile(true);
+      }
+    }, SIDEBAR_TRANSITION_DURATION);
+
+    return () => {
+      clearTimeout(finishHidingMenu);
+    };
+  }, [visibleOnMobile]);
+
+  const onHideMobile = () => {
+    setVisibleOnMobile(false);
+
+    if (mobileNavToggleRef) {
+      // When menu is closed with escape key bring focus back to open close toggle
+      mobileNavToggleRef.current?.focus();
     }
   };
 
@@ -145,7 +174,7 @@ export const Sidebar: React.FunctionComponent<SidebarProps> = ({
       key: index,
       slim,
       expandingOrCollapsing,
-      onAccountExpand,
+      onHideMobile,
       onSearchClick,
       currentPath,
       navigate,
@@ -154,12 +183,29 @@ export const Sidebar: React.FunctionComponent<SidebarProps> = ({
 
   return (
     <>
+      <button
+        onClick={onClickOpenCloseToggle}
+        aria-label={gettext('Toggle sidebar')}
+        aria-expanded={visibleOnMobile ? 'true' : 'false'}
+        className={
+          'button sidebar-nav-toggle' +
+          (isMobile ? ' sidebar-nav-toggle--mobile' : '') +
+          (visibleOnMobile ? ' sidebar-nav-toggle--open' : '')
+        }
+        type="button"
+        ref={mobileNavToggleRef}
+      >
+        {visibleOnMobile ? <Icon name="cross" /> : <Icon name="bars" />}
+      </button>
       <div
         className={
           'sidebar' +
           (slim ? ' sidebar--slim' : '') +
           (isMobile ? ' sidebar--mobile' : '') +
-          (isMobile && !visibleOnMobile ? ' sidebar--hidden' : '')
+          (isMobile && !visibleOnMobile ? ' sidebar--hidden' : '') +
+          (isMobile && !visibleOnMobile && closedOnMobile
+            ? ' sidebar--closed'
+            : '')
         }
       >
         <div
@@ -197,19 +243,6 @@ export const Sidebar: React.FunctionComponent<SidebarProps> = ({
           {renderedModules}
         </div>
       </div>
-      <button
-        onClick={onClickOpenCloseToggle}
-        aria-label={gettext('Toggle sidebar')}
-        aria-expanded={visibleOnMobile ? 'true' : 'false'}
-        className={
-          'button sidebar-nav-toggle' +
-          (isMobile ? ' sidebar-nav-toggle--mobile' : '') +
-          (visibleOnMobile ? ' sidebar-nav-toggle--open' : '')
-        }
-        type="button"
-      >
-        {visibleOnMobile ? <Icon name="cross" /> : <Icon name="bars" />}
-      </button>
     </>
   );
 };

+ 1 - 1
client/src/components/Sidebar/SidebarPanel.scss

@@ -1,5 +1,5 @@
 .sidebar-panel {
-  @apply w-transition w-duration-150;
+  @apply w-transition-sidebar;
   // With CSS variable allows panels with different widths to animate properly
   --width: #{$menu-width};
 

+ 11 - 11
client/src/components/Sidebar/__snapshots__/Sidebar.test.js.snap

@@ -2,6 +2,17 @@
 
 exports[`Sidebar should render with the minimum required props 1`] = `
 <Fragment>
+  <button
+    aria-expanded="false"
+    aria-label="Toggle sidebar"
+    className="button sidebar-nav-toggle"
+    onClick={[Function]}
+    type="button"
+  >
+    <Icon
+      name="bars"
+    />
+  </button>
   <div
     className="sidebar"
   >
@@ -36,16 +47,5 @@ exports[`Sidebar should render with the minimum required props 1`] = `
       </div>
     </div>
   </div>
-  <button
-    aria-expanded="false"
-    aria-label="Toggle sidebar"
-    className="button sidebar-nav-toggle"
-    onClick={[Function]}
-    type="button"
-  >
-    <Icon
-      name="bars"
-    />
-  </button>
 </Fragment>
 `;

+ 3 - 0
client/src/components/Sidebar/index.tsx

@@ -52,6 +52,9 @@ export function initSidebar() {
       element,
       () => {
         document.body.classList.add('ready');
+        document
+          .querySelector('[data-wagtail-sidebar]')
+          ?.classList.remove('sidebar-loading');
       },
     );
   }

+ 2 - 3
client/src/components/Sidebar/menu/MenuItem.scss

@@ -24,7 +24,7 @@
     background: transparent;
     text-align: start;
     color: $color-menu-text;
-    padding: 13px 20px;
+    padding: 13px 15px 13px 20px;
     font-weight: 400;
     overflow: hidden;
 
@@ -85,7 +85,7 @@
 
 .menuitem-label {
   @include transition(opacity $menu-transition-duration ease);
-  margin-inline-start: 1rem;
+  margin-inline-start: 0.875rem;
   line-height: 1;
   text-overflow: ellipsis;
   white-space: nowrap;
@@ -116,7 +116,6 @@
 
     .sidebar-menu-item__link {
       justify-content: flex-start;
-      padding: 20px;
     }
   }
 }

+ 5 - 4
client/src/components/Sidebar/menu/SubMenuItem.scss

@@ -1,7 +1,7 @@
 .sidebar-sub-menu-trigger-icon {
   $root: &;
   display: block;
-  width: 1.25rem;
+  width: 1rem;
   height: 1rem;
   // Remove once we drop support for Safari 13.
   // stylelint-disable-next-line property-disallowed-list
@@ -31,7 +31,7 @@
 }
 
 .sidebar-sub-menu-panel {
-  @apply w-flex w-flex-col w-bg-primary-200 w-h-screen;
+  @apply w-flex w-flex-col w-bg-primary-200 w-h-screen w-transition-sidebar;
   width: $menu-width;
 
   > h2,
@@ -40,8 +40,8 @@
   }
 
   > h2 {
-    // w-min-h-[160px] is to vertically align the title and icon combination to the search input on the left
-    @apply w-min-h-[220px] w-mt-0 w-px-4 w-box-border w-text-center w-text-white w-mb-0 w-inline-flex w-flex-col w-justify-center w-items-center;
+    // w-min-h-[160px] and w-mt-[35px] classes are to vertically align the title and icon combination to the search input on the left
+    @apply w-min-h-[160px] w-mt-[45px] w-px-4 w-box-border w-text-center w-text-white w-mb-0 w-inline-flex w-flex-col w-justify-center w-items-center w-transition-sidebar;
 
     &:before {
       font-size: 4em;
@@ -64,6 +64,7 @@
   }
 
   > ul {
+    @apply w-scrollbar-thin;
     flex-grow: 1;
     padding: 0;
     margin: 0;

+ 2 - 17
client/src/components/Sidebar/modules/MainMenu.scss

@@ -2,14 +2,6 @@
 .sidebar-main-menu {
   overflow: auto;
   overflow-x: hidden;
-  // So the last items in the menu will be seen when the menu is vertically scrollable
-  margin-bottom: 52px;
-
-  @include transition(margin-bottom $menu-transition-duration ease);
-
-  &--open-footer {
-    margin-bottom: 127px;
-  }
 
   &__list {
     margin: 0;
@@ -40,9 +32,8 @@
 }
 
 .sidebar-footer {
-  @apply w-bg-primary w-bottom-0 w-fixed;
+  @apply w-bg-primary w-mt-auto;
   transition: width $menu-transition-duration ease !important; // Override body.ready
-  width: $menu-width;
 
   > ul,
   ul > li {
@@ -53,8 +44,6 @@
 
   ul > li {
     position: relative;
-
-    @include transition(border-color $menu-transition-duration ease);
   }
 
   > ul {
@@ -90,10 +79,8 @@
   }
 
   @at-root .sidebar--slim #{&} {
-    width: $menu-width-slim;
-
     &__account {
-      @apply w-px-0 w-pb-3 w-justify-center;
+      @apply w-pb-3;
     }
 
     &__account-toggle {
@@ -102,8 +89,6 @@
   }
 
   &--open {
-    width: $menu-width !important; // Override collapsed style
-
     > ul {
       $footer-submenu-height: 85px;
       max-height: $footer-submenu-height;

+ 2 - 13
client/src/components/Sidebar/modules/MainMenu.test.js

@@ -4,16 +4,10 @@ import { Menu } from './MainMenu';
 
 describe('Menu', () => {
   const user = { avatarUrl: 'https://gravatar/profile' };
-  const onAccountExpand = jest.fn();
 
   it('should render with the minimum required props', () => {
     const wrapper = shallow(
-      <Menu
-        accountMenuItems={[]}
-        menuItems={[]}
-        user={user}
-        onAccountExpand={onAccountExpand}
-      />,
+      <Menu accountMenuItems={[]} menuItems={[]} user={user} />,
     );
 
     expect(wrapper).toMatchSnapshot();
@@ -21,12 +15,7 @@ describe('Menu', () => {
 
   it('should toggle the sidebar footer (account) when clicked', () => {
     const wrapper = shallow(
-      <Menu
-        accountMenuItems={[]}
-        menuItems={[]}
-        user={user}
-        onAccountExpand={onAccountExpand}
-      />,
+      <Menu accountMenuItems={[]} menuItems={[]} user={user} />,
     );
 
     // default is closed

+ 24 - 30
client/src/components/Sidebar/modules/MainMenu.tsx

@@ -67,8 +67,9 @@ interface MenuProps {
   user: MainMenuModuleDefinition['user'];
   slim: boolean;
   expandingOrCollapsing: boolean;
-  onAccountExpand: () => void;
+  onHideMobile: () => void;
   currentPath: string;
+
   navigate(url: string): Promise<void>;
 }
 
@@ -77,7 +78,7 @@ export const Menu: React.FunctionComponent<MenuProps> = ({
   accountMenuItems,
   user,
   expandingOrCollapsing,
-  onAccountExpand,
+  onHideMobile,
   slim,
   currentPath,
   navigate,
@@ -91,8 +92,18 @@ export const Menu: React.FunctionComponent<MenuProps> = ({
     navigationPath: '',
     activePath: '',
   });
-  const accountSettingsOpen = state.navigationPath.startsWith('.account');
   const isVisible = !slim || expandingOrCollapsing;
+  const accountSettingsOpen = state.navigationPath.startsWith('.account');
+
+  React.useEffect(() => {
+    // Force account navigation to closed state when in slim mode
+    if (slim && accountSettingsOpen) {
+      dispatch({
+        type: 'set-navigation-path',
+        path: '',
+      });
+    }
+  }, [slim]);
 
   // Whenever currentPath or menu changes, work out new activePath
   React.useEffect(() => {
@@ -138,6 +149,10 @@ export const Menu: React.FunctionComponent<MenuProps> = ({
           type: 'set-navigation-path',
           path: '',
         });
+
+        if (state.navigationPath === '') {
+          onHideMobile();
+        }
       }
     };
 
@@ -164,29 +179,8 @@ export const Menu: React.FunctionComponent<MenuProps> = ({
     };
   }, []);
 
-  // Determine if the sidebar is expanded from account button click
-  const [expandedFromAccountClick, setExpandedFromAccountClick] =
-    React.useState<boolean>(false);
-
-  // Whenever the parent Sidebar component collapses or expands, close any open menus
-  React.useEffect(() => {
-    if (expandingOrCollapsing && !expandedFromAccountClick) {
-      dispatch({
-        type: 'set-navigation-path',
-        path: '',
-      });
-    }
-    if (expandedFromAccountClick) {
-      setExpandedFromAccountClick(false);
-    }
-  }, [expandingOrCollapsing]);
-
   const onClickAccountSettings = () => {
     // Pass account expand information to Sidebar component
-    onAccountExpand();
-    if (slim) {
-      setExpandedFromAccountClick(true);
-    }
 
     if (accountSettingsOpen) {
       dispatch({
@@ -219,9 +213,10 @@ export const Menu: React.FunctionComponent<MenuProps> = ({
           (isVisible ? ' sidebar-footer--visible' : '')
         }
       >
-        <Tippy disabled={!slim} content={gettext('Account')} placement="right">
+        <Tippy disabled={!slim} content={user.name} placement="right">
           <button
-            className="
+            className={`
+            ${slim ? 'w-px-4' : 'w-px-5'}
             sidebar-footer__account
             w-bg-primary
             w-text-white
@@ -232,11 +227,10 @@ export const Menu: React.FunctionComponent<MenuProps> = ({
             w-appearance-none
             w-border-0
             w-overflow-hidden
-            w-px-5
             w-py-3
             hover:w-bg-primary-200
             focus:w-bg-primary-200
-            w-transition"
+            w-transition`}
             title={gettext('Edit your account')}
             onClick={onClickAccountSettings}
             aria-label={gettext('Edit your account')}
@@ -288,7 +282,7 @@ export class MainMenuModuleDefinition implements ModuleDefinition {
   render({
     slim,
     expandingOrCollapsing,
-    onAccountExpand,
+    onHideMobile,
     key,
     currentPath,
     navigate,
@@ -300,7 +294,7 @@ export class MainMenuModuleDefinition implements ModuleDefinition {
         user={this.user}
         slim={slim}
         expandingOrCollapsing={expandingOrCollapsing}
-        onAccountExpand={onAccountExpand}
+        onHideMobile={onHideMobile}
         key={key}
         currentPath={currentPath}
         navigate={navigate}

+ 3 - 3
client/src/components/Sidebar/modules/WagtailBranding.scss

@@ -36,9 +36,9 @@ $logo-size: 110px;
   // Reduce overall size when in slim mode
   .sidebar--slim & {
     @include show-focus-outline-inside();
-    margin: 1.125em auto 2.5em;
-    width: 60px;
-    height: 60px;
+    margin: 1.125em auto 4em;
+    width: 40px;
+    height: 40px;
   }
 
   // Remove background on 404 page

+ 1 - 2
client/src/components/Sidebar/modules/__snapshots__/MainMenu.test.js.snap

@@ -14,7 +14,6 @@ exports[`Menu should render with the minimum required props 1`] = `
     className="sidebar-footer sidebar-footer--visible"
   >
     <ForwardRef(TippyWrapper)
-      content="Account"
       disabled={true}
       placement="right"
     >
@@ -23,6 +22,7 @@ exports[`Menu should render with the minimum required props 1`] = `
         aria-haspopup="menu"
         aria-label="Edit your account"
         className="
+            w-px-5
             sidebar-footer__account
             w-bg-primary
             w-text-white
@@ -33,7 +33,6 @@ exports[`Menu should render with the minimum required props 1`] = `
             w-appearance-none
             w-border-0
             w-overflow-hidden
-            w-px-5
             w-py-3
             hover:w-bg-primary-200
             focus:w-bg-primary-200

+ 4 - 0
client/tailwind.config.js

@@ -76,6 +76,10 @@ module.exports = {
       outlineOffset: {
         inside: '-3px',
       },
+      transitionProperty: {
+        sidebar:
+          'left, inset-inline-start, padding-inline-start, width, transform, margin-top, min-height',
+      },
     },
   },
   plugins: [

+ 1 - 2
wagtail/admin/templates/wagtailadmin/base.html

@@ -4,8 +4,7 @@
 {% block furniture %}
     <template data-wagtail-sidebar-branding-logo>{% block branding_logo %}{% endblock %}</template>
     {% sidebar_props %}
-    <aside id="wagtail-sidebar" data-wagtail-sidebar></aside>
-
+    <aside id="wagtail-sidebar" class="sidebar-loading" data-wagtail-sidebar aria-label="{% trans 'Sidebar' %}"></aside>
     <main class="content-wrapper" id="main">
         <div class="content">
             {# Always show messages div so it can be appended to by JS #}