Remove #AppFrameMainContent link and update SkipToContent link to tar…

…get #AppFrameMain instead (#3912)

* Remove #AppFrameMainContent link and update SkipToContent link to target #AppFrameMain instead
* Allow focus to be placed on a focusable element in <main> instead of invisible link
* Remove unnecessary anchor and add preventDefault when custom skipToContentTarget is clicked

UNRELEASED.md

@@ -8,6 +8,7 @@ Use [the changelog guidelines](https://git.io/polaris-changelog-guidelines) to f
 
 - Added `focus-visible` polyfill and default styles ([#3695](https://github.com/Shopify/polaris-react/pull/3695))
 - Added `removeUnderline` prop to `Button` to remove underline when `plain` and `monochrome` are true([#3998](https://github.com/Shopify/polaris-react/))pull/3998)
+- Removed `#AppFrameMainContent` link and updated SkipToContent link to target `#AppFrameMain` instead ([#3912](https://github.com/Shopify/polaris-react/pull/3912))
 
 ### Bug fixes
 

src/components/Frame/Frame.tsx

@@ -1,4 +1,4 @@
-import React, {PureComponent, createRef} from 'react';
+import React, {PureComponent, createRef, MouseEvent} from 'react';
 import {MobileCancelMajor} from '@shopify/polaris-icons';
 import {durationSlow} from '@shopify/polaris-tokens';
 import {CSSTransition} from 'react-transition-group';
@@ -62,9 +62,6 @@ interface State {
 const GLOBAL_RIBBON_CUSTOM_PROPERTY = '--global-ribbon-height';
 
 const APP_FRAME_MAIN = 'AppFrameMain';
-
-const APP_FRAME_MAIN_ANCHOR_TARGET = 'AppFrameMainContent';
-
 const APP_FRAME_NAV = 'AppFrameNav';
 const APP_FRAME_TOP_BAR = 'AppFrameTopBar';
 const APP_FRAME_LOADING_BAR = 'AppFrameLoadingBar';
@@ -81,8 +78,6 @@ class FrameInner extends PureComponent<CombinedProps, State> {
   private contextualSaveBar: ContextualSaveBarProps | null = null;
   private globalRibbonContainer: HTMLDivElement | null = null;
   private navigationNode = createRef<HTMLDivElement>();
-  private skipToMainContentTargetNode =
-    this.props.skipToContentTarget || createRef<HTMLAnchorElement>();
 
   componentDidMount() {
     this.handleResize();
@@ -216,7 +211,7 @@ class FrameInner extends PureComponent<CombinedProps, State> {
 
     const skipTarget = skipToContentTarget?.current
       ? skipToContentTarget.current.id
-      : APP_FRAME_MAIN_ANCHOR_TARGET;
+      : APP_FRAME_MAIN;
 
     const skipMarkup = (
       <div className={skipClassName}>
@@ -252,15 +247,6 @@ class FrameInner extends PureComponent<CombinedProps, State> {
         />
       ) : null;
 
-    const skipToMainContentTarget = skipToContentTarget ? null : (
-      // eslint-disable-next-line jsx-a11y/anchor-is-valid
-      <a
-        id={APP_FRAME_MAIN_ANCHOR_TARGET}
-        ref={this.skipToMainContentTargetNode}
-        tabIndex={-1}
-      />
-    );
-
     const context = {
       showToast: this.showToast,
       hideToast: this.hideToast,
@@ -288,7 +274,6 @@ class FrameInner extends PureComponent<CombinedProps, State> {
             id={APP_FRAME_MAIN}
             data-has-global-ribbon={Boolean(globalRibbon)}
           >
-            {skipToMainContentTarget}
             <div className={styles.Content}>{children}</div>
           </main>
           <ToastManager toastMessages={toastMessages} />
@@ -379,9 +364,12 @@ class FrameInner extends PureComponent<CombinedProps, State> {
     this.setState({skipFocused: false});
   };
 
-  private handleClick = () => {
-    this.skipToMainContentTargetNode.current &&
-      this.skipToMainContentTargetNode.current.focus();
+  private handleClick = (event: MouseEvent<HTMLAnchorElement>) => {
+    const {skipToContentTarget} = this.props;
+    if (skipToContentTarget && skipToContentTarget.current) {
+      skipToContentTarget.current.focus();
+      event?.preventDefault();
+    }
   };
 
   private handleNavigationDismiss = () => {

src/components/Frame/tests/Frame.test.tsx

@@ -52,13 +52,12 @@ describe('<Frame />', () => {
       expect(skipToContentLinkText).toStrictEqual('Skip to content');
     });
 
-    it('sets focus to the main content target anchor element when the skip to content link is clicked', () => {
+    it('targets the main container element by default', () => {
       const frame = mountWithApp(<Frame />);
       const skipLink = frame.find('a', {children: 'Skip to content'});
 
-      skipLink!.trigger('onClick');
-      expect(document.activeElement).toBe(
-        frame.find('a', {id: 'AppFrameMainContent'})!.domNode,
+      expect(skipLink!.domNode!.getAttribute('href')).toBe(
+        `#${frame!.find('main')!.domNode!.id}`,
       );
     });