Inherit selected value to quick select in EuiSuperDatePicker#3105
Inherit selected value to quick select in EuiSuperDatePicker#3105chandlerprall merged 21 commits intoelastic:masterfrom
Conversation
|
Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually? |
ashikmeerankutty
changed the title
ashikmeerankutty
changed the title
|
Jenkins, test this @ashikmeerankutty Can you also add a little more description like some before/after screenshots? |
src/components/date_picker/super_date_picker/quick_select_popover/quick_select_utlis.js
Outdated
Show resolved
Hide resolved
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_3105/ |
|
@cchaos Thanks for the review, updated the function description and added before and after screenshots. Can you please review it? |
|
Jenkins test this |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_3105/ |
cchaos
left a comment
cchaos
left a comment
There was a problem hiding this comment.
Thanks @ashikmeerankutty. The behavior seems to work well. I'd suggest adding some tests for the new parseTimeParts() method. And then I'd ask @chandlerprall to review since this is mostly JS.
|
+1 to some tests; I'll review once those have been added |
ashikmeerankutty
force-pushed
the
fix/super-date-picker
branch
from
0e22ab2 to
78072f6
Compare
|
@cchaos @chandlerprall Thanks for the review. Added tests 👍 |
RelativeThis appears to be correct when inheriting relative values Absoluteand correctly ignores absolute values Futurebut doesn't react as expected to |
|
@chandlerprall Fixed 👍 |
chandlerprall
left a comment
chandlerprall
left a comment
There was a problem hiding this comment.
This is very clean & straight forward! I have some inconsequential code changes, one question, and would like one more unit test.
src/components/date_picker/super_date_picker/quick_select_popover/quick_select_utils.js
Outdated
Show resolved
Hide resolved
src/components/date_picker/super_date_picker/quick_select_popover/quick_select_utils.js
Outdated
Show resolved
Hide resolved
src/components/date_picker/super_date_picker/quick_select_popover/quick_select_utils.js
Outdated
Show resolved
Hide resolved
src/components/date_picker/super_date_picker/quick_select_popover/quick_select_utils.js
Show resolved
Hide resolved
|
I resolved the changelog conflict as our releases tend to mess those up in PRs jenkins test this |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_3105/ |
chandlerprall
left a comment
chandlerprall
left a comment
There was a problem hiding this comment.
Changes LGTM; Pulled & tested locally.
Thanks again @ashikmeerankutty !
4 participants
Summary
Fixes #1597
Inherit selected value to quick select.
Before
After
Checklist
- [ ] Check against all themes for compatibility in both light and dark modes- [ ] Checked in mobile- [ ] Checked in IE11 and Firefox- [ ] Props have proper autodocs- [ ] Added documentation examples- [ ] Added or updated jest tests- [ ] Checked for breaking changes and labeled appropriately- [ ] Checked for accessibility including keyboard-only and screenreader modes