Refactor overlays to be positioned in the inverse direction #324
Conversation
|
|
||
| addDecorator(story => ( | ||
| <VerticalCenter style={{textAlign: 'left', padding: '50px', minHeight: isChromatic() ? null : '100vh', boxSizing: 'border-box', display: 'flex', justifyContent: 'center'}}> | ||
| <div style={{maxWidth: '100%'}}>{story()}</div> |
There was a problem hiding this comment.
Fixes story centering
| return ( | ||
| <Provider theme={theme} colorScheme={colorScheme} scale={scaleValue} locale={localeValue} toastPlacement={toastPositionValue} typekitId="pbi5ojv"> | ||
| <div style={{paddingTop: '20px', paddingLeft: '20px', paddingRight: '20px'}}><div style={{fontSize: '18px', paddingLeft: '10px', paddingRight: '10px'}}><strong>Status</strong></div><StatusLight variant={props.options.status || 'negative'}>{statusMap[props.options.status || 'negative']}</StatusLight></div> | ||
| <div style={{position: 'absolute', paddingTop: '20px', paddingLeft: '20px', paddingRight: '20px'}}><div style={{fontSize: '18px', paddingLeft: '10px', paddingRight: '10px'}}><strong>Status</strong></div><StatusLight variant={props.options.status || 'negative'}>{statusMap[props.options.status || 'negative']}</StatusLight></div> |
There was a problem hiding this comment.
Prevents all the stories from scrolling
|
Build successful! 🎉 |
| let onBlur = useCallback((e: FocusEvent) => { | ||
| isFocusWithin.current = ref.current.contains(e.relatedTarget as Element); | ||
| }, []); | ||
| }, [ref]); |
There was a problem hiding this comment.
I think @LFDanLu figured this should be ref.current?
There was a problem hiding this comment.
Then we'd be making a new function every time the ref changed value. ref is mutated so I think this is correct as it is.
| }; | ||
|
|
||
| // Reset JSDom's default margin on the body | ||
| document.body.style.margin = 0; |
There was a problem hiding this comment.
would this be useful to do globally during test setup instead?
|
|
||
| let contents = ( | ||
| <Provider ref={ref} UNSAFE_style={{position: 'absolute', zIndex: 100000, top: 0, left: 0}}> | ||
| <Provider ref={ref} UNSAFE_style={{background: 'transparent'}}> |
There was a problem hiding this comment.
background transparent? curious what this is for
There was a problem hiding this comment.
Provider has a background in spectrum css.
There was a problem hiding this comment.
Actually, maybe not needed anymore. I think it was when I had it absolute positioned over the whole page.
There was a problem hiding this comment.
Turns out it's needed otherwise Provider doesn't render a DOM node at all, and just acts as a context provider... We can revisit this.
| classNames( | ||
| styles, | ||
| 'spectrum-Popover', | ||
| `spectrum-Popover--${placement.split(' ')[0]}`, |
…rse-positioning # Conflicts: # packages/@react-spectrum/dialog/src/Dialog.tsx # packages/@react-spectrum/menu/src/MenuTrigger.tsx # packages/@react-spectrum/overlays/src/Popover.tsx
|
Build successful! 🎉 |
|
Build successful! 🎉 |
This refactors our overlay positioning code to position overlays in the inverse direction from the placement. For example, when placement is
top, position using thebottomCSS property. This allows us to avoid measuring the overlay size to determine the position, which allows the overlay to resize after opening without needing to be repositioned. This is important for menus that update dynamically while they are open, e.g. autocomplete.In addition, the flipping logic is improved so that rather than only flipping if the entire overlay would fit in the opposite direction, it flips if it doesn't fit in the specified direction and there is more space available in the opposite direction. This is much better for e.g. long popups at the bottom of the screen - rather than getting a tiny menu at the bottom of the screen, it will flip to the opposite direction so more of the list can be seen at once. This is now also determined based on the scroll height of the overlay (or a child inside) rather than the height, which should be much better as well.
Finally, this gets rid of the double absolute positioning we had which caused squished menus by measuring based on the
offsetParent(e.g.document.body) rather than the immediate container.