From 9568dc67bf43c4a6995f8c80fbdd14ccc8929c1f Mon Sep 17 00:00:00 2001
From: Josh Black <joshblack@users.noreply.github.com>
Date: Fri, 26 Apr 2024 17:01:37 -0500
Subject: [PATCH 1/6] refactor(Banner): update region to use a dedicated
 aria-label

---
 packages/react/src/Banner/Banner.test.tsx     |  34 ++++-
 packages/react/src/Banner/Banner.tsx          | 129 +++++++++---------
 .../Banner/__snapshots__/Banner.test.tsx.snap |   2 +-
 .../react/src/internal/hooks/useMergedRefs.ts |  16 +++
 4 files changed, 111 insertions(+), 70 deletions(-)
 create mode 100644 packages/react/src/internal/hooks/useMergedRefs.ts

diff --git a/packages/react/src/Banner/Banner.test.tsx b/packages/react/src/Banner/Banner.test.tsx
index 0da386c115e..eead7e8ed38 100644
--- a/packages/react/src/Banner/Banner.test.tsx
+++ b/packages/react/src/Banner/Banner.test.tsx
@@ -25,12 +25,38 @@ describe('Banner', () => {
 
   it('should render as a region element', () => {
     render(<Banner title="test" />)
-    expect(screen.getByRole('region', {name: 'test'})).toBeInTheDocument()
+    expect(screen.getByRole('region', {name: 'Information'})).toBeInTheDocument()
+    expect(screen.getByRole('heading', {name: 'test'})).toBeInTheDocument()
   })
 
-  it('should label the landmark element with the title prop', () => {
+  it('should label the landmark element with the corresponding variant label text', () => {
     render(<Banner title="test" />)
-    expect(screen.getByRole('region')).toEqual(screen.getByLabelText('test'))
+    expect(screen.getByRole('region')).toEqual(screen.getByLabelText('Information'))
+  })
+
+  it('should label the landmark element with the label for the critical variant', () => {
+    render(<Banner title="test" variant="critical" />)
+    expect(screen.getByRole('region')).toEqual(screen.getByLabelText('Critical'))
+  })
+
+  it('should label the landmark element with the label for the info variant', () => {
+    render(<Banner title="test" variant="info" />)
+    expect(screen.getByRole('region')).toEqual(screen.getByLabelText('Information'))
+  })
+
+  it('should label the landmark element with the label for the success variant', () => {
+    render(<Banner title="test" variant="success" />)
+    expect(screen.getByRole('region')).toEqual(screen.getByLabelText('Success'))
+  })
+
+  it('should label the landmark element with the label for the upsell variant', () => {
+    render(<Banner title="test" variant="upsell" />)
+    expect(screen.getByRole('region')).toEqual(screen.getByLabelText('Recommendation'))
+  })
+
+  it('should label the landmark element with the label for the warning variant', () => {
+    render(<Banner title="test" variant="warning" />)
+    expect(screen.getByRole('region')).toEqual(screen.getByLabelText('Warning'))
   })
 
   it('should default the title to a h2', () => {
@@ -50,7 +76,7 @@ describe('Banner', () => {
   it('should rendering a description with the `description` prop', () => {
     render(<Banner title="test" description="test-description" />)
     expect(screen.getByText('test-description')).toBeInTheDocument()
-    expect(screen.getByRole('region', {name: 'test'})).toContainElement(screen.getByText('test-description'))
+    expect(screen.getByRole('region', {name: 'Information'})).toContainElement(screen.getByText('test-description'))
   })
 
   it('should support a primary action', async () => {
diff --git a/packages/react/src/Banner/Banner.tsx b/packages/react/src/Banner/Banner.tsx
index 13fb6196dde..2cb0e72c813 100644
--- a/packages/react/src/Banner/Banner.tsx
+++ b/packages/react/src/Banner/Banner.tsx
@@ -1,10 +1,11 @@
 import cx from 'clsx'
-import React, {createContext, useContext, useEffect, useId, useMemo} from 'react'
+import React, {useEffect} from 'react'
 import styled from 'styled-components'
 import {AlertIcon, InfoIcon, StopIcon, CheckCircleIcon, XIcon} from '@primer/octicons-react'
 import {Button, IconButton} from '../Button'
 import {get} from '../constants'
 import {VisuallyHidden} from '../internal/components/VisuallyHidden'
+import {useMergedRefs} from '../internal/hooks/useMergedRefs'
 
 type BannerVariant = 'critical' | 'info' | 'success' | 'upsell' | 'warning'
 
@@ -64,74 +65,84 @@ const iconForVariant: Record<BannerVariant, React.ReactNode> = {
   warning: <AlertIcon />,
 }
 
+const labels: Record<BannerVariant, string> = {
+  critical: 'Critical',
+  info: 'Information',
+  success: 'Success',
+  upsell: 'Recommendation',
+  warning: 'Warning',
+}
+
 export const Banner = React.forwardRef<HTMLElement, BannerProps>(function Banner(
   {children, description, hideTitle, icon, onDismiss, primaryAction, secondaryAction, title, variant = 'info', ...rest},
-  ref,
+  forwardRef,
 ) {
-  const titleId = useId()
-  const value = useMemo(() => {
-    return {
-      titleId,
-    }
-  }, [titleId])
   const dismissible = variant !== 'critical' && onDismiss
   const hasActions = primaryAction || secondaryAction
+  const bannerRef = React.useRef<HTMLElement>(null)
+  const ref = useMergedRefs(forwardRef, bannerRef)
 
   if (__DEV__) {
-    // Note: __DEV__ will make it so that this hook is consistently called, or
-    // not called, depending on environment
+    // This hook is called consistently depending on the environment
     // eslint-disable-next-line react-hooks/rules-of-hooks
     useEffect(() => {
-      const title = document.getElementById(titleId)
-      if (!title) {
+      if (title) {
+        return
+      }
+
+      const {current: banner} = bannerRef
+      if (!banner) {
+        return
+      }
+
+      const hasTitle = banner.querySelector('[data-banner-title]')
+      if (!hasTitle) {
         throw new Error(
-          'The Banner component requires a title to be provided as the `title` prop or through `Banner.Title`',
+          'Expected a title to be provided to the <Banner> component with the `title` prop or through `<Banner.Title>` but no title was found',
         )
       }
-    }, [titleId])
+    }, [title])
   }
 
   return (
-    <BannerContext.Provider value={value}>
-      <StyledBanner
-        {...rest}
-        aria-labelledby={titleId}
-        as="section"
-        data-dismissible={onDismiss ? '' : undefined}
-        data-title-hidden={hideTitle ? '' : undefined}
-        data-variant={variant}
-        tabIndex={-1}
-        ref={ref}
-      >
-        <style>{BannerContainerQuery}</style>
-        <div className="BannerIcon">{icon && variant === 'info' ? icon : iconForVariant[variant]}</div>
-        <div className="BannerContainer">
-          <div className="BannerContent">
-            {title ? (
-              hideTitle ? (
-                <VisuallyHidden>
-                  <BannerTitle>{title}</BannerTitle>
-                </VisuallyHidden>
-              ) : (
+    <StyledBanner
+      {...rest}
+      aria-label={labels[variant]}
+      as="section"
+      data-dismissible={onDismiss ? '' : undefined}
+      data-title-hidden={hideTitle ? '' : undefined}
+      data-variant={variant}
+      tabIndex={-1}
+      ref={ref}
+    >
+      <style>{BannerContainerQuery}</style>
+      <div className="BannerIcon">{icon && variant === 'info' ? icon : iconForVariant[variant]}</div>
+      <div className="BannerContainer">
+        <div className="BannerContent">
+          {title ? (
+            hideTitle ? (
+              <VisuallyHidden>
                 <BannerTitle>{title}</BannerTitle>
-              )
-            ) : null}
-            {description ? <BannerDescription>{description}</BannerDescription> : null}
-            {children}
-          </div>
-          {hasActions ? <BannerActions primaryAction={primaryAction} secondaryAction={secondaryAction} /> : null}
+              </VisuallyHidden>
+            ) : (
+              <BannerTitle>{title}</BannerTitle>
+            )
+          ) : null}
+          {description ? <BannerDescription>{description}</BannerDescription> : null}
+          {children}
         </div>
-        {dismissible ? (
-          <IconButton
-            aria-label="Dismiss banner"
-            onClick={onDismiss}
-            className="BannerDismiss"
-            icon={XIcon}
-            variant="invisible"
-          />
-        ) : null}
-      </StyledBanner>
-    </BannerContext.Provider>
+        {hasActions ? <BannerActions primaryAction={primaryAction} secondaryAction={secondaryAction} /> : null}
+      </div>
+      {dismissible ? (
+        <IconButton
+          aria-label="Dismiss banner"
+          onClick={onDismiss}
+          className="BannerDismiss"
+          icon={XIcon}
+          variant="invisible"
+        />
+      ) : null}
+    </StyledBanner>
   )
 })
 
@@ -342,9 +353,8 @@ export type BannerTitleProps<As extends HeadingElement> = {
 
 export function BannerTitle<As extends HeadingElement>(props: BannerTitleProps<As>) {
   const {as: Heading = 'h2', className, children, ...rest} = props
-  const banner = useBanner()
   return (
-    <Heading {...rest} id={banner.titleId} className={cx('BannerTitle', className)}>
+    <Heading {...rest} className={cx('BannerTitle', className)} data-banner-title="">
       {children}
     </Heading>
   )
@@ -399,14 +409,3 @@ export function BannerSecondaryAction({children, className, ...rest}: BannerSeco
     </Button>
   )
 }
-
-type BannerContextValue = {titleId: string}
-const BannerContext = createContext<BannerContextValue | null>(null)
-
-function useBanner(): BannerContextValue {
-  const value = useContext(BannerContext)
-  if (value) {
-    return value
-  }
-  throw new Error('Component must be used within a <Banner> component')
-}
diff --git a/packages/react/src/Banner/__snapshots__/Banner.test.tsx.snap b/packages/react/src/Banner/__snapshots__/Banner.test.tsx.snap
index 3a1a9768fbc..34992a57706 100644
--- a/packages/react/src/Banner/__snapshots__/Banner.test.tsx.snap
+++ b/packages/react/src/Banner/__snapshots__/Banner.test.tsx.snap
@@ -1,3 +1,3 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`Banner should throw an error if no title is provided 1`] = `"The Banner component requires a title to be provided as the \`title\` prop or through \`Banner.Title\`"`;
+exports[`Banner should throw an error if no title is provided 1`] = `"Expected a title to be provided to the <Banner> component with the \`title\` prop or through \`<Banner.Title>\` but no title was found"`;
diff --git a/packages/react/src/internal/hooks/useMergedRefs.ts b/packages/react/src/internal/hooks/useMergedRefs.ts
new file mode 100644
index 00000000000..7c67f20fadc
--- /dev/null
+++ b/packages/react/src/internal/hooks/useMergedRefs.ts
@@ -0,0 +1,16 @@
+import {useCallback} from 'react'
+
+export function useMergedRefs<T>(
+  ...refs: Array<React.MutableRefObject<T> | React.ForwardedRef<T> | React.RefCallback<T>>
+): React.RefCallback<T> {
+  return useCallback((instance: T) => {
+    for (const ref of refs) {
+      if (typeof ref === 'function') {
+        ref(instance)
+      } else if (ref) {
+        ref.current = instance
+      }
+    }
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, refs)
+}

From 6e1c409bf733d7ae504cf7cf1a7727fb02ddeaf9 Mon Sep 17 00:00:00 2001
From: Josh Black <joshblack@users.noreply.github.com>
Date: Fri, 26 Apr 2024 17:02:55 -0500
Subject: [PATCH 2/6] chore: add changeset, update config to exclude
 codesandbox

---
 .changeset/cold-starfishes-shout.md | 5 +++++
 .changeset/config.json              | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)
 create mode 100644 .changeset/cold-starfishes-shout.md

diff --git a/.changeset/cold-starfishes-shout.md b/.changeset/cold-starfishes-shout.md
new file mode 100644
index 00000000000..ce8c32e1b71
--- /dev/null
+++ b/.changeset/cold-starfishes-shout.md
@@ -0,0 +1,5 @@
+---
+'@primer/react': minor
+---
+
+Update Banner to use an explicit aria-label instead of being labelled by Banner title
diff --git a/.changeset/config.json b/.changeset/config.json
index 7c4d724582b..d826f57cc65 100644
--- a/.changeset/config.json
+++ b/.changeset/config.json
@@ -6,5 +6,5 @@
   "access": "public",
   "baseBranch": "main",
   "updateInternalDependencies": "patch",
-  "ignore": ["docs", "example-*"]
+  "ignore": ["docs", "example-*", "codesandbox"]
 }

From e89bd6a17324e5549b9254cee325a2f9cfa0556e Mon Sep 17 00:00:00 2001
From: Josh Black <joshblack@users.noreply.github.com>
Date: Mon, 29 Apr 2024 15:42:04 -0500
Subject: [PATCH 3/6] feat: add aria-label to Banner

---
 packages/react/src/Banner/Banner.docs.json |  5 +++++
 packages/react/src/Banner/Banner.test.tsx  |  5 +++++
 packages/react/src/Banner/Banner.tsx       | 22 ++++++++++++++++++++--
 3 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/packages/react/src/Banner/Banner.docs.json b/packages/react/src/Banner/Banner.docs.json
index a12ab01b984..fa4570ec9b0 100644
--- a/packages/react/src/Banner/Banner.docs.json
+++ b/packages/react/src/Banner/Banner.docs.json
@@ -6,6 +6,11 @@
   "importPath": "@primer/react/experimental",
   "stories": [],
   "props": [
+    {
+      "name": "aria-label",
+      "type": "string",
+      "description": "Provide an optional label to override the default name for the Banner landmark region"
+    },
     {
       "name": "description",
       "type": "React.ReactNode",
diff --git a/packages/react/src/Banner/Banner.test.tsx b/packages/react/src/Banner/Banner.test.tsx
index eead7e8ed38..dee0b35ca01 100644
--- a/packages/react/src/Banner/Banner.test.tsx
+++ b/packages/react/src/Banner/Banner.test.tsx
@@ -59,6 +59,11 @@ describe('Banner', () => {
     expect(screen.getByRole('region')).toEqual(screen.getByLabelText('Warning'))
   })
 
+  it('should support the `aria-label` prop to override the default label for the landmark', () => {
+    render(<Banner aria-label="test" title="test" variant="warning" />)
+    expect(screen.getByRole('region')).toHaveAttribute('aria-label', 'test')
+  });
+
   it('should default the title to a h2', () => {
     render(<Banner title="test" />)
     expect(screen.getByRole('heading', {level: 2})).toBeInTheDocument()
diff --git a/packages/react/src/Banner/Banner.tsx b/packages/react/src/Banner/Banner.tsx
index 2cb0e72c813..8f61ba1a1d1 100644
--- a/packages/react/src/Banner/Banner.tsx
+++ b/packages/react/src/Banner/Banner.tsx
@@ -10,6 +10,12 @@ import {useMergedRefs} from '../internal/hooks/useMergedRefs'
 type BannerVariant = 'critical' | 'info' | 'success' | 'upsell' | 'warning'
 
 export type BannerProps = React.ComponentPropsWithoutRef<'section'> & {
+  /**
+   * Provide an optional label to override the default name for the Banner
+   * landmark region
+   */
+  'aria-label'?: string
+
   /**
    * Provide an optional description for the Banner. This should provide
    * supplemental information about the Banner
@@ -74,7 +80,19 @@ const labels: Record<BannerVariant, string> = {
 }
 
 export const Banner = React.forwardRef<HTMLElement, BannerProps>(function Banner(
-  {children, description, hideTitle, icon, onDismiss, primaryAction, secondaryAction, title, variant = 'info', ...rest},
+  {
+    'aria-label': label,
+    children,
+    description,
+    hideTitle,
+    icon,
+    onDismiss,
+    primaryAction,
+    secondaryAction,
+    title,
+    variant = 'info',
+    ...rest
+  },
   forwardRef,
 ) {
   const dismissible = variant !== 'critical' && onDismiss
@@ -107,7 +125,7 @@ export const Banner = React.forwardRef<HTMLElement, BannerProps>(function Banner
   return (
     <StyledBanner
       {...rest}
-      aria-label={labels[variant]}
+      aria-label={label ?? labels[variant]}
       as="section"
       data-dismissible={onDismiss ? '' : undefined}
       data-title-hidden={hideTitle ? '' : undefined}

From ff471fa4c72b5de7861066ef86934e7b576da9c9 Mon Sep 17 00:00:00 2001
From: Josh Black <joshblack@github.com>
Date: Tue, 30 Apr 2024 11:12:55 -0500
Subject: [PATCH 4/6] Update packages/react/src/Banner/Banner.test.tsx

Co-authored-by: Kate Higa <16447748+khiga8@users.noreply.github.com>
---
 packages/react/src/Banner/Banner.test.tsx | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/react/src/Banner/Banner.test.tsx b/packages/react/src/Banner/Banner.test.tsx
index dee0b35ca01..592844d6ca4 100644
--- a/packages/react/src/Banner/Banner.test.tsx
+++ b/packages/react/src/Banner/Banner.test.tsx
@@ -60,7 +60,7 @@ describe('Banner', () => {
   })
 
   it('should support the `aria-label` prop to override the default label for the landmark', () => {
-    render(<Banner aria-label="test" title="test" variant="warning" />)
+    render(<Banner aria-label="Test" title="test" variant="warning" />)
     expect(screen.getByRole('region')).toHaveAttribute('aria-label', 'test')
   });
 

From 7e133507482550cdc5f009a9dc24afaa3e2f5709 Mon Sep 17 00:00:00 2001
From: Josh Black <joshblack@github.com>
Date: Tue, 30 Apr 2024 11:13:02 -0500
Subject: [PATCH 5/6] Update packages/react/src/Banner/Banner.test.tsx

Co-authored-by: Kate Higa <16447748+khiga8@users.noreply.github.com>
---
 packages/react/src/Banner/Banner.test.tsx | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/react/src/Banner/Banner.test.tsx b/packages/react/src/Banner/Banner.test.tsx
index 592844d6ca4..88cc75d7ddc 100644
--- a/packages/react/src/Banner/Banner.test.tsx
+++ b/packages/react/src/Banner/Banner.test.tsx
@@ -62,7 +62,7 @@ describe('Banner', () => {
   it('should support the `aria-label` prop to override the default label for the landmark', () => {
     render(<Banner aria-label="Test" title="test" variant="warning" />)
     expect(screen.getByRole('region')).toHaveAttribute('aria-label', 'test')
-  });
+  })
 
   it('should default the title to a h2', () => {
     render(<Banner title="test" />)

From 663af56977101e4a17bb172258ae5fab70f92a2a Mon Sep 17 00:00:00 2001
From: Josh Black <joshblack@users.noreply.github.com>
Date: Fri, 3 May 2024 15:59:00 -0500
Subject: [PATCH 6/6] test: update test label with aria-label

---
 packages/react/src/Banner/Banner.test.tsx | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/react/src/Banner/Banner.test.tsx b/packages/react/src/Banner/Banner.test.tsx
index 88cc75d7ddc..0664cc0c692 100644
--- a/packages/react/src/Banner/Banner.test.tsx
+++ b/packages/react/src/Banner/Banner.test.tsx
@@ -61,7 +61,7 @@ describe('Banner', () => {
 
   it('should support the `aria-label` prop to override the default label for the landmark', () => {
     render(<Banner aria-label="Test" title="test" variant="warning" />)
-    expect(screen.getByRole('region')).toHaveAttribute('aria-label', 'test')
+    expect(screen.getByRole('region')).toHaveAttribute('aria-label', 'Test')
   })
 
   it('should default the title to a h2', () => {