fix(AppFrame): top bar changes and nav item interface update (#1374)

livechat · Oct 3, 2024 · 79063e4 · 79063e4
1 parent a11c336
commit 79063e4
Show file tree

Hide file tree

Showing 19 changed files with 223 additions and 128 deletions.
diff --git a/packages/react-components/src/components/Accordion/Accordion.spec.tsx b/packages/react-components/src/components/Accordion/Accordion.spec.tsx
@@ -41,7 +41,7 @@ describe('<Accordion> component', () => {
     expect(getByText('Content')).toBeInTheDocument();
   });
 
-  it('should call onClose and onOpen handlers on label click', () => {
+  it('should call onClose and onOpen handlers on label click', async () => {
     const onClose = vi.fn();
     const onOpen = vi.fn();
     const { getByRole } = renderComponent({
@@ -50,8 +50,14 @@ describe('<Accordion> component', () => {
       onOpen,
     });
 
+    expect(getByRole('button')).toHaveAttribute('aria-expanded', 'false');
     userEvent.click(getByRole('button'));
     expect(onOpen).toHaveBeenCalledTimes(1);
+
+    await waitFor(() => {
+      expect(getByRole('button')).toHaveAttribute('aria-expanded', 'false');
+    });
+
     userEvent.click(getByRole('button'));
     expect(onClose).toHaveBeenCalledTimes(1);
   });

diff --git a/packages/react-components/src/components/Accordion/Accordion.tsx b/packages/react-components/src/components/Accordion/Accordion.tsx
@@ -33,7 +33,7 @@ export const Accordion: React.FC<IAccordionProps> = ({
   const {
     isOpen: isExpanded,
     isMounted,
-    setIsOpen,
+    setShouldBeVisible,
   } = useAnimations({
     isVisible: currentlyOpen,
     elementRef: contentRef,
@@ -51,10 +51,10 @@ export const Accordion: React.FC<IAccordionProps> = ({
   const handleExpandChange = (isExpanded: boolean) => {
     if (isExpanded) {
       onClose?.();
-      !isControlled && setIsOpen(false);
+      !isControlled && setShouldBeVisible(false);
     } else {
       onOpen?.();
-      !isControlled && setIsOpen(true);
+      !isControlled && setShouldBeVisible(true);
     }
   };
 

diff --git a/...s/react-components/src/components/Accordion/components/AccordionAnimatedLabel.module.scss b/...s/react-components/src/components/Accordion/components/AccordionAnimatedLabel.module.scss
@@ -15,19 +15,7 @@ $base-class: 'accordion-animated-label';
     max-width: 100%;
 
     &--visible {
-      animation: change-visibility var(--transition-duration-fast-2);
-      animation-fill-mode: forwards;
+      opacity: 1;
     }
   }
 }
-
-@keyframes change-visibility {
-  0% {
-    opacity: 0;
-  }
-
-  70%,
-  100% {
-    opacity: 1;
-  }
-}
diff --git a/packages/react-components/src/components/AppFrame/AppFrame.mdx b/packages/react-components/src/components/AppFrame/AppFrame.mdx
@@ -309,6 +309,51 @@ return (
 
 ```
 
+#### How to build top bar alerts
+
+In general, alerts do not have height limits, which allows you to insert any amount of content. The container in which they are located also does not have such limits.
+
+However, it is important to remember to provide the best possible user experience, not to put too much information into the top bar, which could reduce the application window.
+
+Components layout changes depending on the browser window resolution:
+- `width > 1110px` - standard layout with the option of setting the kind for cta buttons
+- `width <= 1100px` - row layout, buttons are automatically changed to kind `text` (`link-inverted` for `warning` alert)
+- `width <= 705px` - mobile version (no radiuses)
+
+It is also important to remember that you should not display a large number of alerts. For this purpose, we suggest a simple solution, consisting in hiding and showing only the currently expected alert (the decision is up to the project which alert has the highest priority)
+
+##### Example
+
+```jsx
+import { NavigationTopBarAlert } from '@livechat/design-system-react-components';
+
+const [visibleAlert, setVisibleAlert] = React.useState(null);
+
+return (
+  <NavigationTopBar>
+    <NavigationTopBarAlert
+      kind="info"
+      isVisible={visibleAlert === 0}
+    >
+      Info
+    </NavigationTopBarAlert>
+    <NavigationTopBarAlert
+      kind="warning"
+      isVisible={visibleAlert === 1}
+    >
+      Warning
+    </NavigationTopBarAlert>
+    <NavigationTopBarAlert
+      kind="error"
+      isVisible={visibleAlert === 2}
+    >
+      Error
+    </NavigationTopBarAlert>
+  </NavigationTopBar>
+);
+
+```
+
 ## Component API <a id="ComponentAPI" />
 
 <ArgTypes of={AppFrameStories.Default} sort="requiredFirst" />
diff --git a/packages/react-components/src/components/AppFrame/AppFrame.module.scss b/packages/react-components/src/components/AppFrame/AppFrame.module.scss
@@ -16,6 +16,7 @@ $base-class: 'app-frame';
     padding-bottom: 6px;
     width: 100%;
     height: 100%;
+    overflow: hidden;
 
     &--mobile {
       padding: 0;

diff --git a/packages/react-components/src/components/AppFrame/AppFrame.stories.tsx b/packages/react-components/src/components/AppFrame/AppFrame.stories.tsx
@@ -89,9 +89,7 @@ export const Default = (): React.ReactElement => {
   const [activeItem, setActiveItem] = React.useState('archives');
   const [activeSubItem, setActiveSubItem] = React.useState(0);
   const [topBarVisible, setTopBarVisible] = React.useState(true);
-  const [visibleAlerts, setVisibleAlerts] = React.useState<boolean[]>(
-    Array(3).fill(false)
-  );
+  const [visibleAlert, setVisibleAlert] = React.useState<number | null>(null);
 
   const { products } = useProductSwitcher({
     env: 'labs',
@@ -228,21 +226,22 @@ export const Default = (): React.ReactElement => {
       }
       sideNavigation={getSubNav()}
       topBar={
-        topBarVisible || visibleAlerts.some((alert) => alert) ? (
+        topBarVisible ? (
           <ExampleTopBar
             topBarVisible={topBarVisible}
-            visibleAlerts={visibleAlerts}
-            setAlerts={setVisibleAlerts}
+            visibleAlert={visibleAlert}
+            setVisibleAlert={setVisibleAlert}
           />
         ) : null
       }
     >
       <ExampleAppContent
         showToggle={SectionsWithToggle.includes(activeItem)}
-        alerts={visibleAlerts}
-        setAlerts={setVisibleAlerts}
+        alerts={Array(4).fill(null)}
         topBarVisible={topBarVisible}
         setTopBarVisible={setTopBarVisible}
+        visibleAlert={visibleAlert}
+        setVisibleAlert={setVisibleAlert}
       />
     </AppFrame>
   );

diff --git a/packages/react-components/src/components/AppFrame/AppFrame.tsx b/packages/react-components/src/components/AppFrame/AppFrame.tsx
@@ -5,6 +5,7 @@ import cx from 'clsx';
 import { useAnimations, useMobileViewDetector } from '../../hooks';
 import { AppFrameProvider, useAppFrame } from '../../providers';
 
+import { MOBILE_BREAKPOINT } from './constants';
 import { IAppFrameProps } from './types';
 
 import styles from './AppFrame.module.scss';
@@ -23,7 +24,6 @@ const Frame = (props: IAppFrameProps) => {
     topBarClassName,
     sideNavigationContainerClassName,
     contentClassName,
-    mobileViewBreakpoint = 705,
   } = props;
   const mergedClassNames = cx(styles[baseClass], className);
   const {
@@ -37,7 +37,7 @@ const Frame = (props: IAppFrameProps) => {
     elementRef: sideNavWrapperRef,
   });
   const { isMobile, handleResizeRef } = useMobileViewDetector({
-    mobileBreakpoint: mobileViewBreakpoint,
+    mobileBreakpoint: MOBILE_BREAKPOINT,
   });
 
   React.useEffect(() => {
@@ -115,7 +115,7 @@ const Frame = (props: IAppFrameProps) => {
               >
                 {topBar}
               </div>
-              <div>{mobileNavigation}</div>
+              {mobileNavigation}
             </>
           )}
         </div>
@@ -124,8 +124,12 @@ const Frame = (props: IAppFrameProps) => {
   );
 };
 
-export const AppFrame: React.FC<IAppFrameProps> = (props) => (
-  <AppFrameProvider>
-    <Frame {...props} />
-  </AppFrameProvider>
-);
+export const AppFrame: React.FC<IAppFrameProps> = (props) => {
+  const { isSideNavigationVisible, ...restProps } = props;
+
+  return (
+    <AppFrameProvider isSideNavigationVisible={isSideNavigationVisible}>
+      <Frame {...restProps} />
+    </AppFrameProvider>
+  );
+};
diff --git a/...mponents/src/components/AppFrame/components/MobileNavigation/MobileNavigation.module.scss b/...mponents/src/components/AppFrame/components/MobileNavigation/MobileNavigation.module.scss
@@ -6,7 +6,6 @@ $base-class: 'mobile-navigation';
   flex-shrink: 0;
   align-items: center;
   justify-content: space-around;
-  z-index: 1;
   background-color: var(--navbar-background);
   padding: 6px;
 }
diff --git a/packages/react-components/src/components/AppFrame/components/NavigationItem/types.ts b/packages/react-components/src/components/AppFrame/components/NavigationItem/types.ts
@@ -2,7 +2,9 @@ import * as React from 'react';
 
 import { ComponentCoreProps } from '../../../../utils/types';
 
-export interface INavigationItemProps extends ComponentCoreProps {
+export interface INavigationItemProps
+  extends ComponentCoreProps,
+    Omit<React.AnchorHTMLAttributes<HTMLAnchorElement>, 'onClick'> {
   /**
    * The ID of the item
    */

diff --git a/...mponents/src/components/AppFrame/components/NavigationTopBar/NavigationTopBar.module.scss b/...mponents/src/components/AppFrame/components/NavigationTopBar/NavigationTopBar.module.scss
@@ -1,24 +1,31 @@
 /* stylelint-disable media-query-no-invalid */
 $base-class: 'navigation-top-bar';
-$mobile-breakpoint: 768px;
+$alerts-mobile-breakpoint: 810px;
+$mobile-breakpoint: 705px;
+$alert-size: 42px;
+$mobile-alert-size: 70px;
 
 .#{$base-class} {
   display: flex;
   position: relative;
   flex-direction: column;
-  gap: var(--spacing-1);
   width: 100%;
   overflow: hidden;
 
   &__title {
     display: flex;
     align-items: center;
     justify-content: center;
-    padding: 10px 0;
+    height: 36px;
+  }
+
+  &__alerts-wrapper {
+    overflow: hidden;
   }
 
   &__alert {
     display: flex;
+    position: relative;
     align-items: center;
     justify-content: space-evenly;
     transition: all var(--transition-duration-moderate-1) ease-in-out;
@@ -34,7 +41,11 @@ $mobile-breakpoint: 768px;
 
     &--open {
       padding-block: 5px;
-      min-height: 42px;
+      min-height: $alert-size;
+
+      @media screen and (width <=$mobile-breakpoint) {
+        min-height: $mobile-alert-size;
+      }
     }
 
     &__wrapper {
@@ -75,9 +86,9 @@ $mobile-breakpoint: 768px;
       justify-content: center;
       width: calc(100% - 62px);
 
-      @media screen and (width <=$mobile-breakpoint) {
+      @media screen and (width <=$alerts-mobile-breakpoint) {
         flex-direction: column;
-        gap: var(--spacing-2);
+        gap: 0;
       }
     }
 
@@ -92,10 +103,11 @@ $mobile-breakpoint: 768px;
       padding: 0;
       color: inherit;
 
-      @media screen and (width <=$mobile-breakpoint) {
+      @media screen and (width <=$alerts-mobile-breakpoint) {
         position: absolute;
-        top: var(--spacing-3);
+        top: auto;
         right: var(--spacing-3);
+        bottom: auto;
       }
     }
 
@@ -106,16 +118,19 @@ $mobile-breakpoint: 768px;
       align-items: center;
       justify-content: center;
 
-      @media screen and (width <=$mobile-breakpoint) {
+      @media screen and (width <=$alerts-mobile-breakpoint) {
         flex-shrink: 1;
         flex-wrap: wrap;
       }
     }
 
-    @media screen and (width <=$mobile-breakpoint) {
-      border-radius: 0;
+    @media screen and (width <=$alerts-mobile-breakpoint) {
       padding-inline: var(--spacing-10);
       text-align: center;
     }
+
+    @media screen and (width <=$mobile-breakpoint) {
+      border-radius: 0;
+    }
   }
 }