Add Scrollable container to Frame content

Shopify · Apr 30, 2024 · 6262b78 · 6262b78
1 parent e369c17
commit 6262b78
Show file tree

Hide file tree

Showing 4 changed files with 52 additions and 36 deletions.
diff --git a/.changeset/little-plums-try.md b/.changeset/little-plums-try.md
@@ -0,0 +1,5 @@
+---
+'@shopify/polaris': patch
+---
+
+Added offset width to reframe `Frame`, passed reframe scroll container to sticky manager in `AppProvider`, fixed "Skip to content" z-index
diff --git a/polaris-react/src/components/AppProvider/AppProvider.tsx b/polaris-react/src/components/AppProvider/AppProvider.tsx
@@ -35,6 +35,7 @@ const MAX_SCROLLBAR_WIDTH = 20;
 const SCROLLBAR_TEST_ELEMENT_PARENT_SIZE = 30;
 const SCROLLBAR_TEST_ELEMENT_CHILD_SIZE =
   SCROLLBAR_TEST_ELEMENT_PARENT_SIZE + 10;
+const APP_FRAME_SCROLLABLE = 'AppFrameScollable';
 
 function measureScrollbars() {
   const parentEl = document.createElement('div');
@@ -105,7 +106,15 @@ export class AppProvider extends Component<AppProviderProps, State> {
 
   componentDidMount() {
     if (document != null) {
-      this.stickyManager.setContainer(document);
+      if (!this.props.features?.dynamicTopBarAndReframe) {
+        this.stickyManager.setContainer(document);
+      } else {
+        const scrollContainerElement =
+          document.getElementById(APP_FRAME_SCROLLABLE);
+
+        this.stickyManager.setContainer(scrollContainerElement ?? document);
+      }
+
       this.setBodyStyles();
       this.setRootAttributes();
 

diff --git a/polaris-react/src/components/Frame/Frame.module.css b/polaris-react/src/components/Frame/Frame.module.css
@@ -28,7 +28,7 @@
   /* stylelint-disable-next-line polaris/media-queries/polaris/media-query-allowed-list -- custom breakpoint */
   @media screen and (min-width: 1200px) {
     /* stylelint-disable-next-line polaris/conventions/polaris/custom-property-allowed-list -- private token from component */
-    width: calc(100% - var(--pc-sidebar-width));
+    width: calc(100% - var(--pc-sidebar-width) - var(--pc-frame-offset));
   }
 
   @media (prefers-reduced-motion) {
@@ -290,27 +290,12 @@
 }
 
 .Content-TopBarAndReframe {
-  overflow-y: auto;
-  /* stylelint-disable-next-line polaris/conventions/polaris/custom-property-allowed-list -- top bar global space */
-  margin-bottom: var(--pg-top-bar-height);
-  margin-right: var(--p-space-050);
-
-  .hasSidebar & {
-    /* Sidebar breakpoint is 1200px */
-    /* stylelint-disable-next-line polaris/media-queries/polaris/media-query-allowed-list -- custom breakpoint */
-    @media screen and (min-width: 1200px) {
-      margin-right: unset;
-    }
-  }
-}
-
-.ScrollbarSafeArea-TopBarAndReframe {
   @media (--p-breakpoints-md-up) {
     transition: width var(--p-motion-duration-250) var(--p-motion-ease);
     /* stylelint-disable -- polaris/conventions/polaris/custom-property-allowed-list -- Polaris component custom properties */
     width: calc(
       100vw - var(--pg-navigation-width) -
-        var(--pc-app-provider-scrollbar-width) - var(--p-space-150)
+        var(--pc-app-provider-scrollbar-width)
     );
     /* stylelint-enable -- polaris/conventions/polaris/custom-property-allowed-list */
 
@@ -328,9 +313,9 @@
       /* stylelint-disable-next-line polaris/conventions/polaris/custom-property-allowed-list -- private token from component */
       width: calc(
         100vw - var(--pg-navigation-width) -
-          var(--pc-app-provider-scrollbar-width) - var(--p-space-150) -
-          var(--pc-sidebar-width)
+          var(--pc-app-provider-scrollbar-width) - var(--pc-sidebar-width)
       );
+      margin-right: unset;
     }
 
     @media (prefers-reduced-motion) {
@@ -390,9 +375,9 @@
 
 .Skip {
   /* stylelint-disable-next-line -- Polaris component custom properties */
-  --pc-frame-skip-vertical-offset: 10px;
+  --pc-frame-skip-vertical-offset: 11px;
   position: fixed;
-  z-index: var(--p-z-index-9);
+  z-index: var(--p-z-index-10);
   /* stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY */
   top: var(--pc-frame-skip-vertical-offset);
   /* stylelint-disable-next-line -- generated by polaris-migrator DO NOT COPY */
@@ -436,3 +421,9 @@
     background-color: var(--p-color-bg);
   }
 }
+
+.Scrollable {
+  width: 100%;
+  /* stylelint-disable-next-line polaris/conventions/polaris/custom-property-allowed-list -- top bar global space */
+  height: calc(100% - var(--pg-top-bar-height));
+}
diff --git a/polaris-react/src/components/Frame/Frame.tsx b/polaris-react/src/components/Frame/Frame.tsx
@@ -13,6 +13,7 @@ import {EventListener} from '../EventListener';
 import {Backdrop} from '../Backdrop';
 import {Text} from '../Text';
 import {TrapFocus} from '../TrapFocus';
+import {Scrollable} from '../Scrollable';
 import {dataPolarisTopBar, layer} from '../shared';
 import {setRootProperty} from '../../utilities/set-root-property';
 import {FrameContext} from '../../utilities/frame';
@@ -73,7 +74,10 @@ interface State {
   showContextualSaveBar: boolean;
 }
 
+const APP_FRAME = 'AppFrame';
 const APP_FRAME_MAIN = 'AppFrameMain';
+const APP_FRAME_CONTENT = 'AppFrameContent';
+const APP_FRAME_SCROLLABLE = 'AppFrameScrollable';
 const APP_FRAME_NAV = 'AppFrameNav';
 const APP_FRAME_TOP_BAR = 'AppFrameTopBar';
 const APP_FRAME_LOADING_BAR = 'AppFrameLoadingBar';
@@ -217,9 +221,13 @@ class FrameInner extends PureComponent<CombinedProps, State> {
       skipFocused && styles.focused,
     );
 
+    const skipTargetFallback = this.props.dynamicTopBarAndReframe
+      ? APP_FRAME_SCROLLABLE
+      : APP_FRAME_MAIN;
+
     const skipTarget = skipToContentTarget?.current
       ? skipToContentTarget.current.id
-      : APP_FRAME_MAIN;
+      : skipTargetFallback;
 
     const skipMarkup = (
       <div className={skipClassName}>
@@ -305,7 +313,7 @@ class FrameInner extends PureComponent<CombinedProps, State> {
           {loadingMarkup}
           {navigationOverlayMarkup}
           {hasDynamicTopBar ? (
-            <div className={styles.ShadowBevel}>
+            <div className={styles.ShadowBevel} id={APP_FRAME}>
               {navigationMarkup}
               <main
                 className={classNames(
@@ -315,22 +323,25 @@ class FrameInner extends PureComponent<CombinedProps, State> {
                 id={APP_FRAME_MAIN}
                 data-has-global-ribbon={Boolean(globalRibbon)}
               >
-                <div
-                  className={classNames(
-                    styles.Content,
-                    hasDynamicTopBar && styles['Content-TopBarAndReframe'],
-                  )}
-                >
-                  {hasDynamicTopBar ? (
+                {hasDynamicTopBar ? (
+                  <Scrollable
+                    scrollbarWidth="thin"
+                    className={styles.Scrollable}
+                    id={APP_FRAME_SCROLLABLE}
+                  >
                     <div
-                      className={styles['ScrollbarSafeArea-TopBarAndReframe']}
+                      className={classNames(
+                        styles.Content,
+                        styles['Content-TopBarAndReframe'],
+                      )}
+                      id={APP_FRAME_CONTENT}
                     >
                       {children}
                     </div>
-                  ) : (
-                    children
-                  )}
-                </div>
+                  </Scrollable>
+                ) : (
+                  <div className={styles.Content}>{children}</div>
+                )}
               </main>
             </div>
           ) : (