only make order summary sticky when it's not longer than view #47680
Conversation
woocommercebot
requested review from
a team and
wavvves
and removed request for
a team
senadir
added
type: bug
|
Hi @nielslange, @wavvves, Apart from reviewing the code changes, please make sure to review the testing instructions as well. You can follow this guide to find out what good testing instructions should look like: |
|
Hi @wavvves, Apart from reviewing the code changes, please make sure to review the testing instructions as well. You can follow this guide to find out what good testing instructions should look like: |
| const element = observedRef.current; | ||
| const resizeObserver = new ResizeObserver( ( entries ) => { | ||
| entries.forEach( ( entry ) => { | ||
| if ( entry.target === element ) { | ||
| const { height, width } = entry.contentRect; | ||
| setObservedElement( { height, width } ); | ||
| } | ||
| } ); | ||
| } ); |
There was a problem hiding this comment.
This resizer will track the element height changes (collapse vs expand, add items to it...).
| const intersectionObserver = new IntersectionObserver( | ||
| ( entries ) => { | ||
| entries.forEach( ( entry ) => { | ||
| const { height, width } = entry.boundingClientRect; | ||
| setObservedElement( { height, width } ); | ||
| if ( entry.target.ownerDocument.defaultView ) { | ||
| setViewWindow( { | ||
| height: entry.target.ownerDocument.defaultView | ||
| ?.innerHeight, | ||
| width: entry.target.ownerDocument.defaultView | ||
| ?.innerWidth, | ||
| } ); | ||
| } | ||
| } ); | ||
| }, | ||
| { | ||
| root: null, | ||
| rootMargin: '0px', | ||
| threshold: 1, | ||
| } | ||
| ); |
There was a problem hiding this comment.
This will observe the element when it intersect with the page and is visible in it, useful for cases where the page size changes.
Test using WordPress PlaygroundThe changes in this pull request can be previewed and tested using a WordPress Playground instance. Test this pull request with WordPress Playground. Note that this URL is valid for 30 days from when this comment was last updated. You can update it by closing/reopening the PR or pushing a new commit. |
| @@ -0,0 +1,4 @@ | |||
| Significance: patch | |||
| Type: fix | |||
| Comment: This is a follow up to another PR not yet shipped. | |||
There was a problem hiding this comment.
Hi Nadir, quick question, can you link the other PR please? Can this be reviewed and merged first?
There was a problem hiding this comment.
The other PR is already merged
github-actions
bot
added
the
plugin: woocommerce
senadir
force-pushed
the
fix/only-sticky-summary-if-smaller-than-screen
branch
from
5140f0f
to
3746955
Compare
There was a problem hiding this comment.
Thanks for working on this, @senadir. I've tested this PR against the following browsers:
- Chrome 14.3
- Firefox 126.0
- Safari 17.3
- Microsoft Edge 125.0.2535.51
The behaviour is consistent across all browsers. However, there is a variation in behaviour depending on whether the user is logged in or a guest.
| Guest user:
|
Logged-in user:
|
There was a problem hiding this comment.
Looks good to me - please add a followup issue to fix the TS error on line 19 of checkout-totals-block/frontend
github-actions
bot
added
the
needs: analysis
nigeljamesstevenson
added
needs: external testing
This PR changes when we mark an element as sticky, for the order summary, we should only make it sticky if its height is smaller than the viewport. For this, I created a new hook
useObservedViewportthat should help detect its height and other elements' height. This will be needed once we start solving other zoom issues https://github.com/woocommerce/woocommerce/issues?q=is:open+is:issue+label:%22focus:+accessibility%22+label:%22team:+Rubik%22+zoomHow to test the changes in this Pull Request:
Changelog entry
Significance
Type
Message
Comment
This is a follow up to another PR not yet shipped.