From 1fc9f6698219b7a2774118a06623e349dabd6c61 Mon Sep 17 00:00:00 2001
From: Eric Olkowski <thatblindgeye@gmail.com>
Date: Mon, 10 Oct 2022 14:55:54 -0400
Subject: [PATCH 1/5] feat(Banner): add support for status icons

---
 .../src/components/Banner/Banner.tsx          | 67 +++++++++++++------
 .../Banner/__tests__/Banner.test.tsx          | 66 ++++++++++++++++++
 .../src/components/Banner/examples/Banner.md  |  9 +++
 .../Banner/examples/BannerBasic.tsx           | 16 ++---
 .../Banner/examples/BannerStatus.tsx          | 24 +++++++
 5 files changed, 154 insertions(+), 28 deletions(-)
 create mode 100644 packages/react-core/src/components/Banner/examples/BannerStatus.tsx

diff --git a/packages/react-core/src/components/Banner/Banner.tsx b/packages/react-core/src/components/Banner/Banner.tsx
index f04edb943e7..bf96e5a60e2 100644
--- a/packages/react-core/src/components/Banner/Banner.tsx
+++ b/packages/react-core/src/components/Banner/Banner.tsx
@@ -1,39 +1,68 @@
 import * as React from 'react';
 import styles from '@patternfly/react-styles/css/components/Banner/banner';
 import { css } from '@patternfly/react-styles';
+import { variantIcons } from '../Alert/AlertIcon';
+import { Flex, FlexItem } from '../../layouts';
 
 export interface BannerProps extends React.HTMLProps<HTMLDivElement> {
-  /** Content rendered inside the banner */
+  /** Content rendered inside the banner. */
   children?: React.ReactNode;
-  /** Additional classes added to the banner */
+  /** Additional classes added to the banner. */
   className?: string;
-  /** Variant styles for the banner */
-  variant?: 'default' | 'info' | 'danger' | 'success' | 'warning';
+  /** A custom icon for the banner. This property will override the icon that is set based on
+   * the variant property.
+   */
+  customIcon?: React.ReactNode;
+  /** Flag for indicating whether the banner has a status icon. When set to "true", the icon
+   * will be set based on the variant property.
+   */
+  hasStatusIcon?: boolean;
   /** 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 prop is not passed in */
+  /** Text announced by screen readers to indicate the type of banner when the hasStatusIcon property
+   * is passed in. Defaults to "${variant} banner" if this property is not passed in.
+   */
   screenReaderText?: string;
+  /** Variant styles for the banner. */
+  variant?: 'default' | 'info' | 'danger' | 'success' | 'warning';
 }
 
 export const Banner: React.FunctionComponent<BannerProps> = ({
   children,
   className,
+  customIcon,
+  hasStatusIcon = false,
   variant = 'default',
   screenReaderText,
   isSticky = false,
   ...props
-}: BannerProps) => (
-  <div
-    className={css(
-      styles.banner,
-      styles.modifiers[variant as 'success' | 'danger' | 'warning' | 'info'],
-      isSticky && styles.modifiers.sticky,
-      className
-    )}
-    {...props}
-  >
-    {children}
-    <span className="pf-u-screen-reader">{screenReaderText || `${variant} banner`}</span>
-  </div>
-);
+}: BannerProps) => {
+  const StatusIcon = variantIcons[variant];
+
+  return (
+    <div
+      className={css(
+        styles.banner,
+        styles.modifiers[variant as 'success' | 'danger' | 'warning' | 'info'],
+        isSticky && styles.modifiers.sticky,
+        className
+      )}
+      {...props}
+    >
+      {hasStatusIcon ? (
+        <Flex spaceItems={{ default: 'spaceItemsSm' }}>
+          <FlexItem>
+            <span className="pf-u-screen-reader">{screenReaderText || `${variant} banner`}</span>
+            {customIcon || <StatusIcon />}
+          </FlexItem>
+          <FlexItem>
+            <div className="pf-l-flex__item">{children}</div>
+          </FlexItem>
+        </Flex>
+      ) : (
+        children
+      )}
+    </div>
+  );
+};
 Banner.displayName = 'Banner';
diff --git a/packages/react-core/src/components/Banner/__tests__/Banner.test.tsx b/packages/react-core/src/components/Banner/__tests__/Banner.test.tsx
index 1dc8d86b33e..6dcad7a559b 100644
--- a/packages/react-core/src/components/Banner/__tests__/Banner.test.tsx
+++ b/packages/react-core/src/components/Banner/__tests__/Banner.test.tsx
@@ -1,6 +1,16 @@
 import * as React from 'react';
 import { render, screen } from '@testing-library/react';
 import { Banner } from '../Banner';
+import React from 'react';
+import { render, screen } from '@testing-library/react';
+
+jest.mock('@patternfly/react-icons/dist/esm/icons/check-circle-icon', () => () => 'Check circle icon mock');
+jest.mock('@patternfly/react-icons/dist/esm/icons/exclamation-circle-icon', () => () => 'Exclamation circle icon mock');
+jest.mock('@patternfly/react-icons/dist/esm/icons/exclamation-triangle-icon', () => () =>
+  'Exclamation triangle icon mock'
+);
+jest.mock('@patternfly/react-icons/dist/esm/icons/info-circle-icon', () => () => 'Info circle icon mock');
+jest.mock('@patternfly/react-icons/dist/esm/icons/bell-icon', () => () => 'Bell icon mock');
 
 test('Renders without children', () => {
   render(
@@ -90,3 +100,59 @@ test('Matches the snapshot', () => {
   const { asFragment } = render(<Banner>Test</Banner>);
   expect(asFragment()).toMatchSnapshot();
 });
+
+test('Renders with the bell icon when variant is not passed and hasStatusIcon is passed', () => {
+  render(<Banner hasStatusIcon>Default banner</Banner>);
+
+  expect(screen.getByText('Bell icon mock')).toBeVisible();
+});
+
+test('Renders with the bell icon when variant = "default" and hasStatusIcon is passed', () => {
+  render(
+    <Banner variant="default" hasStatusIcon>
+      Default banner
+    </Banner>
+  );
+
+  expect(screen.getByText('Bell icon mock')).toBeVisible();
+});
+
+test('Renders with the info circle icon when variant = "info" and hasStatusIcon is passed', () => {
+  render(
+    <Banner variant="info" hasStatusIcon>
+      Info banner
+    </Banner>
+  );
+
+  expect(screen.getByText('Info circle icon mock')).toBeVisible();
+});
+
+test('Renders with the exclamation circle icon when variant = "danger" and hasStatusIcon is passed', () => {
+  render(
+    <Banner variant="danger" hasStatusIcon>
+      Danger banner
+    </Banner>
+  );
+
+  expect(screen.getByText('Exclamation circle icon mock')).toBeVisible();
+});
+
+test('Renders with the check circle icon when variant = "success" and hasStatusIcon is passed', () => {
+  render(
+    <Banner variant="success" hasStatusIcon>
+      Success banner
+    </Banner>
+  );
+
+  expect(screen.getByText('Check circle icon mock')).toBeVisible();
+});
+
+test('Renders with the exclamation triangle icon when variant = "warning" and hasStatusIcon is passed', () => {
+  render(
+    <Banner variant="warning" hasStatusIcon>
+      Warning banner
+    </Banner>
+  );
+
+  expect(screen.getByText('Exclamation triangle icon mock')).toBeVisible();
+});
diff --git a/packages/react-core/src/components/Banner/examples/Banner.md b/packages/react-core/src/components/Banner/examples/Banner.md
index 8669d5220ea..0d320d9cce3 100644
--- a/packages/react-core/src/components/Banner/examples/Banner.md
+++ b/packages/react-core/src/components/Banner/examples/Banner.md
@@ -9,5 +9,14 @@ propComponents: ['Banner']
 
 ### Basic
 
+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.
+
 ```ts file="./BannerBasic.tsx"
 ```
+
+### Status
+
+When a banner is used to convey status, it is advised to pass in either the `hasStatusIcon` or `customIcon` property to render an icon inside the banner. This icon should convey the banner variant beyond just the banner color.
+
+```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
index 71a895c37de..31ad5034525 100644
--- a/packages/react-core/src/components/Banner/examples/BannerBasic.tsx
+++ b/packages/react-core/src/components/Banner/examples/BannerBasic.tsx
@@ -2,17 +2,15 @@ import React from 'react';
 import { Banner } from '@patternfly/react-core';
 
 export const BannerBasic: React.FunctionComponent = () => (
-  <React.Fragment>
-    <Banner screenReaderText="This is a default Banner">Default banner</Banner>
+  <>
+    <Banner>Default banner</Banner>
     <br />
-    <Banner variant="info" screenReaderText="This is an info Banner">
-      Info banner
-    </Banner>
+    <Banner variant="info">Blue banner</Banner>
     <br />
-    <Banner variant="danger">Danger banner</Banner>
+    <Banner variant="danger">Red banner</Banner>
     <br />
-    <Banner variant="success">Success banner</Banner>
+    <Banner variant="success">Green banner</Banner>
     <br />
-    <Banner variant="warning">Warning banner</Banner>
-  </React.Fragment>
+    <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
new file mode 100644
index 00000000000..926bad49b74
--- /dev/null
+++ b/packages/react-core/src/components/Banner/examples/BannerStatus.tsx
@@ -0,0 +1,24 @@
+import React from 'react';
+import { Banner } from '@patternfly/react-core';
+
+export const BannerStatus: React.FunctionComponent = () => (
+  <>
+    <Banner hasStatusIcon>Default banner</Banner>
+    <br />
+    <Banner hasStatusIcon variant="info">
+      Info banner
+    </Banner>
+    <br />
+    <Banner hasStatusIcon variant="danger">
+      Danger banner
+    </Banner>
+    <br />
+    <Banner hasStatusIcon variant="success">
+      Success banner
+    </Banner>
+    <br />
+    <Banner hasStatusIcon variant="warning">
+      Warning banner
+    </Banner>
+  </>
+);

From 33719092b5133176dc2b2699403fb30d4722ac4f Mon Sep 17 00:00:00 2001
From: Eric Olkowski <thatblindgeye@gmail.com>
Date: Tue, 11 Oct 2022 14:49:49 -0400
Subject: [PATCH 2/5] Made icon support an example instead of new props

---
 .../src/components/Banner/Banner.tsx          | 63 ++++++-------------
 .../Banner/__tests__/Banner.test.tsx          | 58 +++--------------
 .../src/components/Banner/examples/Banner.md  | 10 ++-
 .../Banner/examples/BannerBasic.tsx           | 18 ++++--
 .../Banner/examples/BannerStatus.tsx          | 25 +++++---
 5 files changed, 66 insertions(+), 108 deletions(-)

diff --git a/packages/react-core/src/components/Banner/Banner.tsx b/packages/react-core/src/components/Banner/Banner.tsx
index bf96e5a60e2..ef0e3a648a9 100644
--- a/packages/react-core/src/components/Banner/Banner.tsx
+++ b/packages/react-core/src/components/Banner/Banner.tsx
@@ -1,28 +1,20 @@
 import * as React from 'react';
 import styles from '@patternfly/react-styles/css/components/Banner/banner';
 import { css } from '@patternfly/react-styles';
-import { variantIcons } from '../Alert/AlertIcon';
-import { Flex, FlexItem } from '../../layouts';
 
 export interface BannerProps extends React.HTMLProps<HTMLDivElement> {
   /** Content rendered inside the banner. */
   children?: React.ReactNode;
   /** Additional classes added to the banner. */
   className?: string;
-  /** A custom icon for the banner. This property will override the icon that is set based on
-   * the variant property.
-   */
-  customIcon?: React.ReactNode;
-  /** Flag for indicating whether the banner has a status icon. When set to "true", the icon
-   * will be set based on the variant property.
-   */
-  hasStatusIcon?: boolean;
   /** 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 when the hasStatusIcon property
-   * is passed in. Defaults to "${variant} banner" if this property is not passed in.
+  /** 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.
    */
-  screenReaderText?: string;
+  screenReaderText?: string | null;
   /** Variant styles for the banner. */
   variant?: 'default' | 'info' | 'danger' | 'success' | 'warning';
 }
@@ -30,39 +22,22 @@ export interface BannerProps extends React.HTMLProps<HTMLDivElement> {
 export const Banner: React.FunctionComponent<BannerProps> = ({
   children,
   className,
-  customIcon,
-  hasStatusIcon = false,
   variant = 'default',
   screenReaderText,
   isSticky = false,
   ...props
-}: BannerProps) => {
-  const StatusIcon = variantIcons[variant];
-
-  return (
-    <div
-      className={css(
-        styles.banner,
-        styles.modifiers[variant as 'success' | 'danger' | 'warning' | 'info'],
-        isSticky && styles.modifiers.sticky,
-        className
-      )}
-      {...props}
-    >
-      {hasStatusIcon ? (
-        <Flex spaceItems={{ default: 'spaceItemsSm' }}>
-          <FlexItem>
-            <span className="pf-u-screen-reader">{screenReaderText || `${variant} banner`}</span>
-            {customIcon || <StatusIcon />}
-          </FlexItem>
-          <FlexItem>
-            <div className="pf-l-flex__item">{children}</div>
-          </FlexItem>
-        </Flex>
-      ) : (
-        children
-      )}
-    </div>
-  );
-};
+}: BannerProps) => (
+  <div
+    className={css(
+      styles.banner,
+      styles.modifiers[variant as 'success' | 'danger' | 'warning' | 'info'],
+      isSticky && styles.modifiers.sticky,
+      className
+    )}
+    {...props}
+  >
+    {screenReaderText !== null && <span className="pf-u-screen-reader">{screenReaderText || `${variant} banner`}</span>}
+    {children}
+  </div>
+);
 Banner.displayName = 'Banner';
diff --git a/packages/react-core/src/components/Banner/__tests__/Banner.test.tsx b/packages/react-core/src/components/Banner/__tests__/Banner.test.tsx
index 6dcad7a559b..f47027169a4 100644
--- a/packages/react-core/src/components/Banner/__tests__/Banner.test.tsx
+++ b/packages/react-core/src/components/Banner/__tests__/Banner.test.tsx
@@ -1,8 +1,6 @@
 import * as React from 'react';
 import { render, screen } from '@testing-library/react';
 import { Banner } from '../Banner';
-import React from 'react';
-import { render, screen } from '@testing-library/react';
 
 jest.mock('@patternfly/react-icons/dist/esm/icons/check-circle-icon', () => () => 'Check circle icon mock');
 jest.mock('@patternfly/react-icons/dist/esm/icons/exclamation-circle-icon', () => () => 'Exclamation circle icon mock');
@@ -101,58 +99,20 @@ test('Matches the snapshot', () => {
   expect(asFragment()).toMatchSnapshot();
 });
 
-test('Renders with the bell icon when variant is not passed and hasStatusIcon is passed', () => {
-  render(<Banner hasStatusIcon>Default banner</Banner>);
-
-  expect(screen.getByText('Bell icon mock')).toBeVisible();
-});
-
-test('Renders with the bell icon when variant = "default" and hasStatusIcon is passed', () => {
-  render(
-    <Banner variant="default" hasStatusIcon>
-      Default banner
-    </Banner>
-  );
-
-  expect(screen.getByText('Bell icon mock')).toBeVisible();
-});
-
-test('Renders with the info circle icon when variant = "info" and hasStatusIcon is passed', () => {
-  render(
-    <Banner variant="info" hasStatusIcon>
-      Info banner
-    </Banner>
-  );
-
-  expect(screen.getByText('Info circle icon mock')).toBeVisible();
-});
-
-test('Renders with the exclamation circle icon when variant = "danger" and hasStatusIcon is passed', () => {
-  render(
-    <Banner variant="danger" hasStatusIcon>
-      Danger banner
-    </Banner>
-  );
+test('Renders with screen reader text by default', () => {
+  render(<Banner>Banner text</Banner>);
 
-  expect(screen.getByText('Exclamation circle icon mock')).toBeVisible();
+  expect(screen.getByText('default banner')).toBeInTheDocument();
 });
 
-test('Renders with the check circle icon when variant = "success" and hasStatusIcon is passed', () => {
-  render(
-    <Banner variant="success" hasStatusIcon>
-      Success banner
-    </Banner>
-  );
+test('Does not render with screen reader text when screenReaderText = null', () => {
+  render(<Banner screenReaderText={null}>Banner text</Banner>);
 
-  expect(screen.getByText('Check circle icon mock')).toBeVisible();
+  expect(screen.queryByText('default banner')).not.toBeInTheDocument();
 });
 
-test('Renders with the exclamation triangle icon when variant = "warning" and hasStatusIcon is passed', () => {
-  render(
-    <Banner variant="warning" hasStatusIcon>
-      Warning banner
-    </Banner>
-  );
+test('Renders with custom screen reader text when screenReaderText is passed', () => {
+  render(<Banner screenReaderText="Custom screen reader text">Banner text</Banner>);
 
-  expect(screen.getByText('Exclamation triangle icon mock')).toBeVisible();
+  expect(screen.getByText('Custom screen reader text')).toBeInTheDocument();
 });
diff --git a/packages/react-core/src/components/Banner/examples/Banner.md b/packages/react-core/src/components/Banner/examples/Banner.md
index 0d320d9cce3..22b67fc6ee2 100644
--- a/packages/react-core/src/components/Banner/examples/Banner.md
+++ b/packages/react-core/src/components/Banner/examples/Banner.md
@@ -5,18 +5,26 @@ cssPrefix: pf-c-banner
 propComponents: ['Banner']
 ---
 
+import CheckCircleIcon from '@patternfly/react-icons/dist/esm/icons/check-circle-icon';
+import ExclamationCircleIcon from '@patternfly/react-icons/dist/esm/icons/exclamation-circle-icon';
+import ExclamationTriangleIcon from '@patternfly/react-icons/dist/esm/icons/exclamation-triangle-icon';
+import InfoCircleIcon from '@patternfly/react-icons/dist/esm/icons/info-circle-icon';
+import BellIcon from '@patternfly/react-icons/dist/esm/icons/bell-icon';
+
 ## Examples
 
 ### Basic
 
 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 either the `hasStatusIcon` or `customIcon` property to render an icon inside the banner. This icon should convey the banner variant beyond just the banner 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.
 
 ```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
index 31ad5034525..c783287e59b 100644
--- a/packages/react-core/src/components/Banner/examples/BannerBasic.tsx
+++ b/packages/react-core/src/components/Banner/examples/BannerBasic.tsx
@@ -3,14 +3,22 @@ import { Banner } from '@patternfly/react-core';
 
 export const BannerBasic: React.FunctionComponent = () => (
   <>
-    <Banner>Default banner</Banner>
+    <Banner screenReaderText={null}>Default banner</Banner>
     <br />
-    <Banner variant="info">Blue banner</Banner>
+    <Banner screenReaderText={null} variant="info">
+      Blue banner
+    </Banner>
     <br />
-    <Banner variant="danger">Red banner</Banner>
+    <Banner screenReaderText={null} variant="danger">
+      Red banner
+    </Banner>
     <br />
-    <Banner variant="success">Green banner</Banner>
+    <Banner screenReaderText={null} variant="success">
+      Green banner
+    </Banner>
     <br />
-    <Banner variant="warning">Gold banner</Banner>
+    <Banner screenReaderText={null} 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
index 926bad49b74..7251469df58 100644
--- a/packages/react-core/src/components/Banner/examples/BannerStatus.tsx
+++ b/packages/react-core/src/components/Banner/examples/BannerStatus.tsx
@@ -1,24 +1,31 @@
 import React from 'react';
 import { Banner } from '@patternfly/react-core';
+import CheckCircleIcon from '@patternfly/react-icons/dist/esm/icons/check-circle-icon';
+import ExclamationCircleIcon from '@patternfly/react-icons/dist/esm/icons/exclamation-circle-icon';
+import ExclamationTriangleIcon from '@patternfly/react-icons/dist/esm/icons/exclamation-triangle-icon';
+import InfoCircleIcon from '@patternfly/react-icons/dist/esm/icons/info-circle-icon';
+import BellIcon from '@patternfly/react-icons/dist/esm/icons/bell-icon';
 
 export const BannerStatus: React.FunctionComponent = () => (
   <>
-    <Banner hasStatusIcon>Default banner</Banner>
+    <Banner>
+      <BellIcon /> Default banner
+    </Banner>
     <br />
-    <Banner hasStatusIcon variant="info">
-      Info banner
+    <Banner variant="info">
+      <InfoCircleIcon /> Info banner
     </Banner>
     <br />
-    <Banner hasStatusIcon variant="danger">
-      Danger banner
+    <Banner variant="danger">
+      <ExclamationCircleIcon /> Danger banner
     </Banner>
     <br />
-    <Banner hasStatusIcon variant="success">
-      Success banner
+    <Banner variant="success">
+      <CheckCircleIcon /> Success banner
     </Banner>
     <br />
-    <Banner hasStatusIcon variant="warning">
-      Warning banner
+    <Banner variant="warning">
+      <ExclamationTriangleIcon /> Warning banner
     </Banner>
   </>
 );

From 4c103313813bada6163efa5a62d2b2e394da1cdd Mon Sep 17 00:00:00 2001
From: Eric Olkowski <thatblindgeye@gmail.com>
Date: Thu, 13 Oct 2022 10:53:36 -0400
Subject: [PATCH 3/5] Added flex layout to status example

---
 .../Banner/__tests__/Banner.test.tsx          | 32 +++-------------
 .../src/components/Banner/examples/Banner.md  |  2 +
 .../Banner/examples/BannerStatus.tsx          | 37 ++++++++++++++++---
 3 files changed, 39 insertions(+), 32 deletions(-)

diff --git a/packages/react-core/src/components/Banner/__tests__/Banner.test.tsx b/packages/react-core/src/components/Banner/__tests__/Banner.test.tsx
index f47027169a4..b1befa38c9a 100644
--- a/packages/react-core/src/components/Banner/__tests__/Banner.test.tsx
+++ b/packages/react-core/src/components/Banner/__tests__/Banner.test.tsx
@@ -2,14 +2,6 @@ import * as React from 'react';
 import { render, screen } from '@testing-library/react';
 import { Banner } from '../Banner';
 
-jest.mock('@patternfly/react-icons/dist/esm/icons/check-circle-icon', () => () => 'Check circle icon mock');
-jest.mock('@patternfly/react-icons/dist/esm/icons/exclamation-circle-icon', () => () => 'Exclamation circle icon mock');
-jest.mock('@patternfly/react-icons/dist/esm/icons/exclamation-triangle-icon', () => () =>
-  'Exclamation triangle icon mock'
-);
-jest.mock('@patternfly/react-icons/dist/esm/icons/info-circle-icon', () => () => 'Info circle icon mock');
-jest.mock('@patternfly/react-icons/dist/esm/icons/bell-icon', () => () => 'Bell icon mock');
-
 test('Renders without children', () => {
   render(
     <div data-testid="banner">
@@ -79,6 +71,12 @@ test('Renders custom screenReaderText passed via prop', () => {
   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');
@@ -98,21 +96,3 @@ test('Matches the snapshot', () => {
   const { asFragment } = render(<Banner>Test</Banner>);
   expect(asFragment()).toMatchSnapshot();
 });
-
-test('Renders with screen reader text by default', () => {
-  render(<Banner>Banner text</Banner>);
-
-  expect(screen.getByText('default banner')).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 with custom screen reader text when screenReaderText is passed', () => {
-  render(<Banner screenReaderText="Custom screen reader text">Banner text</Banner>);
-
-  expect(screen.getByText('Custom screen reader text')).toBeInTheDocument();
-});
diff --git a/packages/react-core/src/components/Banner/examples/Banner.md b/packages/react-core/src/components/Banner/examples/Banner.md
index 22b67fc6ee2..ed9039eaf66 100644
--- a/packages/react-core/src/components/Banner/examples/Banner.md
+++ b/packages/react-core/src/components/Banner/examples/Banner.md
@@ -26,5 +26,7 @@ The following example also passes in the `screenReaderText` property with a valu
 
 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.
 
+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/BannerStatus.tsx b/packages/react-core/src/components/Banner/examples/BannerStatus.tsx
index 7251469df58..d77df9b45f0 100644
--- a/packages/react-core/src/components/Banner/examples/BannerStatus.tsx
+++ b/packages/react-core/src/components/Banner/examples/BannerStatus.tsx
@@ -1,5 +1,5 @@
 import React from 'react';
-import { Banner } from '@patternfly/react-core';
+import { Banner, Flex, FlexItem } from '@patternfly/react-core';
 import CheckCircleIcon from '@patternfly/react-icons/dist/esm/icons/check-circle-icon';
 import ExclamationCircleIcon from '@patternfly/react-icons/dist/esm/icons/exclamation-circle-icon';
 import ExclamationTriangleIcon from '@patternfly/react-icons/dist/esm/icons/exclamation-triangle-icon';
@@ -9,23 +9,48 @@ import BellIcon from '@patternfly/react-icons/dist/esm/icons/bell-icon';
 export const BannerStatus: React.FunctionComponent = () => (
   <>
     <Banner>
-      <BellIcon /> Default banner
+      <Flex spaceItems={{ default: 'spaceItemsSm' }}>
+        <FlexItem>
+          <BellIcon />
+        </FlexItem>
+        <FlexItem>Default banner</FlexItem>
+      </Flex>
     </Banner>
     <br />
     <Banner variant="info">
-      <InfoCircleIcon /> Info banner
+      <Flex spaceItems={{ default: 'spaceItemsSm' }}>
+        <FlexItem>
+          <InfoCircleIcon />
+        </FlexItem>
+        <FlexItem>Info banner</FlexItem>
+      </Flex>
     </Banner>
     <br />
     <Banner variant="danger">
-      <ExclamationCircleIcon /> Danger banner
+      <Flex spaceItems={{ default: 'spaceItemsSm' }}>
+        <FlexItem>
+          <ExclamationCircleIcon />
+        </FlexItem>
+        <FlexItem>Danger banner</FlexItem>
+      </Flex>
     </Banner>
     <br />
     <Banner variant="success">
-      <CheckCircleIcon /> Success banner
+      <Flex spaceItems={{ default: 'spaceItemsSm' }}>
+        <FlexItem>
+          <CheckCircleIcon />
+        </FlexItem>
+        <FlexItem>Success banner</FlexItem>
+      </Flex>
     </Banner>
     <br />
     <Banner variant="warning">
-      <ExclamationTriangleIcon /> Warning banner
+      <Flex spaceItems={{ default: 'spaceItemsSm' }}>
+        <FlexItem>
+          <ExclamationTriangleIcon />
+        </FlexItem>
+        <FlexItem>Warning banner</FlexItem>
+      </Flex>
     </Banner>
   </>
 );

From 1d1cb8ca755c81cfa03a0622872df0c1f9b1f7cf Mon Sep 17 00:00:00 2001
From: Eric Olkowski <thatblindgeye@gmail.com>
Date: Thu, 13 Oct 2022 10:55:50 -0400
Subject: [PATCH 4/5] Updated snapshot after rebase

---
 .../Banner/__tests__/__snapshots__/Banner.test.tsx.snap         | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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
index 1d6551bd157..bd9c2619021 100644
--- 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,12 +5,12 @@ exports[`Matches the snapshot 1`] = `
   <div
     class="pf-c-banner"
   >
-    Test
     <span
       class="pf-u-screen-reader"
     >
       default banner
     </span>
+    Test
   </div>
 </DocumentFragment>
 `;

From 1887ea345941080fa07eb153773826bcfe67e916 Mon Sep 17 00:00:00 2001
From: Eric Olkowski <thatblindgeye@gmail.com>
Date: Mon, 24 Oct 2022 10:32:04 -0400
Subject: [PATCH 5/5] Updated logic for rendering sr text

---
 .../src/components/Banner/Banner.tsx          | 10 ++++-----
 .../Banner/__tests__/Banner.test.tsx          | 22 +++----------------
 .../__snapshots__/Banner.test.tsx.snap        |  5 -----
 .../src/components/Banner/examples/Banner.md  |  6 ++---
 .../Banner/examples/BannerBasic.tsx           | 18 +++++----------
 .../Banner/examples/BannerStatus.tsx          | 10 ++++-----
 6 files changed, 20 insertions(+), 51 deletions(-)

diff --git a/packages/react-core/src/components/Banner/Banner.tsx b/packages/react-core/src/components/Banner/Banner.tsx
index ef0e3a648a9..1e2db401c5c 100644
--- 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
index b1befa38c9a..894f6ec2ae0 100644
--- 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
index bd9c2619021..d595c3708d2 100644
--- 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
index ed9039eaf66..49876b514a6 100644
--- 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
index c783287e59b..31ad5034525 100644
--- 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
index d77df9b45f0..016c067a5b0 100644
--- 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 />