-
Notifications
You must be signed in to change notification settings - Fork 841
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[EuiDatePicker] Add right padding to account for clear icon #5095
Conversation
- Should only appear when `onClear` is passed, and if a date is present (`selected`)
@cchaos I'm not sure how many scenarios we anticipate having small date pickers in, but I'm actually wondering if there's now too much padding on the right and if it's making the text harder to read 😅 I'm wondering if instead of using the If you don't see any issues with it though, then no worries! |
Preview documentation changes for this PR: https://eui.elastic.co/pr_5095/ |
Mainly it's about consistency and maintenance. I'd rather keep using the mixin and decide if, overall, we should just reduce that padding. But let's not break it from the norm in this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implementation is great and fixed the problem!
My following suggestions are just some good patterns we follow.
Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com>
Preview documentation changes for this PR: https://eui.elastic.co/pr_5095/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks for the quick fix!
Thank you for the super easy code suggestions! 🎉 |
Preview documentation changes for this PR: https://eui.elastic.co/pr_5095/ |
Summary
closes #5082
Before
Currently, there's no padding/spacing set to account for the "clear" icon on
EuiDatePicker
(which appears when anonClear
function is passed):After
The solution I went with was mimicking the padding set for the left icon, and adding a new
withClear
class that sets a mirrored right padding ifonClear
is present and if a date has beenselected
):Checklist
QA testing note: You have to manipulate the current "Clearable" DatePicker example to test this. Inspect the input, go up to the parent
.euiFormControlLayout
, and set an inline style ofwidth: 200px;
.- [ ] Props have proper autodocs and playground toggles- [ ] Added documentation- [ ] Checked Code Sandbox works for the any docs examples- [ ] Added or updated jest tests- [ ] Checked for breaking changes and labeled appropriately- [ ] Checked for accessibility including keyboard-only and screenreader modes