DatePicker Calendar is not shown when clicking on the Icon when using MediaInput as a trigger
abdallahzakzouk opened this issue ยท 7 comments
Expectations
Given a usage of Media Input inside a Date Picker as a trigger input, and having an icon as End, Start, or both,
When I click on the Icon, I am expecting the input to be focused and the Calendar to be shown
Reality
On the previous scenario the Input is focused correctly, but the calendar is NOT shown
Steps to Reproduce
- Use Media Input as a trigger inside Date Picker
- Add as Icon as End Icon for the media Input
- View the result, Click on the Icon
Fine Print
- Component: Date Picker, Media Input
- Version: 8.37.1
- Browsers: all
This can be helpful, from my investigation, I see that in file https://github.com/zendeskgarden/react-components/blob/main/packages/datepickers/src/elements/Datepicker/Datepicker.tsx - Line 202
...
/** Ensure click/focus events from associated labels are not triggered */
if (isInputMouseDownRef.current && !state.isOpen) {
...
This ref and the condition for it are showing the Calendar only on clicking on the Input element itself, to not open the Calendar when clicking on the Wrapper that also include the Label, this may be changed to include any click in the field wrapper including the icons and ignoring only the outer content like Label
Hi @abdallahzakzouk. I removed the isInputMouseDownRef.current
conditional in a test branch and the Calendar icon remains un-clickable unfortunately. Here is the branch 8914498. If you have time to investigate, perhaps pull down this branch and experiment with different methods to trigger the open event.
And of course, if things come up and let us know. We can add this one to our team's backlog. ๐
After looking into this, we added this issue into our backlog for Garden attention. Thanks for reporting this! ๐
I double checked and yes you are right, I found that it should be handled from MediaInput component, not from DatePicker
Thank you Michael for your consideration :)
The only child provided to Datepicker
is cloned with events and props attached directly to it. The (incorrect Garden) assumption is that this child has always been an <input />
. In the case of a MediaInput
clone, the only child is a div
with child input
and svg
(s) siblings. In this scenario, a click on the svg
does not bubble to the input
. So the datepicker needs a little help to know how to configure the clone. That's where the refKey
prop comes into play in order to point at the outer MediaInput
div wrapper. Since "wrapperRef" is the prop of interest on the MediaInput API, we want something like this:
<Field>
<Label>{Datepicker.displayName}</Label>
<Datepicker {...args} formatDate={formatDate} isCompact={isCompact} refKey="wrapperRef">
<MediaInput
end={<Icon />}
isCompact={isCompact}
wrapperProps={{ style: { width: isCompact ? 256 : 320 } }}
/>
</Datepicker>
</Field>
But this only gets us part-way there. The Garden Datepicker
needs to be fixed so that events of interest are attached to the wrapperRef.current
rather than to the default. My hypothesis is that we get there by moving the onMouseDown
, onMouseUp
, and onClick
logic into this block, connected to a valid refValue
:
react-components/packages/datepickers/src/elements/Datepicker/Datepicker.tsx
Lines 189 to 192 in 1d5b1df
Local testing bears out this solution. But more would need to be done to thoroughly unit test and also check DatepickerRange
for similar defects/fixes.
If we determine that double-dipping on refKey
is undesirable (I can imagine some use cases where tying a click
to that ref would be wrong), then another option would be to add a new prop, perhaps triggerPropsKey
, that could be used to drop the click events into the proper object. If the prop is undefined
, then leave the code as-is.
Thank you @jzempel for your investigation, I think having an extra prop to be used when needed is a good approach if it's not possible to force the behaviour for all cases, please let me if know if something is needed from my side to help doing this change :)