fix(input-date-picker): Correctly position open component when scrolling #6815
Conversation
github-actions
bot
added
the
bug
| @@ -644,6 +644,7 @@ export class InputDatePicker | |||
|
|
|||
| transitionEl: HTMLDivElement; | |||
|
|
|||
| @Watch("open") | |||
There was a problem hiding this comment.
@jcfranco the problem seems to be when the input-date-picker is rendered the floatingEl has no parent yet. Not sure if its because it's not rendered or if some other stuff isn't ready yet.
So if we watch for open and then setup the floating ui again it can get all the scrolling parents correctly.
Seems like this is the only component affected.
| @@ -654,7 +654,7 @@ export class InputDatePicker | |||
| ? endWrapper || startWrapper | |||
| : startWrapper || endWrapper; | |||
|
|
|||
| connectFloatingUI(this, this.referenceEl, this.floatingEl); | |||
| requestAnimationFrame(() => connectFloatingUI(this, this.referenceEl, this.floatingEl)); | |||
There was a problem hiding this comment.
Need to wait for next frame to connect floating UI because some rendering is still going on. The rendering needs to happen first before trying to get all scrollable parents.
There was a problem hiding this comment.
Interesting. On a related note, I was thinking input-date-picker should be refactored to use popover for consistency and reusability. Maybe that would help eliminate since this doesn't appear to be an issue with the input-time-picker.
There was a problem hiding this comment.
yeah I think that makes sense since popover has a role of "dialog" and those two components seem to open dialogs. For things like combobox using the popover, we would need to be able to modify the role.
driskull
added
the
pr ready for visual snapshots
jcfranco
changed the title
| @@ -580,4 +580,39 @@ describe("calcite-input-date-picker", () => { | |||
| expect(changeEvent).toHaveReceivedEventTimes(1); | |||
| expect(await datepickerEl.getProperty("value")).toEqual(["2022-08-15", "2022-08-20"]); | |||
| }); | |||
|
|
|||
| it("should position on scroll when overlayPositioning is fixed", async () => { | |||
There was a problem hiding this comment.
Nice! Could you try using commonTests#floatingUIOwner? It should cover scrolling updates.
There was a problem hiding this comment.
The test already has a floatingUIOwner test. Maybe it should be updated to test with overlay-positioning="fixed" as well?
There was a problem hiding this comment.
Tried this and it passed so its not catching the issue.
it("owns a floating-ui 2", () =>
floatingUIOwner(
`<calcite-input-date-picker overlay-positioning="fixed" value="2022-11-27" min="2022-11-15" max="2024-11-15"></calcite-input-date-picker>`,
"open",
{ shadowSelector: ".menu-container" }
));There was a problem hiding this comment.
Hmm... I think the helper might need some tweaking. Can you create a follow-up issue for me? Let's proceed with the existing test here.
| @@ -654,7 +654,7 @@ export class InputDatePicker | |||
| ? endWrapper || startWrapper | |||
| : startWrapper || endWrapper; | |||
|
|
|||
| connectFloatingUI(this, this.referenceEl, this.floatingEl); | |||
| requestAnimationFrame(() => connectFloatingUI(this, this.referenceEl, this.floatingEl)); | |||
There was a problem hiding this comment.
Interesting. On a related note, I was thinking input-date-picker should be refactored to use popover for consistency and reusability. Maybe that would help eliminate since this doesn't appear to be an issue with the input-time-picker.
# Conflicts: # src/components/input-date-picker/input-date-picker.e2e.ts
github-actions
bot
added this to the 2023 April Priorities - No breaking changes milestone
Related Issue: #6463
Summary
Sets up floating UI when the component is opened in order to get all scrollable parents.