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 customQuickSelectPanels prop #1549

Merged
merged 3 commits into from
Feb 11, 2019

Conversation

nreese
Copy link
Contributor

@nreese nreese commented Feb 11, 2019

Add customQuickSelectPanels prop to EuiSuperDatePicker.

This prop will allow users to extend EuiSuperDatePicker quick select menu. One such use case is for selecting the entire data set range in Kibana

Copy link
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 and tested locally

@cchaos
Copy link
Contributor

cchaos commented Feb 11, 2019

So I'm really on the fence about the location of this custom items.

image

It seems quite overkill to have a completely separate section with a heading for one custom link (which could be the most common case). But at the same time it is the most versatile. And yet, on the other hand, if the content provided can only ever behave the same way as the quick select links, it makes the most sense to just append to that list instead of creating a whole new section.

A new section would be more appropriate if you could add any type of content (form elements or links or just text or whatever). So I'm leaning towards, either opening up the new section to be more generic, or just allowing the consumer to tap into the quick select list.

If I'm understanding what's going on correctly

@nreese
Copy link
Contributor Author

nreese commented Feb 11, 2019

It seems quite overkill to have a completely separate section with a heading for one custom link

In the use case I am envisioning, there will be multiple links, one per index pattern on a dashboard or map plus a "all data sets" link when there are multiple index patterns.

if the content provided can only ever behave the same way as the quick select links, it makes the most sense to just append to that list instead of creating a whole new section.

It is more complicated than just displaying a link. In the use case I am envisioning, the data set range is not know. When the user clicks "entire dataset timerange", the UI will have to display a loading indicator while a min and max aggregation are run against the time field. Once those return, then then applyTime will be called with the values.

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.

Ok, I'll trust you on this one. But before we add any more sections to this thing we need to consider how large it's getting. It almost doesn't fit on a mobile screen with all sections maxed out.

@nreese nreese merged commit ea94c0c into elastic:master Feb 11, 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.

3 participants