chore(DatePicker): updated controlled examples #9171
Conversation
|
Preview: https://patternfly-react-pr-9171.surge.sh A11y report: https://patternfly-react-pr-9171-a11y.surge.sh |
There was a problem hiding this comment.
This looks great @thatblindgeye and the updates work as expected. While testing I seem to have found a bug with the demos page and I'm curious if you're able to reproduce this - the page seems to jump to the wrong location when the calendar icon is clicked, though the location seems pretty inconsistent
date.picker.mov
|
@jenny-s51 I do get that bug as well. It looks like it's first getting positioned at the bottom of the page first:
Before getting positioned relative to the datepicker input:
(hate to pile onto poor Popper more but this might be another issue with Popper...) |
|
is the |
|
@nicolethoen based on the PR when the logic was first added yes, but it looks like it's been broken in v4 as well. Looks like it might be a couple of things:
|
|
Should we either remove the example for fix it? Maybe as a follow up issue? Seems to be revealing that there is a preexisting bug in the DatePicker. WDYT @tlabaj ? |
|
@thatblindgeye @nicolethoen yes this issue existed in v4. @mcarrano had expresses that it would be nice to get this one fixed for the v5 release. |
thatblindgeye
force-pushed
the
iss9136_datePickerAlignment
branch
from
6b03e68
to
8efb6ef
Compare
| toggleCalendar: (setOpen?: boolean, eventKey?: string) => { | ||
| if (eventKey === KeyTypes.Escape && popoverOpen && !selectOpen) { | ||
| setPopoverOpen((prev) => (setOpen !== undefined ? setOpen : !prev)); | ||
| } | ||
| toggleCalendar: (setOpen?: boolean) => { | ||
| setPopoverOpen((prev) => (setOpen !== undefined ? setOpen : !prev)); |
There was a problem hiding this comment.
@nicolethoen @tlabaj Pushed an update that makes the example work as intended and added an integration test for the functionality.
I removed the eventKey param as it wasn't being used in the example when added, and it doesn't seem like it would be necessary any more? I tested the datepicker in modal demo and pressing Escape to close the datepicker didn't cause the issue of also closing the modal.
| if (popoverOpen) { | ||
| event.stopPropagation(); | ||
| setPopoverOpen(false); | ||
| hideFunction(); | ||
| } |
There was a problem hiding this comment.
This prevents the CalendarMonth from opening again when pressing the "Toggle" button to close it, and also allows clicking outside the CalendarMonth to close it.
There was a problem hiding this comment.
The flashing calendar problem is so apparent in these examples. thats out of scope for this issue, and is reported in this issue and this issue.
|
Your changes have been released in:
Thanks for your contribution! 🎉 |
* chore(DatePicker): updated controlled examples * Updated logic for controlled example
What: Closes #9136
Additional issues: