chore: Revert Flashbar fixes (#787)

cloudscape-design · Feb 24, 2023 · 40f40f5 · 40f40f5
1 parent bc541ba
commit 40f40f5
Show file tree

Hide file tree

Showing 9 changed files with 97 additions and 290 deletions.
diff --git a/pages/app-layout/with-stacked-notifications-and-no-paddings.page.tsx b/pages/app-layout/with-stacked-notifications-and-no-paddings.page.tsx
@@ -0,0 +1,61 @@
+// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+import React from 'react';
+import AppLayout from '~components/app-layout';
+import ScreenshotArea from '../utils/screenshot-area';
+import labels from './utils/labels';
+import Flashbar from '~components/flashbar';
+import Table from '~components/table';
+import Header from '~components/header';
+
+export default function () {
+  return (
+    <ScreenshotArea gutters={false}>
+      <AppLayout
+        ariaLabels={labels}
+        notifications={
+          <Flashbar
+            items={[
+              {
+                type: 'success',
+                header: 'Success message',
+                statusIconAriaLabel: 'success',
+                dismissLabel: 'Dismiss notification',
+                dismissible: true,
+                onDismiss: () => void 0,
+              },
+              {
+                type: 'warning',
+                header: 'Warning message',
+                statusIconAriaLabel: 'warning',
+                dismissLabel: 'Dismiss notification',
+                dismissible: true,
+                onDismiss: () => void 0,
+              },
+            ]}
+            stackItems={true}
+            i18nStrings={{
+              ariaLabel: 'Notifications',
+              notificationBarText: 'Notifications',
+              notificationBarAriaLabel: 'View all notifications',
+              errorIconAriaLabel: 'Error',
+              successIconAriaLabel: 'Success',
+              warningIconAriaLabel: 'Warning',
+              infoIconAriaLabel: 'Information',
+              inProgressIconAriaLabel: 'In progress',
+            }}
+          />
+        }
+        disableContentPaddings={true}
+        content={
+          <Table
+            header={<Header variant="h1">Sticky Table Header</Header>}
+            items={[]}
+            columnDefinitions={[]}
+            stickyHeader={true}
+          />
+        }
+      />
+    </ScreenshotArea>
+  );
+}
diff --git a/pages/app-layout/with-stacked-notifications-and-table.page.tsx b/pages/app-layout/with-stacked-notifications-and-table.page.tsx
diff --git a/src/app-layout/index.tsx b/src/app-layout/index.tsx
@@ -453,7 +453,6 @@ const OldAppLayout = React.forwardRef(
               >
                 {notifications && (
                   <Notifications
-                    disableContentPaddings={disableContentPaddings}
                     testUtilsClassName={testutilStyles.notifications}
                     labels={ariaLabels}
                     topOffset={disableBodyScroll ? 0 : headerHeight}

diff --git a/src/app-layout/notifications/index.tsx b/src/app-layout/notifications/index.tsx
@@ -10,33 +10,24 @@ interface NotificationsProps {
   children?: React.ReactNode;
   labels: AppLayoutProps.Labels | undefined;
   topOffset: number | undefined;
-  disableContentPaddings?: boolean;
 }
 interface NotificationWrapperProps extends NotificationsProps {
   sticky: boolean | undefined;
 }
 
 export const Notifications = React.forwardRef(
-  ({ sticky, disableContentPaddings, ...props }: NotificationWrapperProps, ref: React.Ref<HTMLDivElement>) => {
+  ({ sticky, ...props }: NotificationWrapperProps, ref: React.Ref<HTMLDivElement>) => {
     return sticky ? (
       <div ref={ref} className={styles['notifications-sticky']} style={{ top: props.topOffset }}>
-        <div
-          role="region"
-          className={clsx(props.testUtilsClassName, disableContentPaddings && styles['no-content-paddings'])}
-          aria-label={props.labels?.notifications}
-        >
+        <div role="region" className={props.testUtilsClassName} aria-label={props.labels?.notifications}>
           {props.children}
         </div>
       </div>
     ) : (
       <div
         role="region"
         ref={ref}
-        className={clsx(
-          props.testUtilsClassName,
-          styles.notifications,
-          disableContentPaddings && styles['no-content-paddings']
-        )}
+        className={clsx(props.testUtilsClassName, styles.notifications)}
         aria-label={props.labels?.notifications}
       >
         {props.children}

diff --git a/src/app-layout/notifications/styles.scss b/src/app-layout/notifications/styles.scss
@@ -21,11 +21,3 @@
   position: sticky;
   #{custom-props.$flashbarStickyBottomMargin}: #{awsui.$space-xxl};
 }
-
-.no-content-paddings {
-  /*
-  When using the disableContentPaddings option, the Flashbar will use this custom property to add additional space
-  when the notification bar is rendered, to prevent it from overlapping the content.
-   */
-  #{custom-props.$stackedNotificationsBottomMargin}: #{awsui.$space-scaled-l};
-}
diff --git a/src/app-layout/visual-refresh/notifications.tsx b/src/app-layout/visual-refresh/notifications.tsx
@@ -25,11 +25,6 @@ export default function Notifications() {
     return null;
   }
 
-  /*
-  The notificationsElement ref is assigned to an inner div to prevent internal bottom margin from affecting the
-  calculated height, which is used for sticky elements below.
-   */
-
   return (
     <div
       role="region"
@@ -44,8 +39,9 @@ export default function Notifications() {
         testutilStyles.notifications,
         'awsui-context-content-header'
       )}
+      ref={notificationsElement}
     >
-      <div ref={notificationsElement}>{notifications}</div>
+      {notifications}
     </div>
   );
 }
diff --git a/src/flashbar/collapsible-flashbar.tsx b/src/flashbar/collapsible-flashbar.tsx
@@ -20,7 +20,6 @@ import { sendToggleMetric } from './internal/analytics';
 import { useFlashbar } from './common';
 import { throttle } from '../internal/utils/throttle';
 import { scrollElementIntoView } from '../internal/utils/scrollable-containers';
-import { findUpUntil } from '../internal/utils/dom';
 
 export { FlashbarProps };
 
@@ -113,7 +112,7 @@ export default function CollapsibleFlashbar({ items, ...restProps }: FlashbarPro
           const windowHeight = window.innerHeight;
           // Take the parent region into account if using the App Layout, because it might have additional margins.
           // Otherwise we use the Flashbar component for this calculation.
-          const outerElement = findUpUntil(flashbar, element => element.getAttribute('role') === 'region') || flashbar;
+          const outerElement = flashbar.parentElement?.parentElement || flashbar;
           const applySpacing =
             isFlashbarStackExpanded && Math.ceil(outerElement.getBoundingClientRect().bottom) >= windowHeight;
           if (!applySpacing) {
@@ -286,10 +285,7 @@ export default function CollapsibleFlashbar({ items, ...restProps }: FlashbarPro
         styles.flashbar,
         styles[`breakpoint-${breakpoint}`],
         styles.stack,
-        isCollapsible && styles.collapsible,
-        items.length === 2 && styles['short-list'],
         isFlashbarStackExpanded && styles.expanded,
-        isVisualRefresh && styles['visual-refresh'],
         getVisualContextClassname('flashbar')
       )}
       ref={mergedRef}
@@ -302,8 +298,7 @@ export default function CollapsibleFlashbar({ items, ...restProps }: FlashbarPro
               styles['notification-bar'],
               isVisualRefresh && styles['visual-refresh'],
               isFlashbarStackExpanded ? styles.expanded : styles.collapsed,
-              transitioning && styles['animation-running'],
-              items.length === 2 && styles['short-list']
+              transitioning && styles['animation-running']
             )}
             onClick={toggleCollapseExpand}
             ref={notificationBarRef}