chore: Resolve todo items (#3647)

Author: just-boris

src/__a11y__/to-validate-a11y.ts

@@ -75,7 +75,7 @@ async function toValidateA11y(this: jest.MatcherUtils, element: HTMLElement) {
     );
     // TODO: remove polyfill with es2019 support
     const flattenAxeViolations = flatMap(axeViolations, violation =>
-      violation.nodes.map(node => `${node.failureSummary} [${violation.id}]`)
+      violation.nodes.map(node => `[${violation.id}] ${node.failureSummary} Selector: ${node.target}`)
     );
 
     return compact(

src/__tests__/functional-tests/test-a11y-validator.test.tsx

@@ -42,7 +42,7 @@ describe('a11y validator', () => {
     );
     await expect(expect(container).toValidateA11y()).rejects.toThrow(
       new Error(
-        'Expected HTML to be valid but had the following errors:\nHTML validation\n1. Redundant "for" attribute [no-redundant-for]\nAxe checks\n1. Fix all of the following:\n  ARIA attribute element ID does not exist on the page: aria-labelledby="non-exist-id" [aria-valid-attr-value]'
+        'Expected HTML to be valid but had the following errors:\nHTML validation\n1. Redundant "for" attribute [no-redundant-for]\nAxe checks\n1. [aria-valid-attr-value] Fix all of the following:\n  ARIA attribute element ID does not exist on the page: aria-labelledby="non-exist-id" Selector: #my-field'
       )
     );
   });

src/app-layout/__tests__/main.test.tsx

@@ -128,7 +128,7 @@ describeEachAppLayout({ themes: ['classic', 'refresh-toolbar'], sizes: ['desktop
       await waitFor(() => expect(wrapper.getElement()).toHaveStyle({ minBlockSize: 'calc(100vh - 45px)' }));
     });
 
-    // TODO Enable after fixing disableBodyScroll
+    // disableBodyScroll is not supported in this version
     (theme === 'refresh-toolbar' ? test.skip : test)(
       'should set height instead of min-height when the body scroll is disabled',
       () => {
@@ -139,8 +139,7 @@ describeEachAppLayout({ themes: ['classic', 'refresh-toolbar'], sizes: ['desktop
     );
   });
 
-  // TODO Enable after fixing 'Distinguish landmarks on page'
-  (size === 'mobile' && theme !== 'refresh-toolbar' ? test : test.skip)('a11y', async () => {
+  (size === 'mobile' ? test : test.skip)('a11y', async () => {
     const { container } = renderComponent(
       <AppLayout
         navigationOpen={true}
@@ -152,11 +151,9 @@ describeEachAppLayout({ themes: ['classic', 'refresh-toolbar'], sizes: ['desktop
         breadcrumbs={<div></div>}
         splitPanel={<div></div>}
         ariaLabels={{
-          // notifications?: string;
-          // navigation?: string;
+          drawers: 'Drawers',
           navigationToggle: 'Open navigation',
           navigationClose: 'Close navigation',
-          // tools?: string;
           toolsToggle: 'Open tools',
           toolsClose: 'Close tools',
         }}

src/app-layout/visual-refresh-toolbar/toolbar/index.tsx

@@ -63,15 +63,7 @@ export function AppLayoutToolbarImplementation({
   // not testable in a single-version setup
   toolbarProps = {},
 }: AppLayoutToolbarImplementationProps) {
-  const {
-    breadcrumbs,
-    discoveredBreadcrumbs,
-    verticalOffsets,
-    isMobile,
-    toolbarState,
-    setToolbarState,
-    setToolbarHeight,
-  } = appLayoutInternals;
+  const { breadcrumbs, discoveredBreadcrumbs, verticalOffsets, isMobile, setToolbarHeight } = appLayoutInternals;
   const {
     ariaLabels,
     activeDrawerId,
@@ -93,8 +85,6 @@ export function AppLayoutToolbarImplementation({
     expandedDrawerId,
     setExpandedDrawerId,
   } = toolbarProps;
-  // TODO: expose configuration property
-  const pinnedToolbar = true;
   const drawerExpandedMode = !!expandedDrawerId;
   const ref = useRef<HTMLElement>(null);
   useResizeObserver(ref, entry => setToolbarHeight(entry.borderBoxHeight));
@@ -106,31 +96,6 @@ export function AppLayoutToolbarImplementation({
     // eslint-disable-next-line react-hooks/exhaustive-deps
   }, []);
 
-  useEffect(() => {
-    let lastScrollY = window.scrollY;
-
-    /* istanbul ignore next not testable in JSDOM */
-    const updateScrollDirection = () => {
-      if (pinnedToolbar) {
-        setToolbarState('show');
-        return;
-      }
-      const scrollY = window.scrollY;
-      // 80 is an arbitrary number to have a pause before the toolbar scrolls out of view at the top of the page
-      const direction = scrollY > lastScrollY && scrollY > 80 ? 'hide' : 'show';
-      // 2 as a buffer to avoid mistaking minor accidental mouse moves as scroll
-      if (direction !== toolbarState && (scrollY - lastScrollY > 2 || scrollY - lastScrollY < -2)) {
-        setToolbarState(direction);
-      }
-      lastScrollY = scrollY > 0 ? scrollY : 0;
-    };
-
-    window.addEventListener('scroll', updateScrollDirection);
-    return () => {
-      window.removeEventListener('scroll', updateScrollDirection);
-    };
-  }, [pinnedToolbar, setToolbarState, toolbarState]);
-
   const anyPanelOpenInMobile = !!isMobile && (!!activeDrawerId || (!!navigationOpen && !!hasNavigation));
   useEffect(() => {
     if (anyPanelOpenInMobile) {
@@ -143,7 +108,6 @@ export function AppLayoutToolbarImplementation({
     };
   }, [anyPanelOpenInMobile]);
 
-  const toolbarHidden = toolbarState === 'hide' && !pinnedToolbar;
   const navLandmarkAttributes = navigationOpen
     ? { role: 'presentation' }
     : { role: 'navigation', 'aria-label': ariaLabels?.navigation };
@@ -153,10 +117,9 @@ export function AppLayoutToolbarImplementation({
       ref={ref}
       className={clsx(styles['universal-toolbar'], testutilStyles.toolbar, {
         [testutilStyles['mobile-bar']]: isMobile,
-        [styles['toolbar-hidden']]: toolbarHidden,
       })}
       style={{
-        insetBlockStart: toolbarHidden ? '-60px' : verticalOffsets.toolbar,
+        insetBlockStart: verticalOffsets.toolbar,
       }}
     >
       <div className={styles['toolbar-container']}>

src/app-layout/visual-refresh-toolbar/toolbar/styles.scss

@@ -26,10 +26,6 @@
     inset-block-start: 0px;
   }
 
-  &.toolbar-hidden {
-    opacity: 0;
-  }
-
   > .toolbar-container {
     block-size: 100%;
     align-items: center;