Drop - flip drop position if part of the drop isn't visible #6611
Conversation
… visible and there is more room in the opposite direction
|
one thing I saw in the storybook is since we have |
| const percentVisibleAreaBelow = | ||
| 100 - (targetRect.bottom / windowHeight) * 100; | ||
| const alignToTop = align.top === 'top'; | ||
| const switchTop = !alignToTop ? targetRect.top : targetRect.bottom; |
There was a problem hiding this comment.
just opinion not sure about the naming for switchTop
There was a problem hiding this comment.
Agree the name is confusing. I changed it to switchTarget since this props is new target position the drop will be aligned to if we flip the drop.
| // when percentVisibleAreaBelow>20%. | ||
|
|
||
| if (windowHeight === top || percentVisibleAreaBelow <= 20) { | ||
| if ( |
There was a problem hiding this comment.
can we add a new comment for this section
the old comment was clear but since the behavior changed just for future reference
Changed the story to have |
| } else { | ||
| ({ bottom } = targetRect); | ||
| } | ||
| bottom = alignToTop ? targetRect.bottom : targetRect.top; | ||
| maxHeight = bottom; | ||
| container.style.maxHeight = `${maxHeight}px`; |
There was a problem hiding this comment.
How come sometimes we set container.style.maxHeight without honoring preserveHeight? I was expecting we'd only set the style within if (!preserveHeight) { later down.
| const switchTarget = !alignToTop | ||
| ? targetRect.top | ||
| : targetRect.bottom; | ||
| top = alignToTop ? targetRect.top : targetRect.bottom; |
There was a problem hiding this comment.
My head is a bit confused trying to keep track of all of this. I'm wondering if there's a way to make it a bit clearer, perhaps by renaming some variables, or perhaps by restructuring the code. For instance, would it be clearer if we just checked if (align.top === 'top') { and then put everything for that case under it and then } else { for the other version, rather than trying to merge them together?
What does this PR do?
Changed Drop behavior to flip the drop if part of the drop isn't visible and there is more room in the opposite direction.
I also set the
responsiveprop to true by default. This aligns with what the documentation shows.Where should the reviewer start?
What testing has been done on this PR?
How should this be manually tested?
Do Jest tests follow these best practices?
screenis used for querying.userEventis used in place offireEvent.asFragment()is used for snapshot testing.Any background context you want to provide?
What are the relevant issues?
Closes #6495
Screenshots (if appropriate)
Do the grommet docs need to be updated?
No
Should this PR be mentioned in the release notes?
Yes
Is this change backwards compatible or is it a breaking change?
Backwards compatible