Add time navigation buttons #9253

lukasolson · 2016-11-29T19:14:41Z

Replaces #8546.
Closes #1956.

This PR adds navigation buttons surrounding the time picker. The buttons are zoom in/out and step backward/forward.

Zooming in cuts the current interval in half (but keeping the same center point). Zooming out similarly doubles the current interval (keeping the same center point). Stepping backwards keeps the current interval but moves it backwards one interval, and stepping forward keeps the current interval but moves it forward one interval.

This PR replaces #8546 and merges with master, fixes styling issues, moves the calculations into a separate file and adds tests for it.

Update with new styles from navbar in ui framework

…that had side effects

… test

thomasneirynck

tested this with in Discover, Dashboard, Visualize. Works well.

FWIW, I kind of liked the small border that separated the time-controls from the new/save/open/share/refresh buttons.

thomasneirynck · 2016-11-29T21:09:55Z

jenkins, test this

lukasolson · 2016-11-29T21:15:09Z

FWIW, I kind of liked the small border that separated the time-controls from the new/save/open/share/refresh buttons.

@cjcenizal @alt74 Thoughts? I'm happy to re-add this if it's what we want :)

cjcenizal

Looks awesome man! I had one small request and a few suggestions.

cjcenizal · 2016-11-29T21:34:50Z

src/ui/public/styles/base.less

-    border-radius: 0;
-  }
-}
-


cjcenizal · 2016-11-29T21:43:10Z

src/ui/public/timepicker/__tests__/time_navigation.js

+    const {from, to} = timeNavigation.stepForward(bounds);
+    expect(from).to.be('2016-01-01T00:15:00.000Z');
+    expect(to).to.be('2016-01-01T00:30:00.000Z');
+  });


When I read this it took me a few seconds to figure out the meaning of this test, i.e. how much time is being stepped. Can we change the description to read: should step forward by the amount of the duration?

cjcenizal · 2016-11-29T21:43:45Z

src/ui/public/timepicker/__tests__/time_navigation.js

+    expect(to).to.be('2016-01-01T00:30:00.000Z');
+  });
+
+  it('should step backward', () => {


And here: should step backward by the amount of the duration

cjcenizal · 2016-11-29T21:45:31Z

src/ui/public/timepicker/__tests__/time_navigation.js

+    expect(to).to.be('2016-01-01T00:00:00.000Z');
+  });
+
+  it('should zoom out', () => {


should zoom out to be double the original duration

cjcenizal · 2016-11-29T21:45:54Z

src/ui/public/timepicker/__tests__/time_navigation.js

+    expect(to).to.be('2016-01-01T00:22:30.000Z');
+  });
+
+  it('should zoom in', () => {


should zoom in to be half the original duration

cjcenizal · 2016-11-29T21:47:04Z

src/ui/public/timepicker/kbn_global_timepicker.html

@@ -21,6 +21,18 @@
    </span>
  </div>

+  <div class="localMenuItem" ng-click="zoomIn()">
+    <i class="fa fa-search-plus ng-scope" tooltip="Zoom in"></i>


Instead of using the <i> element, can we use <span> everywhere here? It's more semantic markup.

cjcenizal · 2016-11-29T21:47:38Z

src/ui/public/timepicker/kbn_global_timepicker.js

+      // zoom in, halving the difference between start and end, keeping the same time range center
+      $scope.zoomIn = function () {
+        assign(timefilter.time, timeNavigation.zoomIn(timefilter.getBounds()));
+      };


❤️ these comments

But I think they might be better placed in time_navigation.js, because then they'll be near the logic, which is the part that really needs explaining.

👍 All good feedback. Thanks!

cjcenizal · 2016-11-29T21:48:46Z

src/ui/public/timepicker/kbn_global_timepicker.js

-import chromeNavControlsRegistry from 'ui/registry/chrome_nav_controls';
-import { once, clone } from 'lodash';
+import { once, clone, assign } from 'lodash';
+import dateMath from '@elastic/datemath';


Looks like this isn't being used here?

cjcenizal · 2016-11-29T23:53:38Z

Ping: @uboness for his thoughts on this

lukasolson · 2016-11-30T00:36:48Z

@cjcenizal I've made the suggested changes.

jgowdyelastic · 2016-11-30T11:35:41Z

LGTM
@lukasolson thanks for taking this over to get it finished.

One question,
The original getFromTo function which had the side effect of changing the timepicker mode to absolute if it wasn't already.
I found that without that mode change, when the timepicker was opened up after one of the new buttons had been used, it would be in a confused state.

To reproduce:
Starting in Quick or Relative mode.
Press one of the buttons (e.g. zoom in), note the timepicker label changes.
Open the timepicker menu.
It still thinks it is in Quick or Relative mode when it should be Absolute.

I've not tested this with these new changes and perhaps a recent change in timepicker has solved this.

thomasneirynck · 2016-11-30T17:48:49Z

this failure is due to failing tag cloud test. https://kibana-ci.elastic.co/job/elastic+kibana+pull-request+multijob-intake/2272/console

I'm looking into it. They fail intermittently, about 1/5 times.

uboness · 2016-12-02T13:40:16Z

Design wise... I'm not a big fan of the zoom in/out buttons... This functionality should be in the chart itself (we'll be educating the wrong ux). It is not an obvious feature. What are you zooming exactly? Think a dashboard with a map and a histogram.

I'd also like to change the surrounding arrows to something more subtle (< and >?)

sophiec20 · 2016-12-02T14:17:43Z

What are you zooming exactly? Think a dashboard with a map and a histogram.

The aim is purely to zoom in and out in time. As the time picker is global for the page, then it does not seem unintuitive that the functionality sits with the time picker control in the top nav, rather than in a chart panel. (double negative intended).

We have found the ability to navigate easily by time, to be hugely useful when exploring data around the occurrence of anomalies. For our use case with time series data, then navigating left, right, in and out were all equally used.

Did you mean that the choice of the magnifying glass as an icon is educating the wrong UX? Or is it the whole concept of zooming in and out by time?

uboness · 2016-12-02T14:25:09Z

Zoom is generally not associated with Zooming in time". The timepicker is a generic component that is used in many places. There can be a dashboard where the top most viz is a map. Then this will be misleading. It's not obvious at all that the zoom buttons only relate to the time picker. Definitely in the current design (it's not obvious to me at this point how to do it, if at all... Requires some thinking)

uboness · 2016-12-02T14:26:26Z

I have nothing against zooming in time... I just think it needs to be at the right place

cjcenizal · 2016-12-02T16:02:35Z

I think part of the problem may also be that "zooming" could be the wrong metaphor for what we're really doing (changing the time range to be large or small). I pored over FontAwesome and found some alternative icons to support different metaphors. What do you think of these, @uboness and @sophiec20 ?

Hourglass

This uses the idea of time, and having more of it or less of it.

or

Plus / minus

The problem with the current icons is that the magnifying glass conveys the idea of search. By removing that, we're left with a plus and minus. It's possible this could be confused with "adding" and "removing", though.

Expand / contract

This is the idea of expanding and contracting the range of time. We would need someone to build the contract icon to complete this option.

Expand / contract (abstract)

Same idea, but using a circle that will shrink or grow. We need someone to build both of these icons to complete this option. I think the contract icon can be a bold dot inside of a thin outline. For the expand icon, I'm imagining a lighter "thin" dot inside of a bolder outline.

Reference:

uboness · 2016-12-02T16:55:16Z

Sorry... None of these options tells me "zoom in/out time"... I don't think buttons/icons is the right ux here... an obvious UI for such a thing is a timeline slider where you select the window of the time to zoom to.

Are there any other similar UIs that anyone looked at with similar requirements?

Can we take this functionality out of this pr and let this one only focus on the time navigation (earlier/later buttons)? Can we put a proper design for these navigation buttons (the current one doesn't look good).

I don't want our discussion around the zoom functionality to put navigation on hold

lukasolson · 2016-12-02T20:15:31Z

@uboness Okay, I've removed that functionality.

ppisljar · 2016-12-03T05:34:37Z

I think that if you have zoomin/out icons IN the timepicker, it SHOULD be clear that you are zooming in/out on a time (not on a map).

The term "to zoom in" is often used with charts (point series and the likes) to indicate zooming on a time range. Some examples:
https://www.amcharts.com/demos/line-chart-with-scroll-and-zoom/
http://c3js.org/samples/interaction_zoom.html
http://demos.telerik.com/kendo-ui/bar-charts/pan-and-zoom

And the standard magnifying glass icon with +/- is used often for this use case:
Excel: http://i1-win.softpedia-static.com/screenshots/Excel-Chart_1.png
SAP: https://experience.sap.com/fiori-design-web/wp-content/uploads/sites/5/2014/10/Chart-Toolbar-Zoom-in-zoom-out5-1100x7222.png
matlab: http://i.stack.imgur.com/fd5LU.png
http://www.advsofteng.com/doc/cdcppdoc/images/realtimezoomscroll_qt.png
http://www.netdania.com/Downloads/guides/ChartStation/english/images/zoom.gif
http://help.infragistics.com/Help/Doc/WPF/2012.1/CLR4.0/html/images/Whats_New_xamDataChart_OPD.png

I would say that the most common way to show this kind of controls (in popular software) is by +/- magnifying glass icon, so this is something users would be used to (they did charts in excel before or matlab or ... ).

The other ways:

google for example doesnt use +/- icons they just allow you to select a preset range (day, week, month) ... we already offer this in time picker but i find zoomin/out very useful apart from that.
some charts would allow you to select on a range with a slider control. this only makes sense with point series charts but not with pie, map, ... and i think our zoom in/out should function globally

+1 on making it visually more obvious the controls are part of timepicker
-1 on using non standard icons
-1 on putting the functionallty in the chart itself. this is/should be part of timepicker control and affecting whole dashboard/page you are looking at instead of just specific chart.

uboness · 2016-12-03T15:10:02Z

@lukasolson thx! Next... Let's fix the arrows

@ppisljar sure... But it only works if you put the controls on the chart... Cause it automatically gives you the context for what you're zooming in/out. Putting these control on the dashboard level make little sense. Sure... We can think about other ways to put them in the timepicker such that it'll be obvious that they belong together... But it's not yet obvious to me how best to do that without making the timepicker even more complex control (we're in the process of cleaning it design-wise so we can consider this along the way). I still think the best way to zoom in/out is to have a timeline control on the dashboard.. But we're not there yet).

tbragin · 2016-12-07T07:10:07Z

Only now getting to this review, so I'll respond with my thoughts on the discussion above:

I agree that we should not put previous/next time-based navigation on hold to figure out the zoom in/out if there is no consensus on the UX. I'd be ok shipping just what Lukas has now, and figuring out zoom separately, if we don't feel we have the right UX with the current icons (I don't have a better suggestion).
Related to zoom, in my experience, chart brushing on a date-histogram backed chart to zoom in on a time interval is something most users figure out quickly. Zooming out is harder -- technically you can use the Back button if you realize it'll do what you want and you just zoomed in, but in most cases you may have done other things since zooming in, so Back button won't work. Interestingly, a couple of other vendors in this space have "zoom out" links in timepicker or embedded in some of their date histograms (e.g. Grafana, Splunk).
@jgowdyelastic asked earlier about switching timepicker to "absolute" mode if it's open when user starts navigating. My suggestion would actually be to just close the timepicker. I see little value in keeping it open once the arrow-based navigation begins.
The only weird thing I've experienced was that when you start with a absolute time frame in the past, you see it changed from something uber-precise "May 19th 2015, 16:09:01.858 to May 20th 2015, 16:09:01.858 " to something a lot less precise "~2 years ago to ~2 years ago", but the same thing happens on refresh. I get why we do that -- the long absolute time format is ugly, but in my case the easy-to-read format ended up being completely uninformative. I don't think we should try to fix that as part of this PR though.

So I guess, TLDR -- if we don't tackle zoom in/out as part of this, LGTM after we figure out point (3).

jgowdyelastic · 2016-12-07T10:09:39Z

@tbragin I don't think my original message was very clear, sorry.
When using the navigation buttons, the dates are set to absolute values, however the timepicker is not being set to absolute mode.
If it was in some other mode before the button was pressed (e.g. relative), when the timepicker menu is subsequently opened it will be in a confused state. The to/from values will be absolute but the mode will still be relative.
The original code set timefilter.time.mode = 'absolute' to correct this.

lukasolson · 2016-12-07T16:50:31Z

@uboness I've updated the icons:

@jgowdyelastic If I set the mode to absolute, it can be kind of jarring from the user experience because the tab automatically switches from whatever they were previously on (quick/relative). Also, even if I don't change the mode when they step forward/backward, even though the mode is still quick/relative and the to/from values are absolute, it all seems to function properly. (Did you notice a situation when it wasn't?)

jgowdyelastic · 2016-12-07T17:22:37Z

@lukasolson I've not had the chance to test this new implementation, so I'm going on how the original prelert version behaved.
if the mode was not changed to absolute, when the timepicker menu was reopened, none of the modes would be selected.
If you then clicked on the absolute button you'd see that there were in fact absolute dates selected.

epixa · 2016-12-07T18:14:05Z

I'm all for not blocking the other useful stuff in this PR on zoom in/out since that fell into contention, but not having zoom in/out is really disappointing. I hope there are plans in the discovery group to follow up on that feature once this PR goes in.

If Kibana doesn't have support for zoom in/out, then plugins like prelert are going to continue to have to hack that feature together on top of the existing timepicker. Any time-based log context feature would also most likely be blocked on this as well. Kibana really should have an ability to zoom in and out on the current global time interval.

@lukasolson Can you create a new issue for the time interval zoom behavior so we can stop littering this PR with that discussion.

* Add time navigation buttons * changing vars to const * Address some review points * fixing typo in function name * Remove unused variables * [timepicker] Simplify zoom in/out and forward/back and remove getter that had side effects * [timepicker] Move time navigation calculations to own class and write test * [timepicker] Remove unused styling and classes * [timepicker] Change from i to span, add more explanatory comments * [timepicker] Remove unused variable * Remove zoom in/out buttons from timepicker nav * Change step forward/back timepicker nav button icons

lukasolson · 2016-12-09T22:44:09Z

Backported to 5.x in 33d23d0.

jgowdyelastic and others added 9 commits October 24, 2016 11:11

Add time navigation buttons

7e1932f

changing vars to const

ad58aaa

Address some review points

4e976af

fixing typo in function name

efad64d

Merge branch 'master' into add-time-nav-buttons

412f7a9

Update with new styles from navbar in ui framework

Remove unused variables

2299d7b

[timepicker] Simplify zoom in/out and forward/back and remove getter …

aacde2b

…that had side effects

[timepicker] Move time navigation calculations to own class and write…

50b4f75

… test

[timepicker] Remove unused styling and classes

55d1ae9

lukasolson added review v5.2.0 v6.0.0 labels Nov 29, 2016

lukasolson assigned cjcenizal, thomasneirynck, ppisljar and jgowdyelastic Nov 29, 2016

lukasolson mentioned this pull request Nov 29, 2016

Add time navigation buttons #8546

Closed

thomasneirynck approved these changes Nov 29, 2016

View reviewed changes

cjcenizal suggested changes Nov 29, 2016

View reviewed changes

lukasolson added 2 commits November 29, 2016 17:34

[timepicker] Change from i to span, add more explanatory comments

3feda24

[timepicker] Remove unused variable

315739e

Merge branch 'master' into add-time-nav-buttons

89cb1b6

cjcenizal approved these changes Nov 30, 2016

View reviewed changes

ppisljar removed their assignment Dec 1, 2016

lukasolson added 2 commits December 2, 2016 13:04

Merge branch 'master' into add-time-nav-buttons

371dede

Remove zoom in/out buttons from timepicker nav

a5497d1

lukasolson added 2 commits December 7, 2016 09:23

Merge branch 'master' into add-time-nav-buttons

e5db5be

Change step forward/back timepicker nav button icons

91646df

lukasolson assigned lukasolson and unassigned cjcenizal, thomasneirynck and jgowdyelastic Dec 8, 2016

lukasolson mentioned this pull request Dec 9, 2016

Add zoom in/zoom out to time picker #9434

Closed

lukasolson merged commit b20f996 into elastic:master Dec 9, 2016

tbragin mentioned this pull request Dec 15, 2016

Automatically close timepicker upon selection #9218

Closed

epixa added v5.3.0 and removed v5.2.0 labels Mar 27, 2017

lukasolson deleted the add-time-nav-buttons branch March 27, 2018 21:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add time navigation buttons #9253

Add time navigation buttons #9253

lukasolson commented Nov 29, 2016

thomasneirynck left a comment

thomasneirynck commented Nov 29, 2016

lukasolson commented Nov 29, 2016

cjcenizal left a comment

cjcenizal Nov 29, 2016

cjcenizal Nov 29, 2016

cjcenizal Nov 29, 2016

cjcenizal Nov 29, 2016

cjcenizal Nov 29, 2016

cjcenizal Nov 29, 2016

cjcenizal Nov 29, 2016

cjcenizal Nov 29, 2016

lukasolson Nov 29, 2016

cjcenizal Nov 29, 2016

cjcenizal commented Nov 29, 2016 •

edited

Loading

lukasolson commented Nov 30, 2016

jgowdyelastic commented Nov 30, 2016

thomasneirynck commented Nov 30, 2016

uboness commented Dec 2, 2016

sophiec20 commented Dec 2, 2016

uboness commented Dec 2, 2016

uboness commented Dec 2, 2016

cjcenizal commented Dec 2, 2016 •

edited

Loading

uboness commented Dec 2, 2016

lukasolson commented Dec 2, 2016

ppisljar commented Dec 3, 2016 •

edited

Loading

uboness commented Dec 3, 2016

tbragin commented Dec 7, 2016 •

edited

Loading

jgowdyelastic commented Dec 7, 2016

lukasolson commented Dec 7, 2016

jgowdyelastic commented Dec 7, 2016

epixa commented Dec 7, 2016

lukasolson commented Dec 9, 2016 •

edited

Loading

Add time navigation buttons #9253

Add time navigation buttons #9253

Conversation

lukasolson commented Nov 29, 2016

thomasneirynck left a comment

Choose a reason for hiding this comment

thomasneirynck commented Nov 29, 2016

lukasolson commented Nov 29, 2016

cjcenizal left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cjcenizal commented Nov 29, 2016 • edited Loading

lukasolson commented Nov 30, 2016

jgowdyelastic commented Nov 30, 2016

thomasneirynck commented Nov 30, 2016

uboness commented Dec 2, 2016

sophiec20 commented Dec 2, 2016

uboness commented Dec 2, 2016

uboness commented Dec 2, 2016

cjcenizal commented Dec 2, 2016 • edited Loading

Hourglass

Plus / minus

Expand / contract

Expand / contract (abstract)

uboness commented Dec 2, 2016

lukasolson commented Dec 2, 2016

ppisljar commented Dec 3, 2016 • edited Loading

uboness commented Dec 3, 2016

tbragin commented Dec 7, 2016 • edited Loading

jgowdyelastic commented Dec 7, 2016

lukasolson commented Dec 7, 2016

jgowdyelastic commented Dec 7, 2016

epixa commented Dec 7, 2016

lukasolson commented Dec 9, 2016 • edited Loading

cjcenizal commented Nov 29, 2016 •

edited

Loading

cjcenizal commented Dec 2, 2016 •

edited

Loading

ppisljar commented Dec 3, 2016 •

edited

Loading

tbragin commented Dec 7, 2016 •

edited

Loading

lukasolson commented Dec 9, 2016 •

edited

Loading