From 940d20e115fa442d8f92a30535907a05ca27e71a Mon Sep 17 00:00:00 2001
From: Tobias Skarhed <1438972+tskarhed@users.noreply.github.com>
Date: Mon, 11 Mar 2024 11:43:22 +0100
Subject: [PATCH] Accessibility: Improve landmark markup (#83576)

* Landmark: main

* Landmark: add header

* Submenu: Move conditional display up

* NewsPanel: use h3 as the article label

* Use title for article id

* Update test showing a false positive

* DashboardPage: Expect submenu to not be shown
---
 public/app/core/components/AppChrome/AppChrome.tsx   | 12 ++++++------
 .../dashboard/components/SubMenu/SubMenu.tsx         |  4 ----
 .../dashboard/containers/DashboardPage.test.tsx      |  8 +++++---
 .../features/dashboard/containers/DashboardPage.tsx  |  2 +-
 public/app/plugins/panel/news/component/News.tsx     |  7 +++++--
 5 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/public/app/core/components/AppChrome/AppChrome.tsx b/public/app/core/components/AppChrome/AppChrome.tsx
index b47c655188b..e4bd8b80903 100644
--- a/public/app/core/components/AppChrome/AppChrome.tsx
+++ b/public/app/core/components/AppChrome/AppChrome.tsx
@@ -82,7 +82,7 @@ export function AppChrome({ children }: Props) {
           <LinkButton className={styles.skipLink} href="#pageContent">
             Skip to main content
           </LinkButton>
-          <div className={cx(styles.topNav)}>
+          <header className={cx(styles.topNav)}>
             {!searchBarHidden && <TopSearchBar />}
             <NavToolbar
               searchBarHidden={searchBarHidden}
@@ -93,19 +93,19 @@ export function AppChrome({ children }: Props) {
               onToggleMegaMenu={handleMegaMenu}
               onToggleKioskMode={chrome.onToggleKioskMode}
             />
-          </div>
+          </header>
         </>
       )}
-      <main className={contentClass}>
+      <div className={contentClass}>
         <div className={styles.panes}>
           {!state.chromeless && state.megaMenuDocked && state.megaMenuOpen && (
             <MegaMenu className={styles.dockedMegaMenu} onClose={() => chrome.setMegaMenuOpen(false)} />
           )}
-          <div className={styles.pageContainer} id="pageContent">
+          <main className={styles.pageContainer} id="pageContent">
             {children}
-          </div>
+          </main>
         </div>
-      </main>
+      </div>
       {!state.chromeless && !state.megaMenuDocked && <AppChromeMenu />}
       {!state.chromeless && <CommandPalette />}
       {shouldShowReturnToPrevious && state.returnToPrevious && (
diff --git a/public/app/features/dashboard/components/SubMenu/SubMenu.tsx b/public/app/features/dashboard/components/SubMenu/SubMenu.tsx
index 16a5742e9d4..85e77fc787a 100644
--- a/public/app/features/dashboard/components/SubMenu/SubMenu.tsx
+++ b/public/app/features/dashboard/components/SubMenu/SubMenu.tsx
@@ -51,10 +51,6 @@ class SubMenuUnConnected extends PureComponent<Props> {
 
     const styles = getStyles(theme);
 
-    if (!dashboard.isSubMenuVisible()) {
-      return null;
-    }
-
     const readOnlyVariables = dashboard.meta.isSnapshot ?? false;
 
     return (
diff --git a/public/app/features/dashboard/containers/DashboardPage.test.tsx b/public/app/features/dashboard/containers/DashboardPage.test.tsx
index 24486c53e2a..004f1d951aa 100644
--- a/public/app/features/dashboard/containers/DashboardPage.test.tsx
+++ b/public/app/features/dashboard/containers/DashboardPage.test.tsx
@@ -306,10 +306,12 @@ describe('DashboardPage', () => {
   });
 
   describe('No kiosk mode tv', () => {
-    it('should render dashboard page toolbar and submenu', async () => {
-      setup({ dashboard: getTestDashboard() });
+    it('should render dashboard page toolbar with no submenu', async () => {
+      setup({
+        dashboard: getTestDashboard(),
+      });
       expect(await screen.findAllByTestId(selectors.pages.Dashboard.DashNav.navV2)).toHaveLength(1);
-      expect(screen.getAllByLabelText(selectors.pages.Dashboard.SubMenu.submenu)).toHaveLength(1);
+      expect(screen.queryAllByLabelText(selectors.pages.Dashboard.SubMenu.submenu)).toHaveLength(0);
     });
   });
 
diff --git a/public/app/features/dashboard/containers/DashboardPage.tsx b/public/app/features/dashboard/containers/DashboardPage.tsx
index 71d44e4e993..188c10d9d46 100644
--- a/public/app/features/dashboard/containers/DashboardPage.tsx
+++ b/public/app/features/dashboard/containers/DashboardPage.tsx
@@ -302,7 +302,7 @@ export class UnthemedDashboardPage extends PureComponent<Props, State> {
     }
 
     const inspectPanel = this.getInspectPanel();
-    const showSubMenu = !editPanel && !kioskMode && !this.props.queryParams.editview;
+    const showSubMenu = !editPanel && !kioskMode && !this.props.queryParams.editview && dashboard.isSubMenuVisible();
 
     const showToolbar = kioskMode !== KioskMode.Full && !queryParams.editview;
 
diff --git a/public/app/plugins/panel/news/component/News.tsx b/public/app/plugins/panel/news/component/News.tsx
index 39c95ff82d1..c47f1d1ade7 100644
--- a/public/app/plugins/panel/news/component/News.tsx
+++ b/public/app/plugins/panel/news/component/News.tsx
@@ -19,9 +19,10 @@ function NewsComponent({ width, showImage, data, index }: NewsItemProps) {
   const styles = useStyles2(getStyles);
   const useWideLayout = width > 600;
   const newsItem = data.get(index);
+  const titleId = encodeURI(newsItem.title);
 
   return (
-    <article className={cx(styles.item, useWideLayout && styles.itemWide)}>
+    <article aria-labelledby={titleId} className={cx(styles.item, useWideLayout && styles.itemWide)}>
       {showImage && newsItem.ogImage && (
         <a
           tabIndex={-1}
@@ -39,7 +40,9 @@ function NewsComponent({ width, showImage, data, index }: NewsItemProps) {
           {dateTimeFormat(newsItem.date, { format: 'MMM DD' })}{' '}
         </time>
         <a className={styles.link} href={textUtil.sanitizeUrl(newsItem.link)} target="_blank" rel="noopener noreferrer">
-          <h3 className={styles.title}>{newsItem.title}</h3>
+          <h3 className={styles.title} id={titleId}>
+            {newsItem.title}
+          </h3>
         </a>
         <div className={styles.content} dangerouslySetInnerHTML={{ __html: textUtil.sanitize(newsItem.content) }} />
       </div>