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] Reframe polish: sticky manager and offset #11945

Merged
merged 6 commits into from
May 1, 2024
Merged

[Frame] Reframe polish: sticky manager and offset #11945

merged 6 commits into from
May 1, 2024

Conversation

sophschneider
Copy link
Contributor

@sophschneider sophschneider commented Apr 24, 2024 •
edited
Loading

WHY are these changes introduced?

Part of #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
  2. Check on spin with beta flag on that index tables are sticking to top of reframe
  • Scroll to bottom of page to variants and the header should stick, adjust window height
  1. Check "Skip to content" button with beta flag off and on
  2. Check that popovers scroll with content
  3. Check no layout shift for when scrollbar is hidden/shows with scrollbar set to "always" in settings

🎩 checklist

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

/snapit

1 similar comment
@sophschneider
Copy link
Contributor Author

/snapit

@github-actions GitHub Actions
Copy link
Contributor

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

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

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

@sophschneider
Copy link
Contributor Author

/snapit

@github-actions GitHub Actions
Copy link
Contributor

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

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

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

@sophschneider sophschneider marked this pull request as ready for review April 25, 2024 15:29
@sophschneider sophschneider requested review from laurkim, yurm04 and kyledurand and removed request for laurkim April 25, 2024 15:30
@kyledurand
Copy link
Member

Looks like we may need to adjust content spacing as well:

content-shift.mp4

@kyledurand
Copy link
Member

Seeing some popover funk. Not sure if spin was updated properly

popovers-stuck.mp4

@sophschneider
Copy link
Contributor Author

@kyledurand thanks! Yeah I saw the popover thing too, I think its because the scroll container used to be on document and now its not falling back properly because the whole app isn't wrapped in a scrollable. I'll try and fix it in this PR! Would probably be worth it for me to do a full document audit in polaris to see what else relies on the document scroll container

@sophschneider
Copy link
Contributor Author

moving this back to draft as I fix the above things! ill retag for review after

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

/snapit

@sophschneider sophschneider force-pushed the sticky-and-offset branch 2 times, most recently from 2cdb657 to 97bb2cf Compare April 29, 2024 18:52
@sophschneider
Copy link
Contributor Author

/snapit

@github-actions GitHub Actions
Copy link
Contributor

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

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

"@shopify/polaris-migrator": "0.0.0-snapshot-20240429185346",
"@shopify/polaris": "0.0.0-snapshot-20240429185346",
"@shopify/polaris-tokens": "0.0.0-snapshot-20240429185346",
"@shopify/stylelint-polaris": "0.0.0-snapshot-20240429185346"

@sophschneider
Copy link
Contributor Author

/snapit

@github-actions GitHub Actions
Copy link
Contributor

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

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

"@shopify/polaris-migrator": "0.0.0-snapshot-20240430171916",
"@shopify/polaris": "0.0.0-snapshot-20240430171916",
"@shopify/polaris-tokens": "0.0.0-snapshot-20240430171916",
"@shopify/stylelint-polaris": "0.0.0-snapshot-20240430171916"

@sophschneider
Copy link
Contributor Author

/snapit

@github-actions GitHub Actions
Copy link
Contributor

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

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

"@shopify/polaris-migrator": "0.0.0-snapshot-20240430173334",
"@shopify/polaris": "0.0.0-snapshot-20240430173334",
"@shopify/polaris-tokens": "0.0.0-snapshot-20240430173334",
"@shopify/stylelint-polaris": "0.0.0-snapshot-20240430173334"

@sophschneider
Copy link
Contributor Author

/snapit

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented May 1, 2024

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

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

"@shopify/polaris-migrator": "0.0.0-snapshot-20240501002056",
"@shopify/polaris": "0.0.0-snapshot-20240501002056",
"@shopify/polaris-tokens": "0.0.0-snapshot-20240501002056",
"@shopify/stylelint-polaris": "0.0.0-snapshot-20240501002056"

@sophschneider sophschneider force-pushed the sticky-and-offset branch 2 times, most recently from d3e33dd to 0ef181c Compare May 1, 2024 01:23
@sophschneider
Copy link
Contributor Author

/snapit

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented May 1, 2024

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

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

"@shopify/polaris-migrator": "0.0.0-snapshot-20240501014340",
"@shopify/polaris": "0.0.0-snapshot-20240501014340",
"@shopify/polaris-tokens": "0.0.0-snapshot-20240501014340",
"@shopify/stylelint-polaris": "0.0.0-snapshot-20240501014340"

@sophschneider
Copy link
Contributor Author

/snapit

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented May 1, 2024

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

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

"@shopify/polaris-migrator": "0.0.0-snapshot-20240501020632",
"@shopify/polaris": "0.0.0-snapshot-20240501020632",
"@shopify/polaris-tokens": "0.0.0-snapshot-20240501020632",
"@shopify/stylelint-polaris": "0.0.0-snapshot-20240501020632"

sophschneider
sophschneider commented May 1, 2024
View reviewed changes
polaris-react/src/components/Frame/Frame.module.css
}

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

Choose a reason for hiding this comment

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

I have an issue https://github.com/Shopify/polaris-backlog/issues/1606 to update these to transforms for performance

@sophschneider
Copy link
Contributor Author

/snapit

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented May 1, 2024

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

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

"@shopify/polaris-migrator": "0.0.0-snapshot-20240501135743",
"@shopify/polaris": "0.0.0-snapshot-20240501135743",
"@shopify/polaris-tokens": "0.0.0-snapshot-20240501135743",
"@shopify/stylelint-polaris": "0.0.0-snapshot-20240501135743"

@sophschneider sophschneider marked this pull request as ready for review May 1, 2024 14:11
@sophschneider
Copy link
Contributor Author

@kyledurand theres still a bit of a layout shift but its due to the changes in scroll gutter, it should be just a bit!

kyledurand
kyledurand approved these changes May 1, 2024
View reviewed changes
Copy link
Member

@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.

Code and 🎩 look good! I'm slightly worried about changing the shadow bevel wrapper to fixed positioning but I couldn't find any issues 👍

laurkim
laurkim approved these changes May 1, 2024
View reviewed changes
Copy link
Contributor

@laurkim laurkim left a comment

Choose a reason for hiding this comment

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

💯

@sophschneider
Copy link
Contributor Author

@kyledurand yeah I had to do that because scrolling to anchor tags was causing some janky behaviour moving the shadow bevel container. ill let you know if I see any issues with it!

@sophschneider sophschneider merged commit b59743a into main May 1, 2024
13 checks passed
@sophschneider sophschneider deleted the sticky-and-offset branch May 1, 2024 17:18
@shopify-github-actions-access shopify-github-actions-access bot mentioned this pull request May 3, 2024
Merged
chloerice pushed a commit that referenced this pull request May 6, 2024
@shopify-github-actions-access @github-actions
Version Packages (#11946)
9833c5d 
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

- [#11979](#11979)
[`982491f0f`](982491f)
Thanks [@kyledurand](https://github.com/kyledurand)! - Added `animateIn`
transition option to Collapsible


- [#11967](#11967)
[`e50472f85`](e50472f)
Thanks [@kyledurand](https://github.com/kyledurand)! - Added `variant`
prop to Collapsible

### Patch Changes

- [#11976](#11976)
[`4f3bf9948`](4f3bf99)
Thanks [@chloerice](https://github.com/chloerice)! - Fixed sibling
`FormLayout.Item` widths not remaining equal when wrapped in
`FormLayout.Group`


- [#11945](#11945)
[`b59743a76`](b59743a)
Thanks [@sophschneider](https://github.com/sophschneider)! - Added
offset width to reframe `Frame` and passed reframe scroll container to
sticky manager in `AppProvider`


- [#11965](#11965)
[`7a702388d`](7a70238)
Thanks [@sophschneider](https://github.com/sophschneider)! - Added
scrollbar styles for reframe


- [#11944](#11944)
[`d1d69e919`](d1d69e9)
Thanks [@stefanlegg](https://github.com/stefanlegg)! - Add support for
hiding selectable checkbox on a per `IndexTable.Row` basis via
`hideSelectable` prop\`


- [#11947](#11947)
[`995079cc7`](995079c)
Thanks [@sophschneider](https://github.com/sophschneider)! - Fixed
`Sticky` to update sticky items when props change

- Updated dependencies
\[[`12dbc2cd8`](12dbc2c),
[`8ce6211c9`](8ce6211),
[`7a702388d`](7a70238)]:
    -   @shopify/[email protected]

## @shopify/[email protected]

### Minor Changes

- [#11965](#11965)
[`7a702388d`](7a70238)
Thanks [@sophschneider](https://github.com/sophschneider)! - Added
`color-scrollbar-thumb-bg` token

### Patch Changes

- [#11981](#11981)
[`12dbc2cd8`](12dbc2c)
Thanks [@sophschneider](https://github.com/sophschneider)! - Updated
internal only whiteAlpha scale and dark experimental theme with new
values


- [#11853](#11853)
[`8ce6211c9`](8ce6211)
Thanks [@aaronccasanova](https://github.com/aaronccasanova)! - Added
`"sideEffect": "false"` to the `package.json` to enable treeshaking

## @shopify/[email protected]

### Patch Changes

- Updated dependencies
\[[`12dbc2cd8`](12dbc2c),
[`8ce6211c9`](8ce6211),
[`7a702388d`](7a70238)]:
    -   @shopify/[email protected]
    -   @shopify/[email protected]

## @shopify/[email protected]

### Patch Changes

- Updated dependencies
\[[`12dbc2cd8`](12dbc2c),
[`8ce6211c9`](8ce6211),
[`7a702388d`](7a70238)]:
    -   @shopify/[email protected]

## [email protected]

### Patch Changes

- Updated dependencies
\[[`12dbc2cd8`](12dbc2c),
[`8ce6211c9`](8ce6211),
[`7a702388d`](7a70238)]:
    -   @shopify/[email protected]

## [email protected]

### Patch Changes

- Updated dependencies
\[[`4f3bf9948`](4f3bf99),
[`b59743a76`](b59743a),
[`12dbc2cd8`](12dbc2c),
[`982491f0f`](982491f),
[`7a702388d`](7a70238),
[`8ce6211c9`](8ce6211),
[`e50472f85`](e50472f),
[`d1d69e919`](d1d69e9),
[`995079cc7`](995079c),
[`7a702388d`](7a70238)]:
    -   @shopify/[email protected]
    -   @shopify/[email protected]

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@laurkim laurkim laurkim approved these changes

@kyledurand kyledurand kyledurand approved these changes

@yurm04 yurm04 Awaiting requested review from yurm04

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.
3 participants
@sophschneider @kyledurand @laurkim