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

[Sticky] Fix Sticky to update items when props change #11947

Merged
merged 1 commit into from Apr 29, 2024
Merged

[Sticky] Fix Sticky to update items when props change #11947

merged 1 commit into from Apr 29, 2024

Conversation

sophschneider
Copy link
Contributor

@sophschneider sophschneider commented Apr 24, 2024 •
edited

WHY are these changes introduced?

Fixes https://github.com/Shopify/polaris-backlog/issues/1603

WHAT is this pull request doing?

Sticky was only registering sticky elements on componentDidMount. For the IndexTable, this happens before the boundingElement (i.e. the IndexTable) is mounted. Thus the boundingElement of the sticky header element in the sticky manager was actually null and the manager could not calculate when the table ends.

This PR fixes that problem by adding a check in componentDidUpdate to see if props changed. If props change, it unregisters the old sticky item and registers a new one.

How to 🎩

Products bug https://admin.web.business-platform-8abn.sophie-schneider.us.spin.dev/store/shop5/products/2
Companies bug https://admin.web.business-platform-8abn.sophie-schneider.us.spin.dev/store/shop5/companies/1

🎩 checklist

@sophschneider
Copy link
Contributor Author

/snapit

@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-icons": "0.0.0-snapshot-20240424235624",
"@shopify/polaris-migrator": "0.0.0-snapshot-20240424235624",
"@shopify/polaris": "0.0.0-snapshot-20240424235624",
"@shopify/polaris-tokens": "0.0.0-snapshot-20240424235624",
"@shopify/stylelint-polaris": "0.0.0-snapshot-20240424235624"

@sophschneider sophschneider marked this pull request as ready for review April 25, 2024 01:28
LA1CH3
LA1CH3 approved these changes Apr 25, 2024
View reviewed changes
Copy link
Contributor

@LA1CH3 LA1CH3 left a comment

Choose a reason for hiding this comment

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

Tophatted on admin PDP and LGTM. Thanks for the fix on this 🙏

tatianau
tatianau approved these changes Apr 25, 2024
View reviewed changes
Copy link
Contributor

@tatianau tatianau left a comment

Choose a reason for hiding this comment

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

Wonderful, thank you so much @sophschneider 🙏 ❤️ 🚀

ishanighosh1
ishanighosh1 approved these changes Apr 25, 2024
View reviewed changes
Copy link

@ishanighosh1 ishanighosh1 left a comment

Choose a reason for hiding this comment

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

Reviewed the spin instance for the Companies section! It looks good! :)

@sophschneider sophschneider merged commit 995079c into main Apr 29, 2024
13 checks passed
@sophschneider sophschneider deleted the sticky-bug branch April 29, 2024 16:33
@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/polaris@13.3.0

### 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/polaris-tokens@9.1.0

## @shopify/polaris-tokens@9.1.0

### 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/polaris-migrator@1.0.2

### Patch Changes

- Updated dependencies
\[[`12dbc2cd8`](12dbc2c),
[`8ce6211c9`](8ce6211),
[`7a702388d`](7a70238)]:
    -   @shopify/polaris-tokens@9.1.0
    -   @shopify/stylelint-polaris@16.0.2

## @shopify/stylelint-polaris@16.0.2

### Patch Changes

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

## polaris-for-vscode@1.0.2

### Patch Changes

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

## polaris.shopify.com@1.0.5

### 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/polaris@13.3.0
    -   @shopify/polaris-tokens@9.1.0

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

@LA1CH3 LA1CH3 LA1CH3 approved these changes

@tatianau tatianau tatianau approved these changes

@ishanighosh1 ishanighosh1 ishanighosh1 approved these changes

@jesstelford jesstelford Awaiting requested review from jesstelford

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@sophschneider @LA1CH3 @tatianau @ishanighosh1