Updated logic for rendering sr text

patternfly · Oct 24, 2022 · 1887ea3 · 1887ea3
1 parent 1d1cb8c
commit 1887ea3
Show file tree

Hide file tree

Showing 6 changed files with 20 additions and 51 deletions.
diff --git a/packages/react-core/src/components/Banner/Banner.tsx b/packages/react-core/src/components/Banner/Banner.tsx
@@ -9,12 +9,10 @@ export interface BannerProps extends React.HTMLProps<HTMLDivElement> {
   className?: string;
   /** If set to true, the banner sticks to the top of its container */
   isSticky?: boolean;
-  /** Text announced by screen readers to indicate the type of banner. Defaults to "${variant} banner"
-   * if this property is not passed in.
-   *
-   * Pass in null to omit this text from the banner when the banner does not convey status/severity.
+  /** Text announced by screen readers to indicate the type of banner. This prop should only
+   * be passed in when the banner conveys status/severity.
    */
-  screenReaderText?: string | null;
+  screenReaderText?: string;
   /** Variant styles for the banner. */
   variant?: 'default' | 'info' | 'danger' | 'success' | 'warning';
 }
@@ -36,7 +34,7 @@ export const Banner: React.FunctionComponent<BannerProps> = ({
     )}
     {...props}
   >
-    {screenReaderText !== null && <span className="pf-u-screen-reader">{screenReaderText || `${variant} banner`}</span>}
+    {screenReaderText && <span className="pf-u-screen-reader">{screenReaderText}</span>}
     {children}
   </div>
 );

diff --git a/packages/react-core/src/components/Banner/__tests__/Banner.test.tsx b/packages/react-core/src/components/Banner/__tests__/Banner.test.tsx
@@ -51,32 +51,16 @@ test('Renders with class name pf-m-info when "info" is passed to variant prop',
   expect(screen.getByText('Test')).toHaveClass('pf-m-info');
 });
 
-test('Renders pf-u-screen-reader class by default for screenReaderText', () => {
+test('Does not render pf-u-screen-reader class by default', () => {
   render(<Banner>Test</Banner>);
-  expect(screen.getByText('default banner')).toHaveClass('pf-u-screen-reader', { exact: true });
+  expect(screen.getByText('Test')).not.toContainHTML('<span class="pf-u-screen-reader"></span>');
 });
 
-test('Renders screenReaderText as "default banner" by default', () => {
-  render(<Banner>Test</Banner>);
-  expect(screen.getByText('default banner')).toBeInTheDocument();
-});
-
-test('Renders screenReaderText as "success banner" when variant="success"', () => {
-  render(<Banner variant="success">Test</Banner>);
-  expect(screen.getByText('success banner')).toBeInTheDocument();
-});
-
-test('Renders custom screenReaderText passed via prop', () => {
+test('Renders screenReaderText passed via prop', () => {
   render(<Banner screenReaderText="Custom screen reader text">Test</Banner>);
   expect(screen.getByText('Custom screen reader text')).toBeInTheDocument();
 });
 
-test('Does not render with screen reader text when screenReaderText = null', () => {
-  render(<Banner screenReaderText={null}>Banner text</Banner>);
-
-  expect(screen.queryByText('default banner')).not.toBeInTheDocument();
-});
-
 test('Renders without pf-m-sticky by default', () => {
   render(<Banner>Test</Banner>);
   expect(screen.getByText('Test')).not.toHaveClass('pf-m-sticky');

diff --git a/packages/react-core/src/components/Banner/__tests__/__snapshots__/Banner.test.tsx.snap b/packages/react-core/src/components/Banner/__tests__/__snapshots__/Banner.test.tsx.snap
@@ -5,11 +5,6 @@ exports[`Matches the snapshot 1`] = `
   <div
     class="pf-c-banner"
   >
-    <span
-      class="pf-u-screen-reader"
-    >
-      default banner
-    </span>
     Test
   </div>
 </DocumentFragment>

diff --git a/packages/react-core/src/components/Banner/examples/Banner.md b/packages/react-core/src/components/Banner/examples/Banner.md
@@ -17,16 +17,16 @@ import BellIcon from '@patternfly/react-icons/dist/esm/icons/bell-icon';
 
 Banners can be styled with one of 5 different colors. A basic banner should only be used when the banner color does not represent status or severity.
 
-The following example also passes in the `screenReaderText` property with a value of `null` to prevent visually hidden text meant for screen readers from rendering. When using a basic banner, a value of `null` or text that does not convey status/severity should be passed into `screenReaderText`.
-
 ```ts file="./BannerBasic.tsx"
+
 ```
 
 ### Status
 
-When a banner is used to convey status, it is advised to pass in an icon inside the banner to convey the status in a way besides just color.
+When a banner is used to convey status, it is advised to pass in an icon inside the banner to convey the status in a way besides just color. The `screenReaderText` prop should also be passed in to convey the status/severity of the banner to users of certain assistive technologies such as screen readers.
 
 In the following example, a flex layout is used inside the banner content to show one possible way to create spacing between the icons and banner text.
 
 ```ts file="./BannerStatus.tsx"
+
 ```
diff --git a/packages/react-core/src/components/Banner/examples/BannerBasic.tsx b/packages/react-core/src/components/Banner/examples/BannerBasic.tsx
@@ -3,22 +3,14 @@ import { Banner } from '@patternfly/react-core';
 
 export const BannerBasic: React.FunctionComponent = () => (
   <>
-    <Banner screenReaderText={null}>Default banner</Banner>
+    <Banner>Default banner</Banner>
     <br />
-    <Banner screenReaderText={null} variant="info">
-      Blue banner
-    </Banner>
+    <Banner variant="info">Blue banner</Banner>
     <br />
-    <Banner screenReaderText={null} variant="danger">
-      Red banner
-    </Banner>
+    <Banner variant="danger">Red banner</Banner>
     <br />
-    <Banner screenReaderText={null} variant="success">
-      Green banner
-    </Banner>
+    <Banner variant="success">Green banner</Banner>
     <br />
-    <Banner screenReaderText={null} variant="warning">
-      Gold banner
-    </Banner>
+    <Banner variant="warning">Gold banner</Banner>
   </>
 );
diff --git a/packages/react-core/src/components/Banner/examples/BannerStatus.tsx b/packages/react-core/src/components/Banner/examples/BannerStatus.tsx
@@ -8,7 +8,7 @@ import BellIcon from '@patternfly/react-icons/dist/esm/icons/bell-icon';
 
 export const BannerStatus: React.FunctionComponent = () => (
   <>
-    <Banner>
+    <Banner screenReaderText="Default banner">
       <Flex spaceItems={{ default: 'spaceItemsSm' }}>
         <FlexItem>
           <BellIcon />
@@ -17,7 +17,7 @@ export const BannerStatus: React.FunctionComponent = () => (
       </Flex>
     </Banner>
     <br />
-    <Banner variant="info">
+    <Banner screenReaderText="Info banner" variant="info">
       <Flex spaceItems={{ default: 'spaceItemsSm' }}>
         <FlexItem>
           <InfoCircleIcon />
@@ -26,7 +26,7 @@ export const BannerStatus: React.FunctionComponent = () => (
       </Flex>
     </Banner>
     <br />
-    <Banner variant="danger">
+    <Banner screenReaderText="Danger banner" variant="danger">
       <Flex spaceItems={{ default: 'spaceItemsSm' }}>
         <FlexItem>
           <ExclamationCircleIcon />
@@ -35,7 +35,7 @@ export const BannerStatus: React.FunctionComponent = () => (
       </Flex>
     </Banner>
     <br />
-    <Banner variant="success">
+    <Banner screenReaderText="Success banner" variant="success">
       <Flex spaceItems={{ default: 'spaceItemsSm' }}>
         <FlexItem>
           <CheckCircleIcon />
@@ -44,7 +44,7 @@ export const BannerStatus: React.FunctionComponent = () => (
       </Flex>
     </Banner>
     <br />
-    <Banner variant="warning">
+    <Banner screenReaderText="Warning banner" variant="warning">
       <Flex spaceItems={{ default: 'spaceItemsSm' }}>
         <FlexItem>
           <ExclamationTriangleIcon />