Browse Source

ref(banner): Collocate banner specific logic (#26380)

Evan Purkhiser 3 years ago
parent
commit
99b2887c82

+ 70 - 38
static/app/components/banner.tsx

@@ -2,50 +2,28 @@ import * as React from 'react';
 import {css} from '@emotion/react';
 import styled from '@emotion/styled';
 
+import Button from 'app/components/button';
 import ButtonBar from 'app/components/buttonBar';
-import {IconClose} from 'app/icons/iconClose';
+import {IconClose} from 'app/icons';
 import {t} from 'app/locale';
 import space from 'app/styles/space';
 
-type Props = {
-  title?: string;
-  subtitle?: string;
-  isDismissable?: boolean;
-  onCloseClick?: () => void;
-  className?: string;
-} & BannerWrapperProps;
+const makeKey = (prefix: string) => `${prefix}-banner-dismissed`;
+
+function dismissBanner(bannerKey: string) {
+  localStorage.setItem(makeKey(bannerKey), 'true');
+}
 
-class Banner extends React.Component<Props> {
-  static defaultProps: Partial<Props> = {
-    isDismissable: true,
+function useDismissable(bannerKey: string) {
+  const key = makeKey(bannerKey);
+  const [value, setValue] = React.useState(localStorage.getItem(key));
+
+  const dismiss = () => {
+    setValue('true');
+    dismissBanner(bannerKey);
   };
 
-  render() {
-    const {
-      title,
-      subtitle,
-      isDismissable,
-      onCloseClick,
-      children,
-      backgroundImg,
-      backgroundComponent,
-      className,
-    } = this.props;
-
-    return (
-      <BannerWrapper backgroundImg={backgroundImg} className={className}>
-        {backgroundComponent}
-        {isDismissable ? (
-          <StyledIconClose aria-label={t('Close')} onClick={onCloseClick} />
-        ) : null}
-        <BannerContent>
-          <BannerTitle>{title}</BannerTitle>
-          <BannerSubtitle>{subtitle}</BannerSubtitle>
-          <StyledButtonBar gap={1}>{children}</StyledButtonBar>
-        </BannerContent>
-      </BannerWrapper>
-    );
-  }
+  return [value === 'true', dismiss] as const;
 }
 
 type BannerWrapperProps = {
@@ -53,6 +31,52 @@ type BannerWrapperProps = {
   backgroundComponent?: React.ReactNode;
 };
 
+type Props = BannerWrapperProps & {
+  title?: string;
+  subtitle?: string;
+  isDismissable?: boolean;
+  dismissKey?: string;
+  className?: string;
+};
+
+type BannerType = React.FC<Props> & {
+  /**
+   * Helper function to hide banners outside of their usage
+   */
+  dismiss: typeof dismissBanner;
+};
+
+const Banner: BannerType = ({
+  title,
+  subtitle,
+  isDismissable = true,
+  dismissKey = 'generic-banner',
+  className,
+  backgroundImg,
+  backgroundComponent,
+  children,
+}) => {
+  const [dismissed, dismiss] = useDismissable(dismissKey);
+
+  if (dismissed) {
+    return null;
+  }
+
+  return (
+    <BannerWrapper backgroundImg={backgroundImg} className={className}>
+      {backgroundComponent}
+      {isDismissable ? <CloseButton onClick={dismiss} /> : null}
+      <BannerContent>
+        <BannerTitle>{title}</BannerTitle>
+        <BannerSubtitle>{subtitle}</BannerSubtitle>
+        <StyledButtonBar gap={1}>{children}</StyledButtonBar>
+      </BannerContent>
+    </BannerWrapper>
+  );
+};
+
+Banner.dismiss = dismissBanner;
+
 const BannerWrapper = styled('div')<BannerWrapperProps>`
   ${p =>
     p.backgroundImg
@@ -111,7 +135,7 @@ const StyledButtonBar = styled(ButtonBar)`
   width: fit-content;
 `;
 
-const StyledIconClose = styled(IconClose)`
+const CloseButton = styled(Button)`
   position: absolute;
   display: block;
   top: ${space(2)};
@@ -121,4 +145,12 @@ const StyledIconClose = styled(IconClose)`
   z-index: 1;
 `;
 
+CloseButton.defaultProps = {
+  icon: <IconClose />,
+  label: t('Close'),
+  priority: 'link',
+  borderless: true,
+  size: 'xsmall',
+};
+
 export default Banner;

+ 33 - 0
static/app/utils/useMedia.tsx

@@ -0,0 +1,33 @@
+import {useEffect, useState} from 'react';
+
+/**
+ * Hook that updates when a media query result changes
+ */
+export default function useMedia(query: string) {
+  if (!window.matchMedia) {
+    return false;
+  }
+
+  const [state, setState] = useState(() => window.matchMedia(query).matches);
+
+  useEffect(() => {
+    let mounted = true;
+    const mql = window.matchMedia(query);
+    const onChange = () => {
+      if (!mounted) {
+        return;
+      }
+      setState(!!mql.matches);
+    };
+
+    mql.addListener(onChange);
+    setState(mql.matches);
+
+    return () => {
+      mounted = false;
+      mql.removeListener(onChange);
+    };
+  }, [query]);
+
+  return state;
+}

+ 6 - 4
static/app/views/eventsV2/banner.tsx

@@ -13,6 +13,8 @@ import FeatureTourModal, {
 import {t} from 'app/locale';
 import {Organization} from 'app/types';
 import {trackAnalyticsEvent} from 'app/utils/analytics';
+import theme from 'app/utils/theme';
+import useMedia from 'app/utils/useMedia';
 
 import BackgroundSpace from './backgroundSpace';
 
@@ -75,11 +77,9 @@ const TOUR_STEPS: TourStep[] = [
 type Props = {
   organization: Organization;
   resultsUrl: string;
-  isSmallBanner: boolean;
-  onHideBanner: () => void;
 };
 
-function DiscoverBanner({organization, resultsUrl, isSmallBanner, onHideBanner}: Props) {
+function DiscoverBanner({organization, resultsUrl}: Props) {
   function onAdvance(step: number, duration: number) {
     trackAnalyticsEvent({
       eventKey: 'discover_v2.tour.advance',
@@ -99,6 +99,8 @@ function DiscoverBanner({organization, resultsUrl, isSmallBanner, onHideBanner}:
     });
   }
 
+  const isSmallBanner = useMedia(`(max-width: ${theme.breakpoints[1]})`);
+
   return (
     <Banner
       title={t('Discover Trends')}
@@ -106,7 +108,7 @@ function DiscoverBanner({organization, resultsUrl, isSmallBanner, onHideBanner}:
         'Customize and save queries by search conditions, event fields, and tags'
       )}
       backgroundComponent={<BackgroundSpace />}
-      onCloseClick={onHideBanner}
+      dismissKey="discover"
     >
       <Button
         size={isSmallBanner ? 'xsmall' : undefined}

+ 2 - 57
static/app/views/eventsV2/landing.tsx

@@ -24,19 +24,12 @@ import {Organization, SavedQuery, SelectValue} from 'app/types';
 import {trackAnalyticsEvent} from 'app/utils/analytics';
 import EventView from 'app/utils/discover/eventView';
 import {decodeScalar} from 'app/utils/queryString';
-import theme from 'app/utils/theme';
 import withOrganization from 'app/utils/withOrganization';
 
 import Banner from './banner';
 import {DEFAULT_EVENT_VIEW} from './data';
 import QueryList from './queryList';
-import {
-  getPrebuiltQueries,
-  isBannerHidden,
-  setBannerHidden,
-  setRenderPrebuilt,
-  shouldRenderPrebuilt,
-} from './utils';
+import {getPrebuiltQueries, setRenderPrebuilt, shouldRenderPrebuilt} from './utils';
 
 const SORT_OPTIONS: SelectValue<string>[] = [
   {label: t('My Queries'), value: 'myqueries'},
@@ -55,15 +48,11 @@ type Props = {
 } & AsyncComponent['props'];
 
 type State = {
-  isBannerHidden: boolean;
-  isSmallBanner: boolean;
   savedQueries: SavedQuery[] | null;
   savedQueriesPageLinks: string;
 } & AsyncComponent['state'];
 
 class DiscoverLanding extends AsyncComponent<Props, State> {
-  mq = window.matchMedia?.(`(max-width: ${theme.breakpoints[1]})`);
-
   state: State = {
     // AsyncComponent state
     loading: true,
@@ -72,31 +61,11 @@ class DiscoverLanding extends AsyncComponent<Props, State> {
     errors: {},
 
     // local component state
-    isBannerHidden: isBannerHidden(),
     renderPrebuilt: shouldRenderPrebuilt(),
-    isSmallBanner: this.mq?.matches,
     savedQueries: null,
     savedQueriesPageLinks: '',
   };
 
-  componentDidMount() {
-    if (this.mq) {
-      this.mq.addListener(this.handleMediaQueryChange);
-    }
-  }
-
-  componentWillUnmount() {
-    if (this.mq) {
-      this.mq.removeListener(this.handleMediaQueryChange);
-    }
-  }
-
-  handleMediaQueryChange = (changed: MediaQueryListEvent) => {
-    this.setState({
-      isSmallBanner: changed.matches,
-    });
-  };
-
   shouldReload = true;
 
   getSavedQuerySearchQuery(): string {
@@ -171,13 +140,6 @@ class DiscoverLanding extends AsyncComponent<Props, State> {
   }
 
   componentDidUpdate(prevProps: Props) {
-    const isHidden = isBannerHidden();
-    if (isHidden !== this.state.isBannerHidden) {
-      // eslint-disable-next-line react/no-did-update-set-state
-      this.setState({
-        isBannerHidden: isHidden,
-      });
-    }
     const PAYLOAD_KEYS = ['sort', 'cursor', 'query'] as const;
 
     const payloadKeysChanged = !isEqual(
@@ -196,11 +158,6 @@ class DiscoverLanding extends AsyncComponent<Props, State> {
     this.fetchData({reloading: true});
   };
 
-  handleBannerClick = () => {
-    setBannerHidden(true);
-    this.setState({isBannerHidden: true});
-  };
-
   handleSearchQuery = (searchQuery: string) => {
     const {location} = this.props;
     ReactRouter.browserHistory.push({
@@ -232,24 +189,12 @@ class DiscoverLanding extends AsyncComponent<Props, State> {
   };
 
   renderBanner() {
-    const bannerDismissed = this.state.isBannerHidden;
-
-    if (bannerDismissed) {
-      return null;
-    }
     const {location, organization} = this.props;
     const eventView = EventView.fromNewQueryWithLocation(DEFAULT_EVENT_VIEW, location);
     const to = eventView.getResultsViewUrlTarget(organization.slug);
     const resultsUrl = `${to.pathname}?${stringify(to.query)}`;
 
-    return (
-      <Banner
-        organization={organization}
-        resultsUrl={resultsUrl}
-        isSmallBanner={this.state.isSmallBanner}
-        onHideBanner={this.handleBannerClick}
-      />
-    );
+    return <Banner organization={organization} resultsUrl={resultsUrl} />;
   }
 
   renderActions() {

+ 2 - 2
static/app/views/eventsV2/savedQuery/index.tsx

@@ -7,6 +7,7 @@ import {Client} from 'app/api';
 import Feature from 'app/components/acl/feature';
 import FeatureDisabled from 'app/components/acl/featureDisabled';
 import GuideAnchor from 'app/components/assistant/guideAnchor';
+import Banner from 'app/components/banner';
 import Button from 'app/components/button';
 import ButtonBar from 'app/components/buttonBar';
 import {CreateAlertFromViewButton} from 'app/components/createAlertButton';
@@ -21,7 +22,6 @@ import EventView from 'app/utils/discover/eventView';
 import {getDiscoverLandingUrl} from 'app/utils/discover/urls';
 import withApi from 'app/utils/withApi';
 import withProjects from 'app/utils/withProjects';
-import {setBannerHidden} from 'app/views/eventsV2/utils';
 import InputControl from 'app/views/settings/components/forms/controls/input';
 
 import {handleCreateQuery, handleDeleteQuery, handleUpdateQuery} from './utils';
@@ -161,7 +161,7 @@ class SavedQueryButtonGroup extends React.PureComponent<Props, State> {
       (savedQuery: SavedQuery) => {
         const view = EventView.fromSavedQuery(savedQuery);
 
-        setBannerHidden(true);
+        Banner.dismiss('discover');
         this.setState({queryName: ''});
         browserHistory.push(view.getResultsViewUrlTarget(organization.slug));
       }

+ 0 - 9
static/app/views/eventsV2/utils.tsx

@@ -523,15 +523,6 @@ export function generateFieldOptions({
   return fieldOptions;
 }
 
-const BANNER_DISMISSED_KEY = 'discover-banner-dismissed';
-
-export function isBannerHidden(): boolean {
-  return localStorage.getItem(BANNER_DISMISSED_KEY) === 'true';
-}
-export function setBannerHidden(value: boolean) {
-  localStorage.setItem(BANNER_DISMISSED_KEY, value ? 'true' : 'false');
-}
-
 const RENDER_PREBUILT_KEY = 'discover-render-prebuilt';
 
 export function shouldRenderPrebuilt(): boolean {

+ 20 - 0
tests/js/spec/components/banner.spec.tsx

@@ -0,0 +1,20 @@
+import {mountWithTheme} from 'sentry-test/enzyme';
+
+import Banner from 'app/components/banner';
+
+describe('Banner', function () {
+  it('can be dismissed', function () {
+    const banner = mountWithTheme(<Banner dismissKey="test" />);
+    expect(banner.find('BannerWrapper').exists()).toBe(true);
+
+    banner.find('CloseButton').simulate('click');
+
+    expect(banner.find('BannerWrapper').exists()).toBe(false);
+    expect(localStorage.getItem('test-banner-dismissed')).toBe('true');
+  });
+
+  it('is not dismissable', function () {
+    const banner = mountWithTheme(<Banner isDismissable={false} />);
+    expect(banner.find('CloseButton').exists()).toBe(false);
+  });
+});

+ 5 - 4
tests/js/spec/views/eventsV2/savedQuery/index.spec.jsx

@@ -1,10 +1,10 @@
 import {mountWithTheme} from 'sentry-test/enzyme';
 
 import EventView from 'app/utils/discover/eventView';
+import DiscoverBanner from 'app/views/eventsV2/banner';
 import {ALL_VIEWS} from 'app/views/eventsV2/data';
 import SavedQueryButtonGroup from 'app/views/eventsV2/savedQuery';
 import * as utils from 'app/views/eventsV2/savedQuery/utils';
-import {isBannerHidden, setBannerHidden} from 'app/views/eventsV2/utils';
 
 const SELECTOR_BUTTON_SAVE_AS = 'button[aria-label="Save as"]';
 const SELECTOR_BUTTON_SAVED = '[data-test-id="discover2-savedquery-button-saved"]';
@@ -100,7 +100,6 @@ describe('EventsV2 > SaveQueryButtonGroup', function () {
         errorsView,
         undefined
       );
-      setBannerHidden(false);
 
       // Click on ButtonSaveAs to open dropdown
       const buttonSaveAs = wrapper.find('DropdownControl').first();
@@ -114,8 +113,10 @@ describe('EventsV2 > SaveQueryButtonGroup', function () {
 
       // Click on Save in the Dropdown
       await buttonSaveAs.find('ButtonSaveDropDown Button').simulate('click');
-      // Banner should be hidden.
-      expect(isBannerHidden()).toEqual(true);
+
+      // The banner should not render
+      const banner = mountWithTheme(<DiscoverBanner />);
+      expect(banner.find('BannerTitle').exists()).toBe(false);
     });
 
     it('saves a well-formed query', async () => {