-
Notifications
You must be signed in to change notification settings - Fork 677
fix(SelectPanel): Correctly recalculate position on overflow #5562
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.
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
Merged
Merged
Changes from all commits
Commits
Show all changes
32 commits
Select commit
Hold shift + click to select a range
80dfbfd
wip: SelectPanel overflow
francinelucca 25920d6
Merge branch 'main' into francinelucca/select-panel-overflow
francinelucca a47034c
Merge branch 'main' of github.com:primer/react into francinelucca/sel…
francinelucca b863080
fix(useAnchoredPosition): refine reposition logic
francinelucca 382e4d1
Merge branch 'main' into francinelucca/select-panel-overflow
francinelucca d792c50
Create silent-cameras-care.md
francinelucca dcf559d
fix: lint
francinelucca 0834641
merge branch 'francinelucca/select-panel-overflow' of github.com:prim…
francinelucca ef2930d
test(AnchoredOverlay): update snapshot
francinelucca 59b9ce5
reorganize stories
francinelucca 3e70f11
fix(Overlay): add default max height
francinelucca ed5fbbe
fix(SelectPanel): revert preventOverflow changes
francinelucca 9a0fb5a
test(vrt): update snapshots
francinelucca c2f2dcc
Merge branch 'main' into francinelucca/select-panel-overflow
francinelucca ef73210
Revert "test(vrt): update snapshots"
francinelucca 81601de
Merge branch 'main' into francinelucca/select-panel-overflow
francinelucca 08a9f9c
test(vrt): update snapshots
francinelucca 961eb39
Merge branch 'main' into francinelucca/select-panel-overflow
francinelucca ed98b1f
fix(useResizeObserver): SSR compatibility
francinelucca 7c812cc
Revert "test(vrt): update snapshots"
francinelucca 87201d2
test(vrt): update snapshots
francinelucca 7701ebd
Merge branch 'main' of github.com:primer/react into francinelucca/sel…
francinelucca 6cb8394
Merge branch 'main' of github.com:primer/react into francinelucca/sel…
francinelucca 5c5f4fc
fix(SelectPanel): fix flashing race condition and cleanup code
francinelucca 971d1a9
docs(AnchoredOverlay): document new pinPosition pro
francinelucca 02b81d4
fix tests
francinelucca 35d58aa
fix tests
francinelucca d16252b
fix tests
francinelucca 8c1e8cf
test(vrt): update snapshots
francinelucca 7615786
Revert "test(vrt): update snapshots"
francinelucca dc18438
remove test code
francinelucca c874e45
Merge branch 'main' into francinelucca/select-panel-overflow
francinelucca File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "@primer/react": patch | ||
| --- | ||
|
|
||
| fix(SelectPanel): Correctly recalculate position on overflow |
Binary file modified
BIN
+83 Bytes
(100%)
...ponents/Overlay.test.ts-snapshots/Overlay-SX-Props-dark-high-contrast-linux.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified
BIN
+106 Bytes
(100%)
...onents/Overlay.test.ts-snapshots/Overlay-SX-Props-light-high-contrast-linux.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
110 changes: 110 additions & 0 deletions
110
packages/react/src/AnchoredOverlay/AnchoredOverlay.dev.stories.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,110 @@ | ||
| import type {Meta} from '@storybook/react' | ||
| import React, {useState} from 'react' | ||
|
|
||
| import {Button} from '../Button' | ||
| import {AnchoredOverlay} from '.' | ||
| import {Stack} from '../Stack' | ||
| import {Dialog, Spinner} from '..' | ||
|
|
||
| const meta = { | ||
| title: 'Components/AnchoredOverlay/Dev', | ||
| component: AnchoredOverlay, | ||
| } satisfies Meta<typeof AnchoredOverlay> | ||
|
|
||
| export default meta | ||
|
|
||
| export const RepositionAfterContentGrows = () => { | ||
| const [open, setOpen] = useState(false) | ||
|
|
||
| const [loading, setLoading] = useState(true) | ||
|
|
||
| React.useEffect(() => { | ||
| window.setTimeout(() => { | ||
| if (open) setLoading(false) | ||
| }, 2000) | ||
| }, [open]) | ||
|
|
||
| return ( | ||
| <Stack direction="vertical" justify="space-between" style={{height: 'calc(100vh - 200px)'}}> | ||
| <div> | ||
| What to expect: | ||
| <ul> | ||
| <li>The anchored overlay should open below the anchor (default position)</li> | ||
| <li>After 2000ms, the amount of content in the overlay grows</li> | ||
| <li>the overlay should reposition itself above the anchor so that it stays inside the window</li> | ||
| </ul> | ||
| </div> | ||
| <AnchoredOverlay | ||
| renderAnchor={props => ( | ||
| <Button {...props} sx={{width: 'fit-content'}}> | ||
| Button | ||
| </Button> | ||
| )} | ||
| open={open} | ||
| onOpen={() => setOpen(true)} | ||
| onClose={() => { | ||
| setOpen(false) | ||
| setLoading(true) | ||
| }} | ||
| > | ||
| {loading ? ( | ||
| <> | ||
| <Spinner /> | ||
| loading for 2000ms | ||
| </> | ||
| ) : ( | ||
| <div style={{height: '300px'}}>content with 300px height</div> | ||
| )} | ||
| </AnchoredOverlay> | ||
| </Stack> | ||
| ) | ||
| } | ||
|
|
||
| export const RepositionAfterContentGrowsWithinDialog = () => { | ||
| const [open, setOpen] = useState(false) | ||
|
|
||
| const [loading, setLoading] = useState(true) | ||
|
|
||
| React.useEffect(() => { | ||
| window.setTimeout(() => { | ||
| if (open) setLoading(false) | ||
| }, 2000) | ||
| }, [open]) | ||
|
|
||
| return ( | ||
| <Dialog onClose={() => {}}> | ||
| <Stack direction="vertical" justify="space-between" style={{height: 'calc(100vh - 300px)'}}> | ||
| <div> | ||
| What to expect: | ||
| <ul> | ||
| <li>The anchored overlay should open below the anchor (default position)</li> | ||
| <li>After 2000ms, the amount of content in the overlay grows</li> | ||
| <li>the overlay should reposition itself above the anchor so that it stays inside the window</li> | ||
| </ul> | ||
| </div> | ||
| <AnchoredOverlay | ||
| renderAnchor={props => ( | ||
| <Button {...props} sx={{width: 'fit-content'}}> | ||
| Button | ||
| </Button> | ||
| )} | ||
| open={open} | ||
| onOpen={() => setOpen(true)} | ||
| onClose={() => { | ||
| setOpen(false) | ||
| setLoading(true) | ||
| }} | ||
| > | ||
| {loading ? ( | ||
| <> | ||
| <Spinner /> | ||
| loading for 2000ms | ||
| </> | ||
| ) : ( | ||
| <div style={{height: '300px'}}>content with 300px height</div> | ||
| )} | ||
| </AnchoredOverlay> | ||
| </Stack> | ||
| </Dialog> | ||
| ) | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,6 +8,7 @@ import useLayoutEffect from '../utils/useIsomorphicLayoutEffect' | |
| export interface AnchoredPositionHookSettings extends Partial<PositionSettings> { | ||
| floatingElementRef?: React.RefObject<Element> | ||
| anchorElementRef?: React.RefObject<Element> | ||
| pinPosition?: boolean | ||
|
camertron marked this conversation as resolved.
|
||
| } | ||
|
|
||
| /** | ||
|
|
@@ -30,22 +31,61 @@ export function useAnchoredPosition( | |
| const floatingElementRef = useProvidedRefOrCreate(settings?.floatingElementRef) | ||
| const anchorElementRef = useProvidedRefOrCreate(settings?.anchorElementRef) | ||
| const [position, setPosition] = React.useState<AnchorPosition | undefined>(undefined) | ||
| // eslint-disable-next-line @typescript-eslint/no-unused-vars | ||
| const [_, setPrevHeight] = React.useState<number | undefined>(undefined) | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. used to force the height of the selectPanel when filtered and it wants to shrink but it's anchored at the top (we want it to stay at the top and lock the height) |
||
|
|
||
| const topPositionChanged = (prevPosition: AnchorPosition | undefined, newPosition: AnchorPosition) => { | ||
| return ( | ||
| prevPosition && | ||
| ['outside-top', 'inside-top'].includes(prevPosition.anchorSide) && | ||
| // either the anchor changed or the element is trying to shrink in height | ||
| (prevPosition.anchorSide !== newPosition.anchorSide || prevPosition.top < newPosition.top) | ||
| ) | ||
| } | ||
|
|
||
| const updateElementHeight = () => { | ||
| let heightUpdated = false | ||
| setPrevHeight(prevHeight => { | ||
| // if the element is trying to shrink in height, restore to old height to prevent it from jumping | ||
| if (prevHeight && prevHeight > (floatingElementRef.current?.clientHeight ?? 0)) { | ||
| requestAnimationFrame(() => { | ||
| ;(floatingElementRef.current as HTMLElement).style.height = `${prevHeight}px` | ||
| }) | ||
| heightUpdated = true | ||
| } | ||
| return prevHeight | ||
| }) | ||
| return heightUpdated | ||
| } | ||
|
|
||
| const updatePosition = React.useCallback( | ||
| () => { | ||
| if (floatingElementRef.current instanceof Element && anchorElementRef.current instanceof Element) { | ||
| setPosition(getAnchoredPosition(floatingElementRef.current, anchorElementRef.current, settings)) | ||
| const newPosition = getAnchoredPosition(floatingElementRef.current, anchorElementRef.current, settings) | ||
| setPosition(prev => { | ||
| if (settings?.pinPosition && topPositionChanged(prev, newPosition)) { | ||
| const anchorTop = anchorElementRef.current?.getBoundingClientRect().top ?? 0 | ||
| const elementStillFitsOnTop = anchorTop > (floatingElementRef.current?.clientHeight ?? 0) | ||
|
|
||
| if (elementStillFitsOnTop && updateElementHeight()) { | ||
| return prev | ||
| } | ||
| } | ||
| return newPosition | ||
| }) | ||
| } else { | ||
| setPosition(undefined) | ||
| } | ||
| setPrevHeight(floatingElementRef.current?.clientHeight) | ||
| }, | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| [floatingElementRef, anchorElementRef, ...dependencies], | ||
| ) | ||
|
|
||
| useLayoutEffect(updatePosition, [updatePosition]) | ||
|
|
||
| useResizeObserver(updatePosition) | ||
| useResizeObserver(updatePosition) // watches for changes in window size | ||
| useResizeObserver(updatePosition, floatingElementRef as React.RefObject<HTMLElement>) // watches for changes in floating element size | ||
|
|
||
| return { | ||
| floatingElementRef, | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add this to
AnchoredOverlay.docs.json?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes! Done!!