Skip to content

Commit

Permalink
[Sticky] Fix Sticky to update items when props change (#11947)
Browse files Browse the repository at this point in the history
### WHY are these changes introduced?

Fixes Shopify/archive-polaris-backlog-2024#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

- [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 Apr 29, 2024
1 parent 9c5b5b2 commit 995079c
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 0 deletions.
5 changes: 5 additions & 0 deletions .changeset/thin-paws-wait.md
@@ -0,0 +1,5 @@
---
'@shopify/polaris': patch
---

Fixed `Sticky` to update sticky items when props change
30 changes: 30 additions & 0 deletions polaris-react/src/components/Sticky/Sticky.tsx
Expand Up @@ -55,6 +55,36 @@ class StickyInner extends Component<CombinedProps, State> {
});
}

componentDidUpdate() {
const {
boundingElement,
offset = false,
disableWhenStacked = false,
stickyManager,
} = this.props;

if (!this.stickyNode || !this.placeHolderNode) return;

const stickyManagerItem = stickyManager.getStickyItem(this.stickyNode);
const didPropsChange =
!stickyManagerItem ||
boundingElement !== stickyManagerItem.boundingElement ||
offset !== stickyManagerItem.offset ||
disableWhenStacked !== stickyManagerItem.disableWhenStacked;

if (!didPropsChange) return;

stickyManager.unregisterStickyItem(this.stickyNode);
stickyManager.registerStickyItem({
stickyNode: this.stickyNode,
placeHolderNode: this.placeHolderNode,
handlePositioning: this.handlePositioning,
offset,
boundingElement,
disableWhenStacked,
});
}

componentWillUnmount() {
const {stickyManager} = this.props;
if (!this.stickyNode) return;
Expand Down
4 changes: 4 additions & 0 deletions polaris-react/src/utilities/sticky-manager/sticky-manager.ts
Expand Up @@ -67,6 +67,10 @@ export class StickyManager {
this.stickyItems.splice(nodeIndex, 1);
}

getStickyItem(node: HTMLElement) {
return this.stickyItems.find(({stickyNode}) => node === stickyNode);
}

setContainer(el: Document | HTMLElement) {
this.container = el;
if (isDocument(el)) {
Expand Down

0 comments on commit 995079c

Please sign in to comment.