refactor: wp-791 dropdown menu (#1028)

* refactor: WP-791 dropdown button menu - remove most of the redundant code - use Dropdown not ButtonDropdown - fix layout of menu and button in DataFilesSidebar - clean up our .dropdown-menu styles - use variables for arrow and border * refactor: WP-791 DataFilesBread… dropdown menu `….scss`: - use class not ID - position via `top` not `margin-top` - explain menu position's "magic" value - explain why positioning is necessary[^1] `….jsx`: - use class not ID use Dropdown not ButtonDropdown[^2] [^1]: Not desired, but necessary if we use our custom <Button> as is. [^2]: I don't see ButtonDropdown in latest ReactStrap (v9). * chore: remove class data-files-btn This class is assigned to buttons that: - **either** have Bootstrap btn-primary (in which case the `.workbench-content .btn-primary` styles it) - **or** are our custom `<Button>` (in which case the `composes: c-button` styles it) * doc: save `refactor/WP-791-dropdown-menu` PR desc * Revert "chore: remove class data-files-btn" This reverts commit 1b644ed. * style: npm run prettier:fix * chore: remove unnecessary `!important` * enhance: use `<DropdownItem divider>` consistently * chore: refactor-WP-791-dropdown-menu-pr-desc.md --------- Co-authored-by: Chandra Y <[email protected]>
TACC · Jan 8, 2025 · de51694 · de51694
1 parent 29e1bce
commit de51694
Show file tree

Hide file tree

Showing 6 changed files with 48 additions and 102 deletions.
diff --git a/client/src/components/DataFiles/DataFilesBreadcrumbs/DataFilesBreadcrumbs.scss b/client/src/components/DataFiles/DataFilesBreadcrumbs/DataFilesBreadcrumbs.scss
@@ -42,32 +42,15 @@
   padding-left: var(--horizontal-buffer);
 }
 
-/* Nested to prevent styles from affecting CMS header dropdown */
-/* HACK: Using ID to increase specificity (until source of problem is fixed) */
-/* HELP: Why does DataFilesSidebar not need such specificity? */
-/* .go-to-button-dropdown { */
-#go-to-button-dropdown {
-  /* To fix menu not showing */
-  /* HELP: Why does DataFilesSidebar not need this fix? */
+.go-to-button-dropdown {
   .dropdown-menu {
-    opacity: 1 !important;
-    pointer-events: auto !important;
-  }
-  /* To restyle */
-  .dropdown-menu {
-    margin-top: 38px;
+    /* To push menu down by the height of its button */
+    /* FAQ: Required because `tag={Button}` loses auto-position of menu */
+    top: 28px !important; /* to override `inset` from Popper via Reactstrap */
   }
   .dropdown-menu::before,
   .dropdown-menu::after {
     left: 23px;
-    margin-left: 0;
-  }
-  .dropdown-menu::after {
-    top: -9px;
-  }
-
-  .dropdown-item {
-    display: inline-block;
   }
 }
 
@@ -85,12 +68,12 @@
   padding-left: 20px;
 }
 
-#go-to-button-dropdown .complex-dropdown-item-root,
+.complex-dropdown-item-root,
 .complex-dropdown-item-project {
-  display: flex !important;
+  display: flex;
 }
 
-#go-to-button-dropdown .link-hover:hover {
+.go-to-button-dropdown .link-hover:hover {
   text-decoration: none;
 }
 

diff --git a/client/src/components/DataFiles/DataFilesDropdown/DataFilesDropdown.jsx b/client/src/components/DataFiles/DataFilesDropdown/DataFilesDropdown.jsx
@@ -3,10 +3,10 @@ import PropTypes from 'prop-types';
 import { Link, useLocation } from 'react-router-dom';
 import { Button } from '_common';
 import {
+  Dropdown,
   DropdownToggle,
   DropdownMenu,
   DropdownItem,
-  ButtonDropdown,
 } from 'reactstrap';
 import { useSystemDisplayName, useSystems } from 'hooks/datafiles';
 import '../DataFilesBreadcrumbs/DataFilesBreadcrumbs.scss';
@@ -75,10 +75,9 @@ const BreadcrumbsDropdown = ({
   const sliceStart = scheme === 'projects' && systemName ? 0 : 1;
   return (
     <div id="path-button-wrapper">
-      <ButtonDropdown
+      <Dropdown
         isOpen={dropdownOpen}
         toggle={toggleDropdown}
-        id="go-to-button-dropdown"
         className="go-to-button-dropdown"
       >
         <DropdownToggle tag={Button}>Go to ...</DropdownToggle>
@@ -132,7 +131,7 @@ const BreadcrumbsDropdown = ({
             </DropdownItem>
           </Link>
         </DropdownMenu>
-      </ButtonDropdown>
+      </Dropdown>
     </div>
   );
 };

diff --git a/client/src/components/DataFiles/DataFilesSidebar/DataFilesSidebar.jsx b/client/src/components/DataFiles/DataFilesSidebar/DataFilesSidebar.jsx
@@ -5,7 +5,7 @@ import {
   Nav,
   NavItem,
   NavLink,
-  ButtonDropdown,
+  Dropdown,
   DropdownMenu,
   DropdownToggle,
   DropdownItem,
@@ -67,8 +67,8 @@ const DataFilesAddButton = ({ readOnly }) => {
   const writeItemStyle = disabled ? 'read-only' : '';
 
   return (
-    <div id="add-button-wrapper">
-      <ButtonDropdown isOpen={dropdownOpen} toggle={toggleDropdown}>
+    <>
+      <Dropdown isOpen={dropdownOpen} toggle={toggleDropdown}>
         <DropdownToggle
           color="primary"
           id="data-files-add"
@@ -89,6 +89,7 @@ const DataFilesAddButton = ({ readOnly }) => {
               <i className="icon-folder" /> Shared Workspace
             </DropdownItem>
           )}
+          <DropdownItem divider />
           <DropdownItem
             className={`complex-dropdown-item ${styles[writeItemStyle]}`}
             onClick={toggleUploadModal}
@@ -101,8 +102,8 @@ const DataFilesAddButton = ({ readOnly }) => {
             </span>
           </DropdownItem>
         </DropdownMenu>
-      </ButtonDropdown>
-    </div>
+      </Dropdown>
+    </>
   );
 };
 

diff --git a/client/src/components/DataFiles/DataFilesSidebar/DataFilesSidebar.module.scss b/client/src/components/DataFiles/DataFilesSidebar/DataFilesSidebar.module.scss
@@ -13,5 +13,5 @@
 
 .add-button-item {
   padding-bottom: 20px;
-  padding-left: 20px;
+  padding-inline: var(--global-space--section-left);
 }
diff --git a/client/src/components/DataFiles/DataFilesSidebar/DataFilesSidebar.scss b/client/src/components/DataFiles/DataFilesSidebar/DataFilesSidebar.scss
@@ -10,70 +10,29 @@
   border-radius: 0;
 }
 
-#add-button-wrapper {
-  padding-left: var(--horizontal-buffer);
-}
 #data-files-add {
-  width: 174px;
+  width: 100%;
 }
 
 .data-files-nav {
   padding-top: 20px;
 }
 
-/* Nested to prevent styles from affecting CMS header dropdown */
 .data-files-sidebar {
   .dropdown-menu {
-    border-color: var(--global-color-accent--normal);
-    border-radius: 0;
-    margin: 11px 3px 0;
-    width: 200px;
-    vertical-align: top;
-  }
-  .dropdown-menu::before {
-    position: absolute;
-    top: -10px;
-    left: 67px;
-    border-right: 10px solid transparent;
-    border-bottom: 10px solid var(--global-color-accent--normal);
-    border-left: 10px solid transparent;
-    margin-left: 20px;
-    content: '';
+    /* To make menu wider than button yet still centered */
+    width: calc(100% + var(--global-space--section-left));
+    left: calc(
+      var(--global-space--section-left) / -2
+    ) !important; /* to override `inset` from Popper via Reactstrap */
   }
+  .dropdown-menu::before,
   .dropdown-menu::after {
-    top: -9px;
-    left: 68px;
-    border-right: 9px solid transparent;
-    border-bottom: 9px solid #ffffff;
-    border-left: 9px solid transparent;
-    margin-left: 20px;
-    content: '';
-  }
-
-  .dropdown-item {
-    padding: 10px 6px;
-    color: var(--global-color-primary--x-dark);
-    font-size: 14px;
-    i {
-      padding-right: 19px;
-      font-size: 20px;
-      vertical-align: middle;
-    }
-    &:hover {
-      color: var(--global-color-primary--x-dark);
-    }
-  }
-  .dropdown-item:focus,
-  .dropdown-item:hover {
-    /* FAQ: Before FP-1083, value was #E6E0FB, which matched Design
-            and was `--global-color-accent` at 25% opacity on white…
-            which is what `--global-color-accent--weak` is now */
-    background-color: var(--global-color-accent--weak);
+    left: calc(50% - var(--arrow-size));
   }
 }
 
 .complex-dropdown-item {
-  border-top: 1px solid #707070;
   display: flex;
 }
 

diff --git a/client/src/styles/components/dropdown-menu.css b/client/src/styles/components/dropdown-menu.css
@@ -6,32 +6,34 @@ A menu of navigation elements.
 Styleguide Components.Dropdown
 */
 
-/* Nested to prevent styles from affecting CMS header dropdown */
-.workbench-wrapper {
+.workbench-wrapper /* nested so CMS header dropdown is unaffected */ {
   .dropdown-menu {
-    border-color: var(--global-color-accent--normal);
-    border-radius: 0;
-    margin-top: 11px;
-    padding: 0;
-    min-width: 200px;
-    width: auto;
-    vertical-align: top;
+    --border-width: 1px;
+    --border-color: var(--global-color-accent--normal);
+    --arrow-size: 10px;
+
+    margin-top: var(--arrow-size); /* to make space for arrow */
+
+    padding-block: unset; /* to undo Bootstrap */
+    border-radius: unset; /* to undo Bootstrap */
+
+    border: var(--border-width) solid var(--border-color);
   }
   .dropdown-menu::before,
   .dropdown-menu::after {
     position: absolute;
-    left: 65px;
-    border-right: 10px solid transparent;
-    border-bottom: 10px solid var(--global-color-accent--normal);
-    border-left: 10px solid transparent;
-    margin-left: 20px;
+
+    border-right: var(--arrow-size) solid transparent;
+    border-bottom: var(--arrow-size) solid var(--global-color-accent--normal);
+    border-left: var(--arrow-size) solid transparent;
+
     content: '';
   }
   .dropdown-menu::before {
-    top: -10px;
+    top: calc(var(--arrow-size) * -1);
   }
   .dropdown-menu::after {
-    top: -9px;
+    top: calc(( var(--arrow-size) - var(--border-width)) * -1);
     border-bottom-color: var(--global-color-primary--xx-light);
   }
 
@@ -50,9 +52,11 @@ Styleguide Components.Dropdown
   }
   .dropdown-item:focus,
   .dropdown-item:hover {
-    /* FAQ: Before FP-1083, value was #E6E0FB, which matched Design
-            and was `--global-color-accent` at 25% opacity on white…
-            which is what `--global-color-accent--weak` is now */
     background-color: var(--global-color-accent--weak);
   }
+
+  .dropdown-divider {
+    margin-block: 0.25em;
+    border-color: var(--global-color-primary--dark);
+  }
 }