From ef37f073dfae4e04b2d595083fa35d2e566d6057 Mon Sep 17 00:00:00 2001 From: Sophie Schneider Date: Wed, 16 Aug 2023 15:59:08 -0400 Subject: [PATCH] [Banner] Consolidate se23 logic and styles (#10001) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ### WHY are these changes introduced? Fixes https://github.com/Shopify/polaris/issues/9919 ### WHAT is this pull request doing? * Removes se23 conditional logic for `Banner.tsx` * Consolidates se23 conditional styles for `Banner.scss` * Removes unused styles * Merges `BannerExperimental` with main Banner ### How to 🎩 Compare production and this PR's chromatic storybook to make sure styles are the same [Production](https://storybook.polaris.shopify.com/?path=/story/all-components-banner--all&globals=polarisSummerEditions2023:true) [This PR](https://5d559397bae39100201eedc1-cwkvegwhxb.chromatic.com/?path=/story/all-components-banner--all) --- .../src/components/Banner/Banner.scss | 314 ++---------- .../src/components/Banner/Banner.tsx | 472 +++++++++-------- .../BannerExperimental.scss | 46 -- .../BannerExperimental/BannerExperimental.tsx | 269 ---------- .../components/BannerExperimental/index.ts | 1 - .../src/components/Banner/components/index.ts | 1 - polaris-react/src/components/Banner/index.ts | 1 + .../components/Banner/tests/Banner.test.tsx | 479 +----------------- .../BannerExperimental => }/utilities.ts | 46 +- 9 files changed, 360 insertions(+), 1269 deletions(-) delete mode 100644 polaris-react/src/components/Banner/components/BannerExperimental/BannerExperimental.scss delete mode 100644 polaris-react/src/components/Banner/components/BannerExperimental/BannerExperimental.tsx delete mode 100644 polaris-react/src/components/Banner/components/BannerExperimental/index.ts delete mode 100644 polaris-react/src/components/Banner/components/index.ts rename polaris-react/src/components/Banner/{components/BannerExperimental => }/utilities.ts (64%) diff --git a/polaris-react/src/components/Banner/Banner.scss b/polaris-react/src/components/Banner/Banner.scss index 94e46564e4b..6326cd4ffc4 100644 --- a/polaris-react/src/components/Banner/Banner.scss +++ b/polaris-react/src/components/Banner/Banner.scss @@ -1,50 +1,9 @@ @import '../../styles/common'; -@mixin banner-variants($in-page) { - border: var(--p-border-width-1) solid var(--p-color-border-strong); - background-color: var(--p-color-bg-app); - - #{$se23} & { - border: none; - background-color: var(--p-color-bg); - } - - @if $in-page { - border-radius: 0; - border-width: var(--p-border-width-1) 0; - - #{$se23} & { - @include shadow-bevel( - $boxShadow: var(--p-shadow-sm), - $borderRadius: var(--p-border-radius-0-experimental), - // The following arguments explicitly ignore the shadow-bevel opt out - $border: none, - $content: '' - ); - } - - @media #{$p-breakpoints-sm-up} { - border-width: var(--p-border-width-1); - border-radius: var(--p-border-radius-2); - - #{$se23} & { - @include shadow-bevel( - $boxShadow: var(--p-shadow-sm), - $borderRadius: var(--p-border-radius-3), - // The following arguments explicitly ignore the shadow-bevel opt out - $border: none, - $content: '' - ); - } - } - } @else { - border-radius: var(--p-border-radius-1); - - #{$se23} & { - box-shadow: none; - border-radius: var(--p-border-radius-2); - } - } +.Banner { + background-color: var(--p-color-bg); + position: relative; + display: flex; &:focus { outline: none; @@ -54,256 +13,79 @@ outline: var(--p-border-width-2) solid var(--p-color-border-interactive-focus); } - - &.statusSuccess { - border-color: var(--p-color-border-success-subdued); - background-color: var(--p-color-bg-success-subdued); - } - - &.statusInfo { - border-color: var(--p-color-border-info-subdued); - background-color: var(--p-color-bg-info-subdued); - } - - &.statusWarning { - border-color: var(--p-color-border-caution-subdued); - background-color: var(--p-color-bg-caution-subdued); - } - - &.statusCritical { - border-color: var(--p-color-border-critical-subdued); - background-color: var(--p-color-bg-critical-subdued); - } -} - -.Banner { - // stylelint-disable -- Polaris component custom properties - --pc-banner-secondary-action-horizontal-padding: var(--p-space-3); - --pc-banner-secondary-action-vertical-padding: var(--p-space-2); - // stylelint-enable - position: relative; - display: flex; - - // stylelint-disable selector-max-class, selector-max-specificity -- generated by polaris-migrator DO NOT COPY - &.statusCritical .PrimaryAction.Button { - border-color: var(--p-color-border-critical-subdued); - - &:hover { - border-color: var(--p-color-border-critical-subdued); - background: var(--p-color-bg-critical-subdued-hover); - } - - &:active { - border-color: var(--p-color-border-critical-subdued); - background: var(--p-color-bg-critical-subdued-active); - } - - &:focus:not(:active) { - border-color: var(--p-color-border-critical-subdued); - background: var(--p-color-bg-critical-subdued); - } - } - - &.statusWarning .PrimaryAction.Button { - border-color: var(--p-color-border-caution-subdued); - - &:hover { - border-color: var(--p-color-border-caution-subdued); - background: var(--p-color-bg-caution-subdued-hover); - } - - &:active { - border-color: var(--p-color-border-caution-subdued); - background: var(--p-color-bg-caution-subdued-active); - } - - &:focus:not(:active) { - border-color: var(--p-color-border-caution-subdued); - background: var(--p-color-bg-caution-subdued); - } - } - - &.statusInfo .PrimaryAction.Button { - border-color: var(--p-color-border-info-subdued); - - &:hover { - border-color: var(--p-color-border-info-subdued); - background: var(--p-color-bg-info-subdued-hover); - } - - &:active { - border-color: var(--p-color-border-info-subdued); - background: var(--p-color-bg-info-subdued-active); - } - - &:focus:not(:active) { - border-color: var(--p-color-border-info-subdued); - background: var(--p-color-bg-info-subdued); - } - } - - &.statusSuccess .PrimaryAction.Button { - border-color: var(--p-color-border-success-subdued); - - &:hover { - border-color: var(--p-color-border-success-subdued); - background: var(--p-color-bg-success-subdued-hover); - } - - &:active { - border-color: var(--p-color-border-success-subdued); - background: var(--p-color-bg-success-subdued-active); - } - - &:focus:not(:active) { - border-color: var(--p-color-border-success-subdued); - background: var(--p-color-bg-success-subdued); - } - } - // stylelint-enable selector-max-class, selector-max-specificity -} - -.ContentWrapper { - margin-top: calc(-1 * var(--p-space-05)); - flex: 1 1 auto; } .withinContentContainer { - padding: var(--p-space-4); - - #{$se23} & { - padding: 0; - } - - .Dismiss { - top: var(--p-space-4); - right: var(--p-space-3); - position: absolute; - } - - @include banner-variants($in-page: false); + border-radius: var(--p-border-radius-2); + .Banner { margin-top: var(--p-space-2); - - #{$se23} & { - box-shadow: none; - } } } .withinPage { - border-radius: 0 0 var(--p-border-radius-1) var(--p-border-radius-1); - padding: var(--p-space-5); - - #{$se23} & { - padding: 0; + @include shadow-bevel( + $boxShadow: var(--p-shadow-sm), + $borderRadius: var(--p-border-radius-0-experimental), + // The following arguments explicitly ignore the shadow-bevel opt out + $border: none, + $content: '' + ); + + @media #{$p-breakpoints-sm-up} { + @include shadow-bevel( + $boxShadow: var(--p-shadow-sm), + $borderRadius: var(--p-border-radius-3), + // The following arguments explicitly ignore the shadow-bevel opt out + $border: none, + $content: '' + ); } - @include banner-variants(true); - + .Banner { - margin-top: var(--p-space-5); - } - - .Dismiss { - right: var(--p-space-4); - top: var(--p-space-5); - position: absolute; + margin-top: var(--p-space-4); } } -.hasDismiss { - padding-right: calc(var(--p-space-8) + var(--p-space-2)); +// stylelint-disable -- Duplicate selectors to bump specificity for button svg color override +// https://developer.mozilla.org/en-US/docs/Web/CSS/Specificity#increasing_specificity_by_duplicating_selector - #{$se23} & { - padding: 0; +@mixin recolor-icon($fill-color: null) { + svg, + path { + fill: $fill-color; } } -// We need pretty high specificity to do the descendant selectors -// onto the text, which needs to be the relative positioned wrapper -// so that the borders/ backgrounds do not extend outside of it. - -.SecondaryAction { - // stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY - @include unstyled-button; - display: inline-block; - text-align: left; - text-decoration: none; - // stylelint-disable -- generated by polaris-migrator DO NOT COPY - margin: calc(-1 * var(--pc-banner-secondary-action-vertical-padding)) - calc(-0.5 * var(--pc-banner-secondary-action-horizontal-padding)); - // stylelint-enable - // stylelint-disable -- generated by polaris-migrator DO NOT COPY - padding: var(--pc-banner-secondary-action-vertical-padding) - var(--pc-banner-secondary-action-horizontal-padding); - // stylelint-enable - color: var(--p-color-text); - padding-left: var(--p-space-2); - - &:hover > .Text { - text-decoration: underline; - } - - &:active > .Text { - text-decoration: underline; - } - - &:focus-visible > .Text { - // stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY - @include plain-button-backdrop; - // stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY - @include focus-ring($style: 'focused'); - box-shadow: none; - text-decoration: underline; +.icon-on-color.icon-on-color.icon-on-color { + @include recolor-icon(var(--p-color-icon-on-color)); +} - @media (-ms-high-contrast: active) { - outline: var(--p-border-width-2) dotted; - } - } +.icon-success-strong-experimental.icon-success-strong-experimental.icon-success-strong-experimental { + @include recolor-icon(var(--p-color-icon-success-strong-experimental)); +} - &:visited { - color: inherit; - } +.text-warning-strong.text-warning-strong.text-warning-strong { + @include recolor-icon(var(--p-color-text-warning-strong)); } -.Text { - // stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY - @include focus-ring; +.icon-warning-strong-experimental.icon-warning-strong-experimental.icon-warning-strong-experimental { + @include recolor-icon(var(--p-color-icon-warning-strong-experimental)); } -.Button { - // stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY - @include button-base; - // stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY - @include focus-ring($border-width: 2px); - font-size: var(--p-font-size-100); - font-weight: var(--p-font-weight-medium); - line-height: var(--p-font-line-height-1); - letter-spacing: initial; - color: var(--p-color-text); +.icon-critical-strong-experimental.icon-critical-strong-experimental.icon-critical-strong-experimental { + @include recolor-icon(var(--p-color-icon-critical-strong-experimental)); +} - &:focus-visible { - // stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY - @include focus-ring($style: 'focused'); - } +.text-info-strong.text-info-strong.text-info-strong { + @include recolor-icon(var(--p-color-text-info-strong)); } -.loading { - position: relative; - color: transparent; - pointer-events: none; +.icon-info-strong-experimental.icon-info-strong-experimental.icon-info-strong-experimental { + @include recolor-icon(var(--p-color-icon-info-strong-experimental)); } -.Spinner { - // stylelint-disable-next-line -- Polaris component custom properties - --pc-spinner-size: var(--p-space-5); - position: absolute; - top: 50%; - left: 50%; - // stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY - margin-top: calc(-1 * (var(--pc-spinner-size) / 2)); - // stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY - margin-left: calc(-1 * (var(--pc-spinner-size) / 2)); +.icon-subdued.icon-subdued.icon-subdued { + @include recolor-icon(var(--p-color-icon-subdued)); } +// stylelint-enable diff --git a/polaris-react/src/components/Banner/Banner.tsx b/polaris-react/src/components/Banner/Banner.tsx index dd2ed34515c..f45459eff44 100644 --- a/polaris-react/src/components/Banner/Banner.tsx +++ b/polaris-react/src/components/Banner/Banner.tsx @@ -1,36 +1,36 @@ import React, { forwardRef, + useContext, useRef, useState, - useContext, - useImperativeHandle, + useEffect, + useCallback, } from 'react'; -import { - CancelSmallMinor, - CircleTickMajor, - CircleInformationMajor, - CircleAlertMajor, - DiamondAlertMajor, -} from '@shopify/polaris-icons'; +import type {PropsWithChildren} from 'react'; +import type {ColorTextAlias} from '@shopify/polaris-tokens'; +import {CancelMinor} from '@shopify/polaris-icons'; -import {classNames, variationName} from '../../utilities/css'; -import {BannerContext} from '../../utilities/banner-context'; -import {useI18n} from '../../utilities/i18n'; import type {Action, DisableableAction, LoadableAction} from '../../types'; +import {Text} from '../Text'; +import {VerticalStack} from '../VerticalStack'; +import type {HorizontalStackProps} from '../HorizontalStack'; +import {HorizontalStack} from '../HorizontalStack'; +import type {BoxProps} from '../Box'; +import {Box} from '../Box'; import {Button} from '../Button'; import {ButtonGroup} from '../ButtonGroup'; -import {UnstyledButton, unstyledButtonFrom} from '../UnstyledButton'; -import {UnstyledLink} from '../UnstyledLink'; -import {Spinner} from '../Spinner'; import {Icon} from '../Icon'; import type {IconProps} from '../Icon'; +import {BannerContext} from '../../utilities/banner-context'; import {WithinContentContext} from '../../utilities/within-content-context'; -import {Text} from '../Text'; -import {Box} from '../Box'; -import {useFeatures} from '../../utilities/features'; +import {classNames} from '../../utilities/css'; +import {useBreakpoints} from '../../utilities/breakpoints'; +import {useI18n} from '../../utilities/i18n'; +import {useEventListener} from '../../utilities/use-event-listener'; import styles from './Banner.scss'; -import {BannerExperimental} from './components'; +import type {BannerHandles} from './utilities'; +import {bannerAttributes, useBannerFocus} from './utilities'; export type BannerStatus = 'success' | 'info' | 'warning' | 'critical'; @@ -59,111 +59,16 @@ export const Banner = forwardRef(function Banner( props: BannerProps, bannerRef, ) { - const { - icon, - action, - secondaryAction, - title, - children, - status, - onDismiss, - stopAnnouncements, - hideIcon, - } = props; + const {status, stopAnnouncements} = props; const withinContentContainer = useContext(WithinContentContext); - const i18n = useI18n(); const {wrapperRef, handleKeyUp, handleBlur, handleMouseUp, shouldShowFocus} = useBannerFocus(bannerRef); - const {defaultIcon, iconColor, ariaRoleType} = useBannerAttributes(status); - const iconName = icon || defaultIcon; - const {polarisSummerEditions2023} = useFeatures(); const className = classNames( styles.Banner, - !polarisSummerEditions2023 && - status && - styles[variationName('status', status)], - onDismiss && styles.hasDismiss, shouldShowFocus && styles.keyFocused, withinContentContainer ? styles.withinContentContainer : styles.withinPage, ); - let headingMarkup: React.ReactNode = null; - - if (title) { - headingMarkup = ( - - {title} - - ); - } - - const spinnerMarkup = action?.loading ? ( - - ) : null; - - const primaryActionMarkup = action ? ( - - {action.loading - ? spinnerMarkup - : unstyledButtonFrom(action, { - className: `${styles.Button} ${styles.PrimaryAction}`, - })} - - ) : null; - - const secondaryActionMarkup = secondaryAction ? ( - - ) : null; - - const actionMarkup = - action || secondaryAction ? ( - - - {primaryActionMarkup} - {secondaryActionMarkup} - - - ) : null; - - let contentMarkup: React.ReactNode = null; - - if (children || actionMarkup) { - contentMarkup = ( - - {children} - {actionMarkup} - - ); - } - - const dismissButton = onDismiss && ( -
-
- ); - return (
(function Banner( // eslint-disable-next-line jsx-a11y/no-noninteractive-tabindex tabIndex={0} ref={wrapperRef} - role={ariaRoleType} + role={ + status === 'warning' || status === 'critical' ? 'alert' : 'status' + } aria-live={stopAnnouncements ? 'off' : 'polite'} onMouseUp={handleMouseUp} onKeyUp={handleKeyUp} onBlur={handleBlur} > - {polarisSummerEditions2023 ? ( - - ) : ( - <> - {dismissButton} - {hideIcon ? null : ( - - - - )} -
- {headingMarkup} - {contentMarkup} -
- - )} +
); }); -function SecondaryActionFrom({action}: {action: Action}) { - if (action.url) { +interface BannerLayoutProps { + backgroundColor: BoxProps['background']; + textColor: ColorTextAlias; + bannerTitle: React.ReactNode; + bannerIcon: React.ReactNode; + actionButtons: React.ReactNode; + dismissButton: React.ReactNode; +} + +export function BannerLayout({ + status = 'info', + icon, + hideIcon, + onDismiss, + action, + secondaryAction, + title, + children, +}: BannerProps) { + const i18n = useI18n(); + const withinContentContainer = useContext(WithinContentContext); + const isInlineIconBanner = !title && !withinContentContainer; + const bannerStatus = Object.keys(bannerAttributes).includes(status) + ? status + : 'info'; + const bannerColors = + bannerAttributes[bannerStatus][ + withinContentContainer ? 'withinContentContainer' : 'withinPage' + ]; + + const sharedBannerProps: BannerLayoutProps = { + backgroundColor: bannerColors.background, + textColor: bannerColors.text, + bannerTitle: title ? ( + + {title} + + ) : null, + bannerIcon: hideIcon ? null : ( + + + + ), + actionButtons: + action || secondaryAction ? ( + + {action && ( + + )} + {secondaryAction && ( + + )} + + ) : null, + dismissButton: onDismiss ? ( + - )} - {secondaryAction && ( - - )} - - ) : null, - dismissButton: onDismiss ? ( -