fix: styling for menu and banner (#51)

* fix: styling for menu and banner * Fix: small design issues * exposed dialogPosition and dialogOffset in MenuButton.jsx * fix scss for SingleValue.scss * Fix scss for Banner.jsx
mondaycom · Jan 13, 2021 · 6c52fd1 · 6c52fd1
1 parent 6edbd97
commit 6c52fd1
Show file tree

Hide file tree

Showing 10 changed files with 130 additions and 35 deletions.
diff --git a/src/components/Banner/Banner.jsx b/src/components/Banner/Banner.jsx
@@ -14,6 +14,7 @@ const Banner = forwardRef(
   (
     {
       className,
+      ariaLabel,
       imageAlt,
       imageSrc,
       renderTitle,
@@ -64,7 +65,7 @@ const Banner = forwardRef(
     }, [onClose]);
 
     return (
-      <aside ref={mergedRef} className={cx("banner", className, { rtl })}>
+      <aside ref={mergedRef} className={cx(className, "banner", { rtl })} aria-label={ariaLabel}>
         <div
           className={cx("banner--content", `image-position__${imagePosition}`, {
             "close-button-spacing": !!renderCloseButton
@@ -131,7 +132,11 @@ Banner.propTypes = {
   /**
    * Change to "Right to Left" if set to `true`. Defaults to "Left to Right"
    */
-  rtl: PropTypes.bool
+  rtl: PropTypes.bool,
+  /**
+   * Set the banner's aria label
+   */
+  ariaLabel: PropTypes.string
 };
 
 Banner.defaultProps = {
@@ -145,7 +150,8 @@ Banner.defaultProps = {
   subtitle: "",
   imageClassName: "",
   rtl: false,
-  onClose: null
+  onClose: null,
+  ariaLabel: "Banner"
 };
 
 export default Banner;
diff --git a/src/components/Banner/Banner.scss b/src/components/Banner/Banner.scss
@@ -10,7 +10,8 @@
     padding: $spacing-large;
     border-radius: $border-radius-medium;
     grid-gap: $spacing-small;
-    justify-content: center;
+    justify-content: start;
+    column-gap: 24px;
     align-content: center;
     @include theme-prop(background-color, dark-background-on-secondary-color);
     @include theme-prop(color, primary-text-color);
@@ -77,12 +78,9 @@
     }
 
     .banner--subtitle {
+      @include font-paragraph();
       margin: 0;
       grid-area: subtitle;
-      font-size: 18px;
-      font-weight: 400;
-      line-height: 24px;
-      max-width: 782px;
       display: -webkit-box;
       -webkit-line-clamp: 3;
       -webkit-box-orient: vertical;

diff --git a/src/components/Banner/__tests__/__snapshots__/banner.jest.js.snap b/src/components/Banner/__tests__/__snapshots__/banner.jest.js.snap
@@ -2,7 +2,8 @@
 
 exports[`Banner should render close button 1`] = `
 <aside
-  className="banner mock-classname"
+  aria-label="banner"
+  className="mock-classname banner"
 >
   <div
     className="banner--content image-position__left close-button-spacing"
@@ -32,6 +33,7 @@ exports[`Banner should render close button 1`] = `
 
 exports[`Banner should render correctly without props 1`] = `
 <aside
+  aria-label="banner"
   className="banner"
 >
   <div
@@ -42,7 +44,8 @@ exports[`Banner should render correctly without props 1`] = `
 
 exports[`Banner should render in Right to Left mode 1`] = `
 <aside
-  className="banner mock-classname rtl"
+  aria-label="banner"
+  className="mock-classname banner rtl"
 >
   <div
     className="banner--content image-position__left close-button-spacing"
@@ -72,7 +75,8 @@ exports[`Banner should render in Right to Left mode 1`] = `
 
 exports[`Banner should render title, subtitle and image 1`] = `
 <aside
-  className="banner mock-classname"
+  aria-label="banner"
+  className="mock-classname banner"
 >
   <div
     className="banner--content image-position__left"
@@ -98,6 +102,7 @@ exports[`Banner should render title, subtitle and image 1`] = `
 
 exports[`Banner should render with correct image position class - bottom 1`] = `
 <aside
+  aria-label="banner"
   className="banner"
 >
   <div
@@ -108,6 +113,7 @@ exports[`Banner should render with correct image position class - bottom 1`] = `
 
 exports[`Banner should render with correct image position class - left 1`] = `
 <aside
+  aria-label="banner"
   className="banner"
 >
   <div
@@ -118,6 +124,7 @@ exports[`Banner should render with correct image position class - left 1`] = `
 
 exports[`Banner should render with correct image position class - right 1`] = `
 <aside
+  aria-label="banner"
   className="banner"
 >
   <div
@@ -128,6 +135,7 @@ exports[`Banner should render with correct image position class - right 1`] = `
 
 exports[`Banner should render with correct image position class - top 1`] = `
 <aside
+  aria-label="banner"
   className="banner"
 >
   <div
@@ -138,6 +146,7 @@ exports[`Banner should render with correct image position class - top 1`] = `
 
 exports[`Banner should use custom render function for subtitle 1`] = `
 <aside
+  aria-label="banner"
   className="banner"
 >
   <div
@@ -163,6 +172,7 @@ exports[`Banner should use custom render function for subtitle 1`] = `
 
 exports[`Banner should use custom render function for title 1`] = `
 <aside
+  aria-label="banner"
   className="banner"
 >
   <div

diff --git a/src/components/DialogContentContainer/DialogContentContainer.scss b/src/components/DialogContentContainer/DialogContentContainer.scss
@@ -3,6 +3,10 @@
 @import "../../styles/global-css-settings.scss";
 
 .dialog-content-container {
+  &:focus {
+    outline: none;
+  }
+
   &--small {
     padding: $spacing-small;
   }

diff --git a/src/components/Dropdown/__stories__/dropdown.stories.js b/src/components/Dropdown/__stories__/dropdown.stories.js
@@ -9,6 +9,7 @@ import DescriptionLabel from "../../storybook-helpers/description-label/descript
 import "./dropdown.stories.scss";
 
 const mockColorOptions = [
+  { value: "English", label: "English", isFixed: true },
   { value: "ocean", label: "Ocean", isFixed: true },
   { value: "blue", label: "Blue", isDisabled: true },
   { value: "purple", label: "Purple" },
@@ -21,6 +22,7 @@ const mockVirtualizedOptions = new Array(10000).fill(null).map((_, i) => ({ valu
 
 export const Sandbox = () => {
   const mockColorOptions = [
+    { value: "English", label: "English", isFixed: true },
     { value: "ocean", label: "Ocean", isFixed: true },
     { value: "blue", label: "Blue", isDisabled: true },
     { value: "purple", label: "Purple" },

diff --git a/src/components/Dropdown/components/SingleValue/SingleValue.scss b/src/components/Dropdown/components/SingleValue/SingleValue.scss
@@ -1,3 +1,2 @@
 .dropdown-wrapper__single-value--reset {
-  height: auto;
 }
diff --git a/src/components/Menu/Menu/Menu.scss b/src/components/Menu/Menu/Menu.scss
@@ -2,6 +2,10 @@
 @import "../../../styles/typography.scss";
 
 .monday-style-menu {
+  &:focus {
+    outline: none;
+  }
+
   &--small {
     width: 200px;
   }

diff --git a/src/components/MenuButton/MenuButton.jsx b/src/components/MenuButton/MenuButton.jsx
@@ -15,14 +15,16 @@ const MOVE_BY = { main: 0, secondary: -6 };
 
 const MenuButton = ({
   componentClassName,
-                      openDialogComponentClassName,
+  openDialogComponentClassName,
   children,
   component,
   size,
   open,
   zIndex,
   ariaLabel,
   closeDialogOnContentClick,
+  dialogOffset,
+  dialogPosition,
   dialogClassName,
   dialogPaddingSize
 }) => {
@@ -51,17 +53,25 @@ const MenuButton = ({
     );
   }, [children, dialogPaddingSize]);
 
+  const computedDialogOffset = useMemo(
+    () => ({
+      ...MOVE_BY,
+      ...dialogOffset
+    }),
+    [dialogOffset]
+  );
+
   const Icon = component;
   const iconSize = size - 4;
 
   return (
     <Dialog
       wrapperClassName={dialogClassName}
-      position="bottom-start"
+      position={dialogPosition}
       startingEdge="bottom"
       animationType="expand"
       content={content}
-      moveBy={MOVE_BY}
+      moveBy={computedDialogOffset}
       showTrigger={showTrigger}
       hideTrigger={hideTrigger}
       onDialogDidShow={onDialogDidShow}
@@ -74,7 +84,7 @@ const MenuButton = ({
         role="menu"
         className={cx("menu-button--wrapper", componentClassName, BEMClass(`size-${size}`), {
           [BEMClass("open")]: isOpen,
-          [openDialogComponentClassName]: isOpen
+          [openDialogComponentClassName]: isOpen && openDialogComponentClassName
         })}
         aria-haspopup="true"
         aria-expanded={isOpen}
@@ -94,8 +104,15 @@ const MenuButtonSizes = {
   LARGE: "48"
 };
 
+const DialogPositions = {
+  BOTTOM: "bottom",
+  BOTTOM_START: "bottom-start",
+  BOTTOM_END: "bottom-end"
+};
+
 MenuButton.sizes = MenuButtonSizes;
 MenuButton.paddingSizes = DialogContentContainer.sizes;
+MenuButton.dialogPositions = DialogPositions;
 
 MenuButton.propTypes = {
   componentClassName: PropTypes.string,
@@ -122,11 +139,23 @@ MenuButton.propTypes = {
     Class name to provide the element which wraps the popover/modal/dialog
    */
   dialogClassName: PropTypes.string,
+  /**
+   * main - `dialogOffset.main` - main axis offset; `dialogOffset.secondary` secondary axis offset
+   */
+  dialogOffset: PropTypes.shape({
+    main: PropTypes.number,
+    secondary: PropTypes.number
+  }),
   dialogPaddingSize: PropTypes.oneOf([
     MenuButton.paddingSizes.NONE,
     MenuButton.paddingSizes.SMALL,
     MenuButton.paddingSizes.MEDIUM,
     MenuButton.paddingSizes.LARGE
+  ]),
+  dialogPosition: PropTypes.oneOf([
+    MenuButton.dialogPositions.BOTTOM_START,
+    MenuButton.dialogPositions.BOTTOM,
+    MenuButton.dialogPositions.BOTTOM_END
   ])
 };
 MenuButton.defaultProps = {
@@ -139,7 +168,9 @@ MenuButton.defaultProps = {
   closeDialogOnContentClick: false,
   dialogClassName: "",
   openDialogComponentClassName: "",
-  dialogPaddingSize: DialogContentContainer.sizes.MEDIUM
+  dialogOffset: MOVE_BY,
+  dialogPaddingSize: DialogContentContainer.sizes.MEDIUM,
+  dialogPosition: MenuButton.dialogPositions.BOTTOM_START
 };
 
 export default MenuButton;
diff --git a/src/components/MenuButton/__stories__/menuButton.stories.js b/src/components/MenuButton/__stories__/menuButton.stories.js
@@ -1,8 +1,8 @@
 import React from "react";
-import { select, boolean } from "@storybook/addon-knobs";
+import { select, boolean, number } from "@storybook/addon-knobs";
 import { withPerformance } from "storybook-addon-performance";
 import MenuButton from "../MenuButton";
-import { ComponentStateDescription, FlexLayout } from "../../storybook-helpers";
+import { ComponentStateDescription, FlexLayout, StoryStateColumn, StoryStateRow } from "../../storybook-helpers";
 import DropdownChevronDown from "../../Icon/Icons/components/DropdownChevronDown";
 import "./menuButton.style.scss";
 import { Menu, MenuItem, MenuTitle } from "../../index";
@@ -13,22 +13,33 @@ function MenuButtonContent() {
 }
 
 export const Sandbox = () => (
-  <div>
-    <MenuButton
-      id="Knobs"
-      size={select("Size", MenuButton.sizes, MenuButton.sizes.MEDIUM)}
-      closeDialogOnContentClick={boolean("Close Dialog On Content Click", false)}
-      dialogPaddingSize={select("Dialog Padding Size", MenuButton.paddingSizes, MenuButton.paddingSizes.MEDIUM)}
-      ariaLabel="Default menu icon"
-    >
-      <Menu id="menu" size={Menu.sizes.MEDIUM}>
-        <MenuTitle caption="Look up, you might see" captionPosition={MenuTitle.positions.TOP} />
-        <MenuItem icon={Sun} iconType={MenuItem.iconType.SVG} title="The sun" />
-        <MenuItem icon={Moon} iconType={MenuItem.iconType.SVG} title="The moon" />
-        <MenuItem icon={Favorite} iconType={MenuItem.iconType.SVG} title="And the stars" />
-      </Menu>
-    </MenuButton>
-  </div>
+  <StoryStateRow>
+    <StoryStateColumn centerize>
+      <MenuButton
+        id="Knobs"
+        size={select("Size", MenuButton.sizes, MenuButton.sizes.MEDIUM)}
+        closeDialogOnContentClick={boolean("Close Dialog On Content Click", false)}
+        dialogPaddingSize={select("Dialog Padding Size", MenuButton.paddingSizes, MenuButton.paddingSizes.MEDIUM)}
+        dialogPosition={select(
+          "Dialog Opening Position",
+          MenuButton.dialogPositions,
+          MenuButton.dialogPositions.BOTTOM_START
+        )}
+        dialogOffset={{
+          main: number("Dialog offset in main axis", 0),
+          secondary: number("Dialog offset in secondary axis", 0)
+        }}
+        ariaLabel="Default menu icon"
+      >
+        <Menu id="menu" size={Menu.sizes.MEDIUM}>
+          <MenuTitle caption="Look up, you might see" captionPosition={MenuTitle.positions.TOP} />
+          <MenuItem icon={Sun} iconType={MenuItem.iconType.SVG} title="The sun" />
+          <MenuItem icon={Moon} iconType={MenuItem.iconType.SVG} title="The moon" />
+          <MenuItem icon={Favorite} iconType={MenuItem.iconType.SVG} title="And the stars" />
+        </Menu>
+      </MenuButton>
+    </StoryStateColumn>
+  </StoryStateRow>
 );
 
 export const DifferentIcon = () => (
@@ -74,6 +85,32 @@ export const Sizes = () => (
   </FlexLayout>
 );
 
+export const Positions = () => (
+  <section>
+    <StoryStateRow>
+      <StoryStateColumn centerize title={"Bottom Start"}>
+        <MenuButton dialogPosition={MenuButton.dialogPositions.BOTTOM_START} ariaLabel={"Bottom Start"}>
+          <MenuButtonContent />
+        </MenuButton>
+      </StoryStateColumn>
+    </StoryStateRow>
+    <StoryStateRow>
+      <StoryStateColumn centerize title={"Bottom"}>
+        <MenuButton dialogPosition={MenuButton.dialogPositions.BOTTOM} ariaLabel={"Bottom"}>
+          <MenuButtonContent />
+        </MenuButton>
+      </StoryStateColumn>
+    </StoryStateRow>
+    <StoryStateRow>
+      <StoryStateColumn centerize title={"Bottom End"}>
+        <MenuButton dialogPosition={MenuButton.dialogPositions.BOTTOM_END} ariaLabel={"Bottom End"}>
+          <MenuButtonContent />
+        </MenuButton>
+      </StoryStateColumn>
+    </StoryStateRow>
+  </section>
+);
+
 export default {
   title: "Components|MenuButton",
   component: MenuButton,