Skip to content
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

[EuiSuperDatePicker] Add onRefresh handler #1577

Merged
merged 11 commits into from
Feb 25, 2019

Conversation

sorenlouv
Copy link
Member

@sorenlouv sorenlouv commented Feb 20, 2019

When setting the refresh rate on EuiSuperDatePicker it should keep track of the interval, and call the onRefresh handler whenever the interval fires.
This will save consumers the hassle of implementing this logic themselves.

Checklist

  • Any props added have proper autodocs
  • Documentation examples were added
  • A changelog entry exists and is marked appropriately
  • This was checked for breaking changes and labeled appropriately
  • Jest tests were updated or added to match the most common scenarios

@sorenlouv sorenlouv requested a review from nreese February 20, 2019 07:19
@sorenlouv sorenlouv marked this pull request as ready for review February 20, 2019 22:20
@sorenlouv sorenlouv changed the title [EuiSuperDatePicker] Add onRefresh handler (WIP) [EuiSuperDatePicker] Add onRefresh handler Feb 20, 2019
@sorenlouv
Copy link
Member Author

@nreese How do you suggest we document this feature?

@nreese
Copy link
Contributor

nreese commented Feb 22, 2019

How do you suggest we document this feature

@cchaos and @chandlerprall any thoughts on how best to document?

@cchaos
Copy link
Contributor

cchaos commented Feb 22, 2019

Move the EuiSuperDatePicker to it's own page and split up the examples to address all these edge cases.

@sorenlouv
Copy link
Member Author

sorenlouv commented Feb 22, 2019

Move the EuiSuperDatePicker to it's own page and split up the examples to address all these edge cases.
@cchaos

Is this a refactor of the EuiSuperDatePicker docs, or just for the new onRefresh prop I'm adding? Not quite sure how much is expected of me to land this PR.

@sorenlouv
Copy link
Member Author

Feedback addressed.

Copy link
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.

Thanks! There's just a few outstanding items in the PR checklist to cover, like a changelog entry.

Copy link
Contributor

@nreese nreese left a comment

Choose a reason for hiding this comment

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

lgtm
code review

@sorenlouv sorenlouv merged commit 644fd63 into elastic:master Feb 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants