fix(datetimepickerv2): flyout menu auto positioning #3608
Conversation
✅ Deploy Preview for carbon-addons-iot-react ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for ai-apps-pal-angular ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
|
@jessieyan This usually needs to have it rendered in a portal, in order to break out of an overflow:hidden container, like our cards we need to be able to do so and still have the dropdown stay with the trigger. We can either close the portal on scroll or adjust the offset so it scrolls with the element. We have components in the codebase that manipulate the offset that you can try to look for. |
davidicus
added
status: waiting for author's response 💬
and removed
status: ready for review 👀
labels
@davidicus Do you mean the offered solution in issue 3362 is not the right solution? |
|
We can surface a prop to allow people to render in portal or not but we need to have the dropdown not detach even when it is rendered in portal. |
jessieyan
changed the title
jessieyan
added
status: WIP 👩💻
and removed
status: waiting for author's response 💬
labels
|
@davidicus Change this PR to only resolve the issue #3618 which is to enable the auto positioning |
jessieyan
changed the title
|
@davidicus I saw you left a review comment earlier but I couldn't find it in the PR. |
I had deleted the comment after I saw you were returning a function. Was supposed to be part of the review. |
| button.getBoundingClientRect = generateBoundingClientRect({ | ||
| x: 20, | ||
| y: 20, | ||
| }); |
There was a problem hiding this comment.
Should this be part of the mock implementation? One line 2035 we are setting getBoundingClientRect to a jest mocked function with one value and then resetting it here (2067) to just a function that returns another value.
| ).toHaveLength(0); | ||
| }); | ||
|
|
||
| it('should render in a portal when renderInPortal:true (new time spinner)', () => { |
There was a problem hiding this comment.
In the future we can probably combine these types of variations into one test and use the rerender method to rerender the component with the new prop.
| }); | ||
|
|
||
| const button = screen.getByTestId(`${testId}-datepicker-flyout-button`); | ||
| button.getBoundingClientRect = generateBoundingClientRect({ |
There was a problem hiding this comment.
Same comment about having this set in the mock.
| }); | ||
|
|
||
| const button = screen.getByTestId(`${testId}-datepicker-flyout-button`); | ||
| button.getBoundingClientRect = generateBoundingClientRect({ |
| }); | ||
|
|
||
| const button = screen.getByTestId(`${testId}-datepicker-flyout-button`); | ||
| button.getBoundingClientRect = generateBoundingClientRect({ |
There was a problem hiding this comment.
Same comment about mock in all these test. It is possible that I am not understanding what you are doing here.
| @@ -67,6 +67,7 @@ const defaultProps = { | |||
| closeLabel: 'Close', | |||
| expandLabel: 'Expand to fullscreen', | |||
| }, | |||
| renderInPortal: false, | |||
There was a problem hiding this comment.
Prop should be more specific since it is for a child component. Should match the name of Card component prop
There was a problem hiding this comment.
This should match the Card prop, no?
|
There is some weird positioning if you set the padding to the storybook container to push the element all the way to the end of the page and then click the dropdown. The dropdown renders on top of element but still causes more scroll. Once you click on the date picker to choose a date it jumps up to the top of the element. I would expect this to be rendered on top when initially opened. When there is an overflow hidden on the container and the DateTimePicker dropdown is not rendered in the portal the dropdown is cut off |
|
Issue with Card component and the DateTimePicker in this story
|
|
I can see if set the padding-left to 80 rem. Then a horizontal bar showed up. The flyout menu will adjust its position if the horizontal bar moves. If set padding-left to 71 rem, The flyout menu will pop up on top of the input and will not move its position. |
Opened #3620 |
Closes #3618
Summary
renderInPortalprop so that user can set it to false for some scenariosuseAutoPositioningprop to reposition flyout menu if offscreenChange List (commits, features, bugs, etc)
added
renderInPortalprop in DateTimePickerV2, Card, TableCard, CardToolBaradded
useAutoPositioningprop in DateTimePickerV2fixed the menuOffSet when useAutoPositioning is enabled and in RTL. The passed-in
menuOffsetprop only works for the passed-indirectionprop. If repositioned, thedirectionwill be changed,menuOffsetno long apply to the newdirectionbefore:
after:
fixed the menuOffSet.left. set it to 288px in RTL
Before,
After:
useAutoPositioningandrenderInPortaluseAutoPositioningin single select and range select stories.Acceptance Test (how to verify the PR)
go to range select or single select story
set renderInPortal to false, we should see
flyout-tooltipis grandchild ofdate-time-picker-datepicker-flyout-containerset renderInPortal to true,
flyout-tooltipis independent element outside ofdate-time-picker-datepicker-flyout-containerThings to look for during review
iotorbxclass prefix is using the prefix variabledata-testidattribute. New test ids should have test written to ensure they are not changed or removed.