fix: notes list options menu not toggling correctly (#840)

2022-02-01 20:12:57 +05:30
parent 6150f58c73
commit 1f5d235cd4
5 changed files with 93 additions and 51 deletions
--- a/app/assets/javascripts/components/NotesListOptionsMenu.tsx
+++ b/app/assets/javascripts/components/NotesListOptionsMenu.tsx
@@ -6,19 +6,19 @@ import { useRef, useState } from 'preact/hooks';
 import { Icon } from './Icon';
 import { Menu } from './menu/Menu';
 import { MenuItem, MenuItemSeparator, MenuItemType } from './menu/MenuItem';
-import { useCloseOnClickOutside } from './utils';

 type Props = {
  application: WebApplication;
+  closeOnBlur: (event: { relatedTarget: EventTarget | null }) => void;
  closeDisplayOptionsMenu: () => void;
 };

 export const NotesListOptionsMenu: FunctionComponent<Props> = observer(
-  ({ closeDisplayOptionsMenu, application }) => {
+  ({ closeDisplayOptionsMenu, closeOnBlur, application }) => {
    const menuClassName =
      'sn-dropdown sn-dropdown--animated min-w-70 overflow-y-auto \
 border-1 border-solid border-main text-sm z-index-dropdown-menu \
-flex flex-col py-2 bottom-0 left-2 absolute';
+flex flex-col py-2 top-full bottom-0 left-2 absolute';
    const [sortBy, setSortBy] = useState(() =>
      application.getPreference(PrefKey.SortNotesBy, CollectionSort.CreatedAt)
    );
@@ -118,10 +118,6 @@ flex flex-col py-2 bottom-0 left-2 absolute';

    const menuRef = useRef<HTMLDivElement>(null);

-    useCloseOnClickOutside(menuRef, () => {
-      closeDisplayOptionsMenu();
-    });
-
    return (
      <div ref={menuRef} className={menuClassName}>
        <Menu a11yLabel="Sort by" closeMenu={closeDisplayOptionsMenu}>
@@ -133,6 +129,7 @@ flex flex-col py-2 bottom-0 left-2 absolute';
            type={MenuItemType.RadioButton}
            onClick={toggleSortByDateModified}
            checked={sortBy === CollectionSort.UpdatedAt}
+            onBlur={closeOnBlur}
          >
            <div className="flex flex-grow items-center justify-between">
              <span>Date modified</span>
@@ -150,6 +147,7 @@ flex flex-col py-2 bottom-0 left-2 absolute';
            type={MenuItemType.RadioButton}
            onClick={toggleSortByCreationDate}
            checked={sortBy === CollectionSort.CreatedAt}
+            onBlur={closeOnBlur}
          >
            <div className="flex flex-grow items-center justify-between">
              <span>Creation date</span>
@@ -167,6 +165,7 @@ flex flex-col py-2 bottom-0 left-2 absolute';
            type={MenuItemType.RadioButton}
            onClick={toggleSortByTitle}
            checked={sortBy === CollectionSort.Title}
+            onBlur={closeOnBlur}
          >
            <div className="flex flex-grow items-center justify-between">
              <span>Title</span>
@@ -188,6 +187,7 @@ flex flex-col py-2 bottom-0 left-2 absolute';
            className="py-1 hover:bg-contrast focus:bg-info-backdrop"
            checked={!hidePreview}
            onChange={toggleHidePreview}
+            onBlur={closeOnBlur}
          >
            <div className="flex flex-col max-w-3/4">Show note preview</div>
          </MenuItem>
@@ -196,6 +196,7 @@ flex flex-col py-2 bottom-0 left-2 absolute';
            className="py-1 hover:bg-contrast focus:bg-info-backdrop"
            checked={!hideDate}
            onChange={toggleHideDate}
+            onBlur={closeOnBlur}
          >
            Show date
          </MenuItem>
@@ -204,6 +205,7 @@ flex flex-col py-2 bottom-0 left-2 absolute';
            className="py-1 hover:bg-contrast focus:bg-info-backdrop"
            checked={!hideTags}
            onChange={toggleHideTags}
+            onBlur={closeOnBlur}
          >
            Show tags
          </MenuItem>
@@ -212,6 +214,7 @@ flex flex-col py-2 bottom-0 left-2 absolute';
            className="py-1 hover:bg-contrast focus:bg-info-backdrop"
            checked={!hideEditorIcon}
            onChange={toggleEditorIcon}
+            onBlur={closeOnBlur}
          >
            Show editor icon
          </MenuItem>
@@ -224,6 +227,7 @@ flex flex-col py-2 bottom-0 left-2 absolute';
            className="py-1 hover:bg-contrast focus:bg-info-backdrop"
            checked={!hidePinned}
            onChange={toggleHidePinned}
+            onBlur={closeOnBlur}
          >
            Show pinned notes
          </MenuItem>
@@ -232,6 +236,7 @@ flex flex-col py-2 bottom-0 left-2 absolute';
            className="py-1 hover:bg-contrast focus:bg-info-backdrop"
            checked={!hideProtected}
            onChange={toggleHideProtected}
+            onBlur={closeOnBlur}
          >
            Show protected notes
          </MenuItem>
@@ -240,6 +245,7 @@ flex flex-col py-2 bottom-0 left-2 absolute';
            className="py-1 hover:bg-contrast focus:bg-info-backdrop"
            checked={showArchived}
            onChange={toggleShowArchived}
+            onBlur={closeOnBlur}
          >
            Show archived notes
          </MenuItem>
@@ -248,6 +254,7 @@ flex flex-col py-2 bottom-0 left-2 absolute';
            className="py-1 hover:bg-contrast focus:bg-info-backdrop"
            checked={showTrashed}
            onChange={toggleShowTrashed}
+            onBlur={closeOnBlur}
          >
            Show trashed notes
          </MenuItem>
--- a/app/assets/javascripts/components/NotesView.tsx
+++ b/app/assets/javascripts/components/NotesView.tsx
@@ -5,7 +5,7 @@ import { PANEL_NAME_NOTES } from '@/views/constants';
 import { PrefKey } from '@standardnotes/snjs';
 import { observer } from 'mobx-react-lite';
 import { FunctionComponent } from 'preact';
-import { useEffect, useRef } from 'preact/hooks';
+import { useEffect, useRef, useState } from 'preact/hooks';
 import { NoAccountWarning } from './NoAccountWarning';
 import { NotesList } from './NotesList';
 import { NotesListOptionsMenu } from './NotesListOptionsMenu';
@@ -16,6 +16,12 @@ import {
  PanelResizer,
  PanelResizeType,
 } from './PanelResizer';
+import {
+  Disclosure,
+  DisclosureButton,
+  DisclosurePanel,
+} from '@reach/disclosure';
+import { useCloseOnBlur } from './utils';

 type Props = {
  application: WebApplication;
@@ -25,6 +31,7 @@ type Props = {
 export const NotesView: FunctionComponent<Props> = observer(
  ({ application, appState }) => {
    const notesViewPanelRef = useRef<HTMLDivElement>(null);
+    const displayOptionsMenuRef = useRef<HTMLDivElement>(null);

    const {
      completedFullSync,
@@ -36,8 +43,6 @@ export const NotesView: FunctionComponent<Props> = observer(
      renderedNotes,
      selectedNotes,
      setNoteFilterText,
-      showDisplayOptionsMenu,
-      toggleDisplayOptionsMenu,
      searchBarElement,
      selectNextNote,
      selectPreviousNote,
@@ -49,6 +54,13 @@ export const NotesView: FunctionComponent<Props> = observer(
      panelWidth,
    } = appState.notesView;

+    const [showDisplayOptionsMenu, setShowDisplayOptionsMenu] = useState(false);
+
+    const [closeDisplayOptMenuOnBlur] = useCloseOnBlur(
+      displayOptionsMenuRef,
+      setShowDisplayOptionsMenu
+    );
+
    useEffect(() => {
      handleFilterTextChanged();
    }, [noteFilterText, handleFilterTextChanged]);
@@ -139,6 +151,10 @@ export const NotesView: FunctionComponent<Props> = observer(
      appState.noteTags.reloadTagsContainerMaxWidth();
    };

+    const toggleDisplayOptionsMenu = () => {
+      setShowDisplayOptionsMenu(!showDisplayOptionsMenu);
+    };
+
    return (
      <div
        id="notes-column"
@@ -192,34 +208,42 @@ export const NotesView: FunctionComponent<Props> = observer(
              </div>
              <NoAccountWarning appState={appState} />
            </div>
-            <div id="notes-menu-bar" className="sn-component">
+            <div
+              id="notes-menu-bar"
+              className="sn-component"
+              ref={displayOptionsMenuRef}
+            >
              <div className="sk-app-bar no-edges">
                <div className="left">
-                  <div
-                    className={`sk-app-bar-item ${
-                      showDisplayOptionsMenu ? 'selected' : ''
-                    }`}
-                    onClick={() =>
-                      toggleDisplayOptionsMenu(!showDisplayOptionsMenu)
-                    }
+                  <Disclosure
+                    open={showDisplayOptionsMenu}
+                    onChange={toggleDisplayOptionsMenu}
                  >
-                    <div className="sk-app-bar-item-column">
-                      <div className="sk-label">Options</div>
-                    </div>
-                    <div className="sk-app-bar-item-column">
-                      <div className="sk-sublabel">{optionsSubtitle}</div>
-                    </div>
-                  </div>
+                    <DisclosureButton
+                      className={`sk-app-bar-item bg-contrast border-0 focus:shadow-none ${
+                        showDisplayOptionsMenu ? 'selected' : ''
+                      }`}
+                      onBlur={closeDisplayOptMenuOnBlur}
+                    >
+                      <div className="sk-app-bar-item-column">
+                        <div className="sk-label">Options</div>
+                      </div>
+                      <div className="sk-app-bar-item-column">
+                        <div className="sk-sublabel">{optionsSubtitle}</div>
+                      </div>
+                    </DisclosureButton>
+                    <DisclosurePanel onBlur={closeDisplayOptMenuOnBlur}>
+                      {showDisplayOptionsMenu && (
+                        <NotesListOptionsMenu
+                          application={application}
+                          closeDisplayOptionsMenu={toggleDisplayOptionsMenu}
+                          closeOnBlur={closeDisplayOptMenuOnBlur}
+                        />
+                      )}
+                    </DisclosurePanel>
+                  </Disclosure>
                </div>
              </div>
-              {showDisplayOptionsMenu && (
-                <NotesListOptionsMenu
-                  application={application}
-                  closeDisplayOptionsMenu={() =>
-                    toggleDisplayOptionsMenu(false)
-                  }
-                />
-              )}
            </div>
          </div>
          {completedFullSync && !renderedNotes.length ? (
--- a/app/assets/javascripts/components/menu/MenuItem.tsx
+++ b/app/assets/javascripts/components/menu/MenuItem.tsx
@@ -1,14 +1,10 @@
-import {
-  ComponentChild,
-  ComponentChildren,
-  FunctionComponent,
-  VNode,
-} from 'preact';
+import { ComponentChildren, FunctionComponent, VNode } from 'preact';
 import { forwardRef, Ref } from 'preact/compat';
 import { JSXInternal } from 'preact/src/jsx';
 import { Icon } from '../Icon';
 import { Switch, SwitchProps } from '../Switch';
 import { IconType } from '@standardnotes/snjs';
+import { FOCUSABLE_BUT_NOT_TABBABLE } from '@/views/constants';

 export enum MenuItemType {
  IconButton,
@@ -21,6 +17,7 @@ type MenuItemProps = {
  children: ComponentChildren;
  onClick?: JSXInternal.MouseEventHandler<HTMLButtonElement>;
  onChange?: SwitchProps['onChange'];
+  onBlur?: (event: { relatedTarget: EventTarget | null }) => void;
  className?: string;
  checked?: boolean;
  icon?: IconType;
@@ -34,6 +31,7 @@ export const MenuItem: FunctionComponent<MenuItemProps> = forwardRef(
      children,
      onClick,
      onChange,
+      onBlur,
      className = '',
      type,
      checked,
@@ -45,22 +43,31 @@ export const MenuItem: FunctionComponent<MenuItemProps> = forwardRef(
  ) => {
    return type === MenuItemType.SwitchButton &&
      typeof onChange === 'function' ? (
-      <Switch
-        className="py-1 hover:bg-contrast focus:bg-info-backdrop"
-        checked={checked}
-        onChange={onChange}
+      <button
+        className="sn-dropdown-item focus:bg-info-backdrop focus:shadow-none justify-between"
+        onClick={() => {
+          onChange(!checked);
+        }}
+        onBlur={onBlur}
+        tabIndex={
+          typeof tabIndex === 'number' ? tabIndex : FOCUSABLE_BUT_NOT_TABBABLE
+        }
        role="menuitemcheckbox"
-        tabIndex={typeof tabIndex === 'number' ? tabIndex : -1}
+        aria-checked={checked}
      >
-        {children}
-      </Switch>
+        <span className="flex flex-grow items-center">{children}</span>
+        <Switch className="px-0" checked={checked} />
+      </button>
    ) : (
      <button
        ref={ref}
        role={type === MenuItemType.RadioButton ? 'menuitemradio' : 'menuitem'}
-        tabIndex={typeof tabIndex === 'number' ? tabIndex : -1}
+        tabIndex={
+          typeof tabIndex === 'number' ? tabIndex : FOCUSABLE_BUT_NOT_TABBABLE
+        }
        className={`sn-dropdown-item focus:bg-info-backdrop focus:shadow-none ${className}`}
        onClick={onClick}
+        onBlur={onBlur}
        {...(type === MenuItemType.RadioButton
          ? { 'aria-checked': checked }
          : {})}
--- a/app/assets/javascripts/ui_models/app_state/notes_view_state.ts
+++ b/app/assets/javascripts/ui_models/app_state/notes_view_state.ts
@@ -146,7 +146,7 @@ export class NotesViewState {
        } else if (eventName === AppStateEvent.ActiveEditorChanged) {
          this.handleEditorChange();
        } else if (eventName === AppStateEvent.EditorFocused) {
-          this.toggleDisplayOptionsMenu(false);
+          this.setShowDisplayOptionsMenu(false);
        }
      })
    );
@@ -169,7 +169,7 @@ export class NotesViewState {
      setCompletedFullSync: action,
      setNoteFilterText: action,
      syncSelectedNotes: action,
-      toggleDisplayOptionsMenu: action,
+      setShowDisplayOptionsMenu: action,
      onFilterEnter: action,
      handleFilterTextChanged: action,

@@ -185,7 +185,7 @@ export class NotesViewState {
    this.completedFullSync = completed;
  };

-  toggleDisplayOptionsMenu = (enabled: boolean) => {
+  setShowDisplayOptionsMenu = (enabled: boolean) => {
    this.showDisplayOptionsMenu = enabled;
  };

@@ -505,7 +505,7 @@ export class NotesViewState {

  handleTagChange = () => {
    this.resetScrollPosition();
-    this.toggleDisplayOptionsMenu(false);
+    this.setShowDisplayOptionsMenu(false);
    this.setNoteFilterText('');
    this.application.getDesktopService().searchText();
    this.resetPagination();
--- a/app/assets/stylesheets/_sn.scss
+++ b/app/assets/stylesheets/_sn.scss
@@ -683,6 +683,10 @@
  top: 50%;
 }

+.top-full {
+  top: 100%;
+}
+
 .left-2 {
  left: 0.5rem;
 }