Skip to content

Commit

Permalink
[Frame] Reframe polish: sticky manager and offset (#11945)
Browse files Browse the repository at this point in the history
### WHY are these changes introduced?

Part of [#1470](https://github.com/Shopify/polaris-backlog/issues/1470)

### WHAT is this pull request doing?

1. Accounts for `Frame` offset prop behind `dynamicTopBarAndReframe`
project
2. Update `AppProvider` to pass the new reframe scroll container to the
`stickyManager`
3. Fixes popover scrolls by wrapping frame content in a `Scrollable`

### How to 🎩
https://admin.web.frame-polish.sophie-schneider.us.spin.dev/store/shop1

1. `Frame` [offset story with feature flag
on](https://5d559397bae39100201eedc1-ygmdwznspd.chromatic.com/?path=/story/all-components-frame--with-an-offset&globals=dynamicTopBarAndReframe:!true)
2. Check on s[pin with beta flag on that index
tables](https://admin.web.frame-polish.sophie-schneider.us.spin.dev/store/shop1/products/1)
are sticking to top of reframe
- Scroll to bottom of page to variants and the header should stick,
adjust window height
3. Check "Skip to content" button with beta flag off and on
4. Check that popovers scroll with content
5. Check no layout shift for when scrollbar is hidden/shows with
scrollbar set to "always" in settings


### 🎩 checklist

- [x] Tested a
[snapshot](https://github.com/Shopify/polaris/blob/main/documentation/Releasing.md#-snapshot-releases)
- [x] Tested on
[mobile](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md#cross-browser-testing)
- [x] Tested on [multiple
browsers](https://help.shopify.com/en/manual/shopify-admin/supported-browsers)
  • Loading branch information
sophschneider committed May 1, 2024
1 parent 995079c commit b59743a
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 25 deletions.
5 changes: 5 additions & 0 deletions .changeset/little-plums-try.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@shopify/polaris': patch
---

Added offset width to reframe `Frame` and passed reframe scroll container to sticky manager in `AppProvider`
11 changes: 10 additions & 1 deletion polaris-react/src/components/AppProvider/AppProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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();

Expand Down
49 changes: 39 additions & 10 deletions polaris-react/src/components/Frame/Frame.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -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-frame-offset, 0px));
}

@media (prefers-reduced-motion) {
Expand Down Expand Up @@ -290,10 +290,7 @@
}

.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);
height: 100%;

.hasSidebar & {
/* Sidebar breakpoint is 1200px */
Expand All @@ -310,7 +307,7 @@
/* 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) - var(--pc-frame-offset, 0px)
);
/* stylelint-enable -- polaris/conventions/polaris/custom-property-allowed-list */

Expand All @@ -328,9 +325,10 @@
/* 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) -
var(--pc-frame-offset, 0px)
);
margin-right: unset;
}

@media (prefers-reduced-motion) {
Expand Down Expand Up @@ -425,8 +423,6 @@
}

.ShadowBevel {
width: 100%;

@mixin shadow-bevel var(--p-shadow-100), var(--p-border-radius-300),
var(--p-z-index-1);

Expand All @@ -435,4 +431,37 @@
top: var(--pg-top-bar-height);
background-color: var(--p-color-bg);
}
position: fixed;
width: 100%;
transition: width var(--p-motion-duration-250) var(--p-motion-ease);

@media (prefers-reduced-motion) {
transition: none;
}

@media (--p-breakpoints-md-up) {
/* stylelint-disable-next-line polaris/conventions/polaris/custom-property-allowed-list -- private token from component */
width: calc(100% - var(--pc-frame-offset, 0px));
}

.hasSidebar & {
transition: width var(--p-motion-duration-250) var(--p-motion-ease);

/* Sidebar breakpoint is 1200px */
/* 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) - var(--pc-frame-offset, 0px));
}

@media (prefers-reduced-motion) {
transition: none;
}
}
}

.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));
}
38 changes: 24 additions & 14 deletions polaris-react/src/components/Frame/Frame.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -74,6 +75,8 @@ interface State {
}

const APP_FRAME_MAIN = 'AppFrameMain';
const APP_FRAME_SCROLLABLE = 'AppFrameScrollable';
const APP_FRAME_BEVEL = 'AppFrameBevel';
const APP_FRAME_NAV = 'AppFrameNav';
const APP_FRAME_TOP_BAR = 'AppFrameTopBar';
const APP_FRAME_LOADING_BAR = 'AppFrameLoadingBar';
Expand Down Expand Up @@ -305,7 +308,7 @@ class FrameInner extends PureComponent<CombinedProps, State> {
{loadingMarkup}
{navigationOverlayMarkup}
{hasDynamicTopBar ? (
<div className={styles.ShadowBevel}>
<div className={styles.ShadowBevel} id={APP_FRAME_BEVEL}>
{navigationMarkup}
<main
className={classNames(
Expand All @@ -315,22 +318,29 @@ 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"
horizontal={false}
className={styles.Scrollable}
id={APP_FRAME_SCROLLABLE}
>
<div
className={styles['ScrollbarSafeArea-TopBarAndReframe']}
className={classNames(
styles.Content,
styles['Content-TopBarAndReframe'],
)}
>
{children}
<div
className={styles['ScrollbarSafeArea-TopBarAndReframe']}
>
{children}
</div>
</div>
) : (
children
)}
</div>
</Scrollable>
) : (
<div className={styles.Content}>{children}</div>
)}
</main>
</div>
) : (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@

@media (--p-breakpoints-md-up) {
border-top-right-radius: 0;
padding-top: var(--p-space-400);
}
}

Expand Down

0 comments on commit b59743a

Please sign in to comment.