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.

Sign up for GitHub

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

Jump to bottom

[Infra UI] Add timepicker and chart dragging to Node detail page #23830

Merged
merged 14 commits into from
Oct 15, 2018
Merged

[Infra UI] Add timepicker and chart dragging to Node detail page #23830

merged 14 commits into from
Oct 15, 2018

Conversation

XavierM
Copy link
Contributor

@XavierM XavierM commented Oct 4, 2018

#23394
and fixes #23828

@XavierM XavierM added WIP Work in progress :Ingest UI Feature:Metrics UI Metrics UI feature loe:medium Medium Level of Effort labels Oct 4, 2018
@XavierM XavierM self-assigned this Oct 4, 2018
@elasticmachine
Copy link
Contributor

💔 Build Failed

@weltenwort
Copy link
Member

weltenwort commented Oct 4, 2018 •
edited
Loading

looks like the CI failures are caused by genuine type errors

@XavierM XavierM added review and removed WIP Work in progress labels Oct 5, 2018
@XavierM XavierM requested a review from skh October 5, 2018 15:12
@XavierM
Copy link
Contributor Author

XavierM commented Oct 5, 2018

it looks like that
image

@XavierM XavierM force-pushed the metric-detail-global-date-picker branch from 22c3383 to 3cd42d6 Compare October 5, 2018 15:22
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@XavierM
Copy link
Contributor Author

XavierM commented Oct 5, 2018 •
edited
Loading

FYI @weltenwort, I talked to @simianhacker

  • Bug on Recently used date ranges when you pick a date range
  • Add a cancel button close to the go button
  • Add CrosshairLocation on all the graph
  • Add SectionBrush/Zoom on graph who will also set the date range
  • Add a reset button to go back to default value
  • on the popover on the date range, need to set the default value to Last 1 hour

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@XavierM XavierM requested a review from tbragin October 10, 2018 08:48
@XavierM
Copy link
Contributor Author

XavierM commented Oct 10, 2018

@simianhacker what the default value for the crosshair? for now, I put it in the middle of the chart

@tbragin
Copy link
Contributor

tbragin commented Oct 14, 2018 •
edited
Loading

@simianhacker @XavierM I checked out the PR and used it a bit.

I think it's functional, but I'm confused why we can't just use the same logic as Kibana's timepicker, where in most cases we see "Last 30 minutes" type of display?

screen shot 2018-10-14 at 2 05 45 pm

If we can't fix it for Beta, I'm okay with it as is, but we should get together and discuss how working with time ranges really should work.

@XavierM XavierM force-pushed the metric-detail-global-date-picker branch from 8df08d0 to ae7bf92 Compare October 15, 2018 13:55
@XavierM
Copy link
Contributor Author

XavierM commented Oct 15, 2018

@tbragin
Right now, we have the last hour in the time range, we can change it really easily to a 30 minutes time range.
We can maybe add a small the last recently used date ranges outside of the date picker so it is easier for the user to see what's going on. What do you think?
I do agree that we should change/figure out how to make the experience better.

@elasticmachine
Copy link
Contributor

💔 Build Failed

simianhacker
simianhacker approved these changes Oct 15, 2018
View reviewed changes
Copy link
Member

@simianhacker simianhacker left a comment

Choose a reason for hiding this comment

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

LGTM

I think for this version we should go with what we have "as is" but in the next release I would like to have @elastic/eui-design team look at the time picker and come up with a better UX. I agree with Tanya I like the old Kibana time picker but unfortunately that's only available in Angular (as far as I know).

@XavierM
Copy link
Contributor Author

XavierM commented Oct 15, 2018

jenkins, test this

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@snide
Copy link
Contributor

snide commented Oct 15, 2018

@tbragin @simianhacker A eui/kibana datepicker will be available early in 6.6. You can mess with the UI side of it in elastic/eui#1219

Since most of it is fairly Kibana specific it'll have a Kibana component to handle all the logic and be a pure replacement of the angular UI available currently.

It's likely in the couple weeks, not couple months timeframe.

@XavierM XavierM merged commit 2b689ef into elastic:feature-infra-ui Oct 15, 2018
@simianhacker
Copy link
Member

Thanks for the update @snide! Now I'm even more convinced that this PR will get us to 6.5, then we can switch over to the new one in 6.6

@tbragin
Copy link
Contributor

tbragin commented Oct 15, 2018

Nice job @XavierM :)

@XavierM XavierM deleted the metric-detail-global-date-picker branch March 20, 2019 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@simianhacker simianhacker simianhacker approved these changes

@weltenwort weltenwort Awaiting requested review from weltenwort

@skh skh Awaiting requested review from skh

@tbragin tbragin Awaiting requested review from tbragin

Assignees

@XavierM XavierM

Labels
Feature:Metrics UI Metrics UI feature loe:medium Medium Level of Effort review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@XavierM @elasticmachine @weltenwort @tbragin @snide @simianhacker