Page resize observer & Toolbar/PageHeaderTools responsiveness based on page width#6827
Conversation
| if (useResizeObserver) { | ||
| this.observer = getResizeObserver(this.pageRef.current, this.handleResize); | ||
| } else { | ||
| window.addEventListener('resize', this.handleResize); |
There was a problem hiding this comment.
Curious why we offer useResizeObserver as a choice?
Note that getResizeObserver will use a window event listener if the this.pageRef.current ref is not available.
There was a problem hiding this comment.
hmm, I suppose we can just enable the observer by default without providing a prop. And will update the code to remove the extra listener, thanks!
mattnolting
left a comment
There was a problem hiding this comment.
Fantastic work @jschuler! L🤖 TM!
| ) => | ||
| Object.entries(mods || {}) | ||
| stylePrefix: string = '', | ||
| breakpoint?: 'xs' | 'sm' | 'md' | 'lg' | 'xl' | '2xl' |
There was a problem hiding this comment.
My only feedback, I'd like to see xs -> default until we have an actual value for xs.
There was a problem hiding this comment.
@jschuler I think that at least the stacked toolbar example is behaving differently after this update. Can you just verify that the toolbar examples are all behaving as you'd expect with the updates?
The stacked example is now collapsing the ToolbarToggleGroup at full page width - it didn't do that before.
| export const getBreakpoint = (width: number): 'default' | 'sm' | 'md' | 'lg' | 'xl' | '2xl' => { | ||
| if (width === null) { | ||
| width = canUseDOM ? window.innerWidth : 1200; | ||
| return null; |
There was a problem hiding this comment.
This does cause a classname to get applied - pf-m-breakpoint-null so I feel like that's not quite ideal?
There was a problem hiding this comment.
thanks, I added a check in the page component to make sure width is defined first
|
Your changes have been released in:
Thanks for your contribution! 🎉 |
What: Towards #6030
PageContext. The width is the width of the Page (pf-c-page) container. Children components can use the context to base logic around that (like the Toolbar components in this PR). The getBreakpoint function defaults to the one inutil.ts, but can be overwritten if desired.pf-m-breakpoint-[default|sm|md|lg|xl|2xl]class as a sibling to the Page (pf-c-page) container. This allows for CSS to target page based breakpoints as described in this issueA new util function
getBreakpointwas added to return the breakpoint for a given width.The
util.tsformatBreakpointMods function was updated to accept abreakpointargument.Before, you would have a string of class names that are media query based, i.e.
pf-m-hidden pf-m-visible-on-md pf-m-hidden-on-lg, orpf-m-spacer-none pf-m-spacer-md-on-md, etc. If thebreakpointargument is supplied, we instead return now a single class based on the known width.To test this, I added a lightbulb icon button to the masthead which toggles a drawer in this demo:
https://patternfly-react-pr-6827.surge.sh/components/page/react-demos/sticky-section-group/
Additional issues/PRs: patternfly/patternfly#4622, patternfly/patternfly#4625
TBD: More PRs to follow to update components that use the formatBreakpointMods function, or set breakpoint based modifier classes
Demo (note, this also contains the change from core here patternfly/patternfly#4625 which is not yet merged):
Screen.Recording.2022-01-19.at.3.36.42.PM.mov