Skip to content

EuiSuperDatePicker should show tooltip for the popover close event#3127

Merged
cchaos merged 13 commits intoelastic:masterfrom
ashikmeerankutty:fix3/super-date-picker
Mar 23, 2020
Merged

EuiSuperDatePicker should show tooltip for the popover close event#3127
cchaos merged 13 commits intoelastic:masterfrom
ashikmeerankutty:fix3/super-date-picker

Conversation

@ashikmeerankutty
Copy link
Copy Markdown
Contributor

@ashikmeerankutty ashikmeerankutty commented Mar 19, 2020 •
edited by cchaos
Loading

Summary

Fixes #3124; Fixes #1708

Added isPopoverClosed prop to EuiSuperUpdateButton component that will return true if both the start and end popovers are closed.
If there is a change in the value and the popover is closed the tooltip is displayed.
The showToolTip function is not called for all update operations and is called only if isPopoverClosed

Before

before1

After

after1

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
  • A changelog entry exists and is marked appropriately

@kibanamachine
Copy link
Copy Markdown

kibanamachine commented Mar 19, 2020

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?

@cchaos
Copy link
Copy Markdown
Contributor

cchaos commented Mar 19, 2020

@ashikmeerankutty Please describe in the summary how you have fixed the issue.

@ashikmeerankutty
Copy link
Copy Markdown
Contributor Author

ashikmeerankutty commented Mar 19, 2020

@cchaos Thanks for the review, updated the description with changes. Can you please review it.

@chandlerprall chandlerprall self-requested a review March 19, 2020 21:16
@chandlerprall
Copy link
Copy Markdown
Contributor

chandlerprall commented Mar 19, 2020

This is definitely a lot better than being overwhelmed by the tooltip. Two things that could make this even better, if we can get the popover stuff to function this way:

update prevents interaction

  • the tooltip no longer shows up if hovering over the Update button while the selection panel is open - would be great to continue showing the tooltip whenever the cursor is over Refresh
  • have to click on the Update button twice (if the panel is open) to trigger an update - less than ideal, should only have to click once

@ashikmeerankutty
Copy link
Copy Markdown
Contributor Author

ashikmeerankutty commented Mar 20, 2020

@chandlerprall Fixed both issues. Can you please review it now?

@cchaos
Copy link
Copy Markdown
Contributor

cchaos commented Mar 20, 2020

Jenkins test this

@kibanamachine
Copy link
Copy Markdown

kibanamachine commented Mar 20, 2020

Preview documentation changes for this PR: https://eui.elastic.co/pr_3127/

@cchaos cchaos mentioned this pull request Mar 20, 2020
Closed
cchaos
cchaos suggested changes Mar 20, 2020
View reviewed changes
chandlerprall
chandlerprall approved these changes Mar 23, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@chandlerprall chandlerprall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes LGTM; pulled & tested in the docs

@chandlerprall
Copy link
Copy Markdown
Contributor

chandlerprall commented Mar 23, 2020

jenkins test this

@kibanamachine
Copy link
Copy Markdown

kibanamachine commented Mar 23, 2020

Preview documentation changes for this PR: https://eui.elastic.co/pr_3127/

@chandlerprall chandlerprall requested a review from cchaos March 23, 2020 19:49
cchaos
cchaos approved these changes Mar 23, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@cchaos cchaos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM functionally. Just have one change request for the prop description.

@cchaos cchaos merged commit 0a201b2 into elastic:master Mar 23, 2020
@chandlerprall chandlerprall mentioned this pull request Mar 31, 2020
Closed
@chandlerprall chandlerprall mentioned this pull request Apr 8, 2020
Merged
@ashikmeerankutty ashikmeerankutty deleted the fix3/super-date-picker branch April 15, 2020 09:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@cchaos cchaos cchaos approved these changes

@chandlerprall chandlerprall chandlerprall approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[EuiSuperDatePicker] Update button tooltip occurs too often [EuiSuperDatePicker] closes the Minutes ago select box before I can select anything

4 participants

@ashikmeerankutty @kibanamachine @cchaos @chandlerprall