Improve scrollbar experience in the admin #9701

martenbjork · 2023-07-12T13:08:40Z

Context

In MacOS's System Preferences > Appearance, choose these settings:

Now browse the admin. As navigate between different pages, the body will sometimes overflow, sometimes not. This means scrollbars will come and go. For instance, while loading the data, the skeleton content will overflow and thus trigger scrollbars, but when minimal data comes in, the scrollbars might go away.

This cases the UI to constantly jump ~15px left and right as the scrollbars come and go.

This fix

We thought a lot about how we can improve the scrollbar behaviour.

This PR completely circumvents the scrollbar logic by changing the layout constraints of Polaris core elements.

It makes sure that, when a scrollbar is set to "always on", the content stays clear of the area where the scrollbar may appear.

How to 🎩

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

Copy-paste this code in playground/Playground.tsx:

import React from 'react';
import {Page} from '../src';

export function Playground() {
  return (
    <Page title="Playground">
      {/* Add the code you want to test in here */}
    </Page>
  );
}

🎩 checklist

Tested on mobile
Tested on multiple browsers
Tested for accessibility
Updated the component's README.md with documentation changes
Tophatted documentation changes in the style guide

Tested on device types, browsers and operating systems.

martenbjork · 2023-07-12T13:15:12Z

/snapit

github-actions · 2023-07-12T13:19:12Z

🫰✨ Thanks @martenbjork! Your snapshots have been published to npm.

Test the snapshots by updating your package.json with the newly published versions:

yarn add @shopify/[email protected]

yarn add @shopify/[email protected]

martenbjork · 2023-07-14T15:01:03Z

/snapit

martenbjork · 2023-07-14T15:11:43Z

/snapit

github-actions · 2023-07-14T15:16:10Z

🫰✨ Thanks @martenbjork! Your snapshots have been published to npm.

Test the snapshots by updating your package.json with the newly published versions:

yarn add @shopify/[email protected]

yarn add @shopify/[email protected]

polaris-react/src/components/AppProvider/AppProvider.tsx

mattkubej · 2023-07-17T15:51:50Z

This does seem like it should prevent the jumping with the main content area. However, I think we might need to change the UX of the left navigation if we want to introduce a similar solution there. I suppose the jumping in the left navigation is less egregious in comparison to the main content.

martenbjork · 2023-07-20T14:16:16Z

/snapit

mattkubej · 2023-07-20T14:18:51Z

Looks like a changeset is absent from yarn changeset, so there is a CI phase failing. I'll start reviewing the implementation though 😄

github-actions · 2023-07-20T14:20:34Z

🫰✨ Thanks @martenbjork! Your snapshots have been published to npm.

Test the snapshots by updating your package.json with the newly published versions:

yarn add @shopify/[email protected]

yarn add @shopify/[email protected]

polaris-react/src/components/AppProvider/AppProvider.tsx

mattkubej · 2023-07-20T14:38:58Z

polaris-react/src/components/TopBar/TopBar.tsx

-        <div className={styles.SearchField}>{searchMarkup}</div>
-        <div className={styles.SecondaryMenu}>{secondaryMenu}</div>
-        {userMenu}
+      <div className={styles.Container}>


I think this is going to conflict with what I just drafted here, where I'm converting the TopBar to a grid layout in order to achieve newly desired alignment of the elements. Specifically, the search field.

I'm guessing the Container is being added in this manner to constrain the width, but also allow the TopBar div to stretch the width of the screen. I suppose with the change I have above, the grid styles would then need to be applied to a child div of the Container div. What are your thoughts?

Something we'll need to be cognizant of with the ordering of delivering this and what I have staged.

I'm guessing the Container is being added in this manner to constrain the width, but also allow the TopBar div to stretch the width of the screen.

Correct!

the grid styles would then need to be applied to a child div of the Container div

Yeah, this sounds right to me. I'm basically just adding another child in here and adding display: flex to it as well, so that the flex behaviour is passed down to the children below.

martenbjork · 2023-07-21T13:41:25Z

/snapit

github-actions · 2023-07-21T13:46:23Z

🫰✨ Thanks @martenbjork! Your snapshots have been published to npm.

Test the snapshots by updating your package.json with the newly published versions:

yarn add @shopify/[email protected]

yarn add @shopify/[email protected]

egjiri · 2023-07-21T14:34:24Z

I took a look at this spin instance and compared it to my own that doesn't have this polaris release and noticed a couple things:

I was able to see the jumpiness you're talking about on the current polars but when switching to Polaris Uplift I do not see it jump on my spin. I wonder if uplift already has a fix in place for this. Let me know if I'm missing something here.
This PR increases the space between the account and the right side of the screen which is something to look for it we're ok with that or that will need to be adjusted as well to bring it back to where it was. See Screenshot

Shop 1 · Products · Practical Copper Car · Shopify 2023-07-21 10-28-13

kyledurand

👏

kyledurand · 2023-07-21T15:27:58Z

polaris-react/src/components/AppProvider/AppProvider.tsx

+  const parentEl = document.createElement('div');
+  parentEl.setAttribute(
+    'style',
+    `position: absolute; opacity: 0; transform: translate3d(-9999px, -9999px, 0); pointer-events: none; width:${SCROLLBAR_TEST_ELEMENT_PARENT_SIZE}px; height:${SCROLLBAR_TEST_ELEMENT_PARENT_SIZE}px;`,


Does this need to be 3d? Might be slightly more performant:

Suggested change

`position: absolute; opacity: 0; transform: translate3d(-9999px, -9999px, 0); pointer-events: none; width:${SCROLLBAR_TEST_ELEMENT_PARENT_SIZE}px; height:${SCROLLBAR_TEST_ELEMENT_PARENT_SIZE}px;`,

`position: absolute; opacity: 0; transform: translate(-9999px, -9999px); pointer-events: none; width:${SCROLLBAR_TEST_ELEMENT_PARENT_SIZE}px; height:${SCROLLBAR_TEST_ELEMENT_PARENT_SIZE}px;`,

I think translate3d is more performant because it runs the process in the GPU but I haven't looked at this in awhile so it's worth double checking

This is what I learned way back when too, but I wouldn't be surprised if the whole "trigger GPU" trick has passed its best before date 😀

I was thinking it would be more performant because it shouldn't have to trigger the gpu. There's no visibility in this but yes, probably negligible

mattkubej · 2023-07-22T05:04:26Z

Give me a shout if you want help with rebasing with my latest changes to the TopBar. Alternatively, I could give it a swing directly on your branch if you want me to take care of it. Let me know what works best for you.

jesstelford

Clever. I like it! 👍

kyledurand · 2023-07-24T15:02:33Z

polaris-react/src/components/AppProvider/AppProvider.tsx

+  const parentEl = document.createElement('div');
+  parentEl.setAttribute(
+    'style',
+    `position: absolute; opacity: 0; transform: translate3d(-9999px, -9999px, 0); pointer-events: none; width:${SCROLLBAR_TEST_ELEMENT_PARENT_SIZE}px; height:${SCROLLBAR_TEST_ELEMENT_PARENT_SIZE}px;`,


I was thinking it would be more performant because it shouldn't have to trigger the gpu. There's no visibility in this but yes, probably negligible

martenbjork · 2023-07-24T15:30:06Z

/snapit

github-actions · 2023-07-24T15:35:13Z

🫰✨ Thanks @martenbjork! Your snapshots have been published to npm.

Test the snapshots by updating your package.json with the newly published versions:

yarn add @shopify/[email protected]

yarn add @shopify/[email protected]

mattkubej

Top hatted with and without editions enabled. LGTM!

mattkubej · 2023-09-01T19:23:26Z

Spoke with @martenbjork. I'll merge this through and shepherd this into web.

@heyjoethomas

This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @shopify/[email protected] ### Minor Changes - [#9856](#9856) [`47652f7d6`](47652f7) Thanks [@heyjoethomas](https://github.com/heyjoethomas)! - Updates social media icons, removing them from their containers and adds a one for the Twitch platform. ## @shopify/[email protected] ### Minor Changes - [#10263](#10263) [`67699cb88`](67699cb) Thanks [@aveline](https://github.com/aveline)! - Added migration for `Button` component ## @shopify/[email protected] ### Minor Changes - [#9701](#9701) [`cbf539495`](cbf5394) Thanks [@martenbjork](https://github.com/martenbjork)! - Updated the Frame and Topbar components to stay clear of a scrollbar. This reduces the overall jumpiness in the UI when scrollbars appear and disappear when using a Polaris app. ### Patch Changes - [#10284](#10284) [`eba75d20a`](eba75d2) Thanks [@zakwarsame](https://github.com/zakwarsame)! - - Updated `Filters` query field to initialize with focus based on `mode` state - [#10282](#10282) [`9a2d4f62a`](9a2d4f6) Thanks [@kyledurand](https://github.com/kyledurand)! - Fixed pointer alignment on tooltip - [#10343](#10343) [`12a62b4d7`](12a62b4) Thanks [@mrcthms](https://github.com/mrcthms)! - Fixed UI inconsistencies in the mobile view of the IndexFilters - [#10276](#10276) [`abb50250e`](abb5025) Thanks [@highfieldjames](https://github.com/highfieldjames)! - Updated `TextField` of `type` `number` to focus when a `Spinner` button is clicked - Updated dependencies \[[`47652f7d6`](47652f7)]: - @shopify/[email protected] ## [email protected] ### Patch Changes - Updated dependencies \[[`47652f7d6`](47652f7), [`eba75d20a`](eba75d2), [`9a2d4f62a`](9a2d4f6), [`12a62b4d7`](12a62b4), [`cbf539495`](cbf5394), [`abb50250e`](abb5025)]: - @shopify/[email protected] - @shopify/[email protected] Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

## Context In MacOS's System Preferences > Appearance, choose these settings: <img width="475" alt="image" src="https://github.com/Shopify/polaris/assets/875708/2faa63e4-82ad-4fe1-8abe-7e0268c759f1"> Now browse the admin. As navigate between different pages, the body will sometimes overflow, sometimes not. This means scrollbars will come and go. For instance, while loading the data, the skeleton content will overflow and thus trigger scrollbars, but when minimal data comes in, the scrollbars might go away. This cases the UI to constantly jump ~15px left and right as the scrollbars come and go. ## This fix We thought _[a lot](https://docs.google.com/document/d/1AW7AFldJc6RZT4sHR303bqWMqLf0-CDXExEVf6hFg1w/edit#heading=h.e2bgx61b1bn2)_ about how we can improve the scrollbar behaviour. This PR completely circumvents the scrollbar logic by changing the layout constraints of Polaris core elements. It makes sure that, when a scrollbar is set to "always on", the content stays clear of the area where the scrollbar may appear. ### How to 🎩 🖥 [Local development instructions](https://github.com/Shopify/polaris/blob/main/README.md#local-development) 🗒 [General tophatting guidelines](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md) 📄 [Changelog guidelines](https://github.com/Shopify/polaris/blob/main/.github/CONTRIBUTING.md#changelog)  <details> <summary>Copy-paste this code in <code>playground/Playground.tsx</code>:</summary> ```jsx import React from 'react'; import {Page} from '../src'; export function Playground() { return ( <Page title="Playground"> {/* Add the code you want to test in here */} </Page> ); } ``` </details> ### 🎩 checklist - [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) - [x] Tested for [accessibility](https://github.com/Shopify/polaris/blob/main/documentation/Accessibility%20testing.md) - [x] Updated the component's `README.md` with documentation changes - [x] [Tophatted documentation](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting%20documentation.md) changes in the style guide Tested on device types, browsers and operating systems. <img width="1143" alt="Screenshot 2023-07-20 at 4 30 42 PM" src="https://github.com/Shopify/polaris/assets/875708/40f29f11-60a1-4b03-a7e2-d5c37524e353"> <img width="1143" alt="Screenshot 2023-07-20 at 4 28 12 PM" src="https://github.com/Shopify/polaris/assets/875708/1f55d2ea-7963-46c3-bc72-719e9f26d1f6"> <img width="1208" alt="Screenshot 2023-07-20 at 4 32 38 PM" src="https://github.com/Shopify/polaris/assets/875708/3407960f-5be6-4192-b92c-cff56975a1b5">

Add 'safe area' to reduce UI jumpiness

afbf1de

Only use safe area when scrollbars are set to always on

97b1727

Skip scrollbar check on smaller screen due to edge-to-edge cards

3d2009e

Remove print style

923538e

mattkubej reviewed Jul 17, 2023

View reviewed changes

polaris-react/src/components/AppProvider/AppProvider.tsx Outdated Show resolved Hide resolved

martenbjork added 2 commits July 20, 2023 15:20

Refactor code

365259f

Clean up code

bb8f9ca

martenbjork changed the title ~~Add 'safe area' to reduce UI jumpiness~~ Improve scrollbar experience in the admin Jul 20, 2023

martenbjork requested review from kyledurand and mattkubej July 20, 2023 14:17