[EuiDatePickerRange] Respect endDateControl className prop#5329
[EuiDatePickerRange] Respect endDateControl className prop#5329thompsongl merged 7 commits intoelastic:masterfrom
endDateControl className prop#5329Conversation
cee-chen
left a comment
There was a problem hiding this comment.
Perfection!! Great spot with the className fix as well!
|
@thompsongl Actually, sorry, 1 quick thought - do we want to add a small unit test for this? e.g., it('uses individual EuiDatePicker props', () => {
const component = render(
<EuiDatePickerRange
startDateControl={
<EuiDatePicker popoverPlacement="left" className="hello" />
}
endDateControl={
<EuiDatePicker popoverPlacement="left" className="world" />
}
{...requiredProps}
/>
);
expect(component).toMatchSnapshot();
}); |
|
Can't argue with adding a test that's copy-paste-able |
…nto 5328-datepicker
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_5329/ |
|
Gah I did check the output locally, but apparently still had my override in place |
|
Yeah sorry, I think I meant specifically we need to check the published CI build. There could be other local issues that don't show up until you view the that version. |
I'm not sure we'd also want them to change the left/right position either. We may need to make this a custom prop asking for top/bottom position that affect both the start and end controls in one prop. |
It looks like changing left/right is what they want, not top/bottom: elastic/kibana#116471 Maybe there's a way to expand the <EuiDatePickerRange isCustom={{popoverPlacement: true}} /> // maybe `false` makes more sense?Resulting in |
|
I've asked for clarification: elastic/kibana#116471 (comment) |
|
Ugh, btw, none of this would be a problem if the popover was just one of ours inside of a portal. |
Ughhhh you're right |
|
Found her a workaround. I think the ideal solution is to get ours rendering in a popover. We can do that by splitting the input from the calendar and putting in inline version in a popover. We do something similar in EuiSuperDatePicker. I started to explore the option in this codesandbox. |
|
Thanks, @cchaos! I had the idea to use I'm going to scale this PR back to just the className fix and we can open a new PR with either your idea or a broader component reorg. |
endDateControl popoverPlacement propendDateControl className prop
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_5329/ |
cchaos
left a comment
There was a problem hiding this comment.
👍 Maybe update the PR description too?


Summary
Use the
classNameprop from the correct element (endDateControl)Checklist
Check against all themes for compatibility in both light and dark modes
- [ ] Checked in mobile- [ ] Checked in Chrome, Safari, Edge, and Firefox- [ ] Props have proper autodocs and playground toggles- [ ] Added documentation- [ ] Checked Code Sandbox works for any docs examplesAdded or updated jest and cypress tests
Checked for breaking changes and labeled appropriately
Checked for accessibility including keyboard-only and screenreader modes
A changelog entry exists and is marked appropriately