Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Frame] Add scrollbar width buffer onto Content #11891

Merged
merged 2 commits into from
Apr 17, 2024
Merged

[Frame] Add scrollbar width buffer onto Content #11891

merged 2 commits into from
Apr 17, 2024

Conversation

sophschneider
Copy link
Contributor

@sophschneider sophschneider commented Apr 15, 2024 •
edited
Loading

WHY are these changes introduced?

Part of https://github.com/Shopify/polaris-internal/issues/1379
Follow up to #11826, this change is still behind a feature flag.

  • Moving scrollbar buffer from main to content in Frame since we moved the scrollbar to just the content
  • updating scroll -> auto ensures the scrollbar doesn't show up when there isn't any scroll.

Tophat

Spin

  • turn on scrollbars always show in settings, shorten the window so the scrollbar hides and shows and make sure there isn't a layout shift

Screen Recording 2024-04-17 at 9 56 36 AM

  • Chrome, Firefox, Safari
  • Mobile

@sophschneider sophschneider added the #gsd:38420 Top bar and global reframe project label Apr 15, 2024
@sophschneider
Copy link
Contributor Author

/snapit

@github-actions GitHub Actions
Copy link
Contributor

🫰✨ Thanks @sophschneider! Your snapshothas been published to npm.

Test the snapshot by updating your package.json with the newly published version:

"@shopify/polaris": "0.0.0-snapshot-20240415195121"

@chloerice
Copy link
Member

I think there was a longstanding layout shift issue fixed recently by @tjonx and @kyledurand by always showing the scrollbar 👀

@sophschneider sophschneider changed the title [Frame] Change content overflow from scroll to auto [Frame] Change content overflow from scroll to auto behind feature flag Apr 15, 2024
@sophschneider
Copy link
Contributor Author

@chloerice these are net new styles for my reframe stuff! It shouldn't change anything with the beta flag off. I'm currently tophatting to make sure my new architecture doesn't cause layout shifts as per Kyle's and Marten's changes from last year

@sophschneider
Copy link
Contributor Author

Putting this back in draft! I need to work on it a bit more and reintroduce the gutter widths in order to avoid the layout shift. I think I still need to set the scroll to auto to avoid double scrollbars in iframes (I believe it is right now without the beta flag). I'll retag everyone for review when its ready!

@sophschneider sophschneider marked this pull request as draft April 16, 2024 02:42
@sophschneider
Copy link
Contributor Author

/snapit

@sophschneider sophschneider changed the title [Frame] Change content overflow from scroll to auto behind feature flag [Frame] Add scrollbar width buffer onto Content Apr 16, 2024
@github-actions GitHub Actions
Copy link
Contributor

🫰✨ Thanks @sophschneider! Your snapshothas been published to npm.

Test the snapshot by updating your package.json with the newly published version:

"@shopify/polaris": "0.0.0-snapshot-20240416141947"

@sophschneider
Copy link
Contributor Author

/snapit

@github-actions GitHub Actions
Copy link
Contributor

🫰✨ Thanks @sophschneider! Your snapshothas been published to npm.

Test the snapshot by updating your package.json with the newly published version:

"@shopify/polaris": "0.0.0-snapshot-20240416185448"

@sophschneider
Copy link
Contributor Author

/snapit

@github-actions GitHub Actions
Copy link
Contributor

🫰✨ Thanks @sophschneider! Your snapshothas been published to npm.

Test the snapshot by updating your package.json with the newly published version:

"@shopify/polaris": "0.0.0-snapshot-20240417025114"

@sophschneider sophschneider marked this pull request as ready for review April 17, 2024 14:00
kyledurand
kyledurand approved these changes Apr 17, 2024
View reviewed changes
Copy link
Contributor

@kyledurand kyledurand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎩 'd with sidekick and some app pages. cc @mmapplebeck, can you take a look at the spin instance here to make sure we don't cause the same regression you fixed before?

@sophschneider
Copy link
Contributor Author

@kyledurand @mmapplebeck just put this on staging too! z1lt

@sophschneider
Copy link
Contributor Author

I'm gonna merge this now to get it out with the next release but feel free to provide more feedback and I can do it in a follow up https://github.com/Shopify/polaris-internal/issues/1379

@sophschneider sophschneider merged commit c84d4e8 into main Apr 17, 2024
13 checks passed
@sophschneider sophschneider deleted the scrollbar-reframe branch April 17, 2024 18:00
@shopify-github-actions-access shopify-github-actions-access bot mentioned this pull request Apr 17, 2024
Merged
@sophschneider sophschneider mentioned this pull request Apr 17, 2024
Merged
sophschneider added a commit that referenced this pull request Apr 17, 2024
@sophschneider
[Frame] Accommodate sidebar in scrollbar safe container (#11900)
215b792 
Patching #11891 to accommodate
the sidebar width shift
sophschneider pushed a commit that referenced this pull request Apr 17, 2024
@shopify-github-actions-access @github-actions
Version Packages (#11873)
0354098 
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

- [#11883](#11883)
[`a60d8aa4f`](a60d8aa)
Thanks [@chloerice](https://github.com/chloerice)! - Added a
`disclosureZIndexOverride` prop to `Filters`, `IndexFilters`, and `Tabs`
that is passed to `Popover` and `Tooltip` when provided


- [#11826](#11826)
[`a7fd7ab5d`](a7fd7ab)
Thanks [@sophschneider](https://github.com/sophschneider)! - Added
`contextualSaveBarVisible` and `contextualSaveBarProps` to `Frame`
context

### Patch Changes

- [#11842](#11842)
[`2a93578af`](2a93578)
Thanks [@kyledurand](https://github.com/kyledurand)! - Fixed layout
shift for option lists within popovers


- [#11846](#11846)
[`ce6353b97`](ce6353b)
Thanks [@sophschneider](https://github.com/sophschneider)! - Restyled
Frame content behind dynamicTopBarAndReframe feature flag


- [#11872](#11872)
[`696bcb725`](696bcb7)
Thanks [@mattkubej](https://github.com/mattkubej)! - globally remove
link tap highlighting


- [#11874](#11874)
[`744036706`](7440367)
Thanks [@laurkim](https://github.com/laurkim)! - Added support for ref
to `Image` to handle image load with `EmptyState`


- [#11881](#11881)
[`c96ff56a0`](c96ff56)
Thanks [@sophschneider](https://github.com/sophschneider)! - Fixed Frame
feature override class to get proper max-width for main content.


- [#11885](#11885)
[`af80d3a82`](af80d3a)
Thanks [@craigcolesshopify](https://github.com/craigcolesshopify)! -
[indexTable] Fixed over scroll gap on `IndexTable` for sortable last
headings with `alignment="end"`


- [#11889](#11889)
[`374030428`](3740304)
Thanks [@chloerice](https://github.com/chloerice)! - Fixed `TextField`
zoom on focus due to font-size below 16px


- [#11900](#11900)
[`215b79271`](215b792)
Thanks [@sophschneider](https://github.com/sophschneider)! - Fixed
`Frame` scrollbar safe area to accommodate sidebar


- [#11842](#11842)
[`2a93578af`](2a93578)
Thanks [@kyledurand](https://github.com/kyledurand)! - Changed selected
icon position in Listbox and OptionList


- [#11891](#11891)
[`c84d4e875`](c84d4e8)
Thanks [@sophschneider](https://github.com/sophschneider)! - Moved
`Frame` scrollbar from main to content and set overflow-y from scroll to
auto behind a feature flag

## [email protected]

### Patch Changes

- Updated dependencies
\[[`a60d8aa4f`](a60d8aa),
[`2a93578af`](2a93578),
[`ce6353b97`](ce6353b),
[`696bcb725`](696bcb7),
[`744036706`](7440367),
[`c96ff56a0`](c96ff56),
[`af80d3a82`](af80d3a),
[`374030428`](3740304),
[`215b79271`](215b792),
[`a7fd7ab5d`](a7fd7ab),
[`2a93578af`](2a93578),
[`c84d4e875`](c84d4e8)]:
    -   @shopify/[email protected]

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
AnnaCheba pushed a commit to AnnaCheba/polaris that referenced this pull request Apr 22, 2024
@sophschneider
[Frame] Add scrollbar width buffer onto Content (Shopify#11891)
e981267 
### WHY are these changes introduced?

Part of https://github.com/Shopify/polaris-internal/issues/1379
Follow up to Shopify#11826, this change
is still behind a feature flag.

* Moving scrollbar buffer from main to content in `Frame` since we moved
the scrollbar to just the content
* updating `scroll` -> `auto` ensures the scrollbar doesn't show up when
there isn't any scroll.

## Tophat


[Spin](https://admin.web.web-npdr.sophie-schneider.us.spin.dev/store/shop1/products?selectedView=all)

* turn on scrollbars always show in settings, shorten the window so the
scrollbar hides and shows and make sure there isn't a layout shift

![Screen Recording 2024-04-17 at 9 56
36 AM](https://github.com/Shopify/polaris/assets/20652326/9933ab0c-af83-46a4-b60b-d5626f785680)

- [x] Chrome, Firefox, Safari
- [x] Mobile
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kyledurand kyledurand kyledurand approved these changes

@laurkim laurkim laurkim approved these changes

@chloerice chloerice Awaiting requested review from chloerice

Assignees
No one assigned
Labels
#gsd:38420 Top bar and global reframe project
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@sophschneider @chloerice @kyledurand @laurkim