Add time navigation buttons #8546

jgowdyelastic · 2016-10-05T10:57:17Z

Adds zoom in/out and move left/right buttons to kibana's time picker.

Closes #1956

epixa · 2016-10-05T12:41:44Z

jenkins, test this

epixa · 2016-10-05T12:52:44Z

@jgowdyelastic Can you reissue this PR against master? We'll then backport it to 5.x.

jgowdyelastic · 2016-10-05T13:10:40Z

Done, rebased on master and PR is now against master.

epixa · 2016-10-05T13:12:02Z

@jgowdyelastic Thanks!

epixa · 2016-10-05T13:37:57Z

src/ui/public/timepicker/kbn_global_timepicker.js

+
+      // travel forward in time based on the interval between from and to
+      $scope.foward = function () {
+        var time = getFromTo();


These should all be const unless you will be overriding the reference, in which case they would be let.

ok, good point. i've pushed a change for this.
This was just copied from the kibana 4.x prelert app, which doesn't have much ES6 syntax.

LeeDr · 2016-10-06T11:27:13Z

Please add automated and/or manual tests for this new feature.

dimitris-athanasiou · 2016-10-06T14:14:50Z

Just letting you folks @jgowdyelastic is on vacation, back on Tuesday 11th. He is not ignoring you all.

thomasneirynck

Looks great! I like the addition. It is very useful addition I think.

I only have some real minor cosmetic things.

thomasneirynck · 2016-10-06T20:06:29Z

src/ui/public/timepicker/kbn_global_timepicker.js

@@ -2,6 +2,7 @@ import moment from 'moment';
 import UiModules from 'ui/modules';
 import chromeNavControlsRegistry from 'ui/registry/chrome_nav_controls';


this import is obsolete. linter was also complaining about unused function arguments in this file as well: config, newval, oldVal, $el, attrs

While we're at it, we may as well remove them ;)

thomasneirynck · 2016-10-06T20:09:36Z

src/ui/public/timepicker/kbn_global_timepicker.js

+
+        time.from.subtract(diff, 'milliseconds');
+        timefilter.time.to = origFrom;
+        timefilter.time.from = time.from.toISOString();


minor: consider reordering the to/from assignments for symmetry with all the others.

thomasneirynck · 2016-10-06T20:11:15Z

src/ui/public/timepicker/kbn_global_timepicker.js

@@ -25,6 +26,73 @@ UiModules
      $rootScope.toggleRefresh = () => {
        timefilter.refreshInterval.pause = !timefilter.refreshInterval.pause;
      };
+
+      // functions for the time navigation buttons


minor: consider removing this comment, I think the code is nice & dense enough to be self explanatory.

thomasneirynck · 2016-10-06T20:16:35Z

src/ui/public/timepicker/kbn_global_timepicker.js

+            from: moment(timefilter.time.from)
+          };
+        } else {
+          timefilter.time.mode = 'absolute';


minor: consider not doing the side-effect in this getter/setter. If you prefer to avoid two function-calls, perhaps rename this function to something like setToTimeToAbsoluteAndGetToFrom (yeah, i know :$). But now, this state change is a little buried.

thomasneirynck · 2016-10-06T20:18:32Z

src/ui/public/timepicker/kbn_global_timepicker.html

@@ -20,6 +20,9 @@
  </li>

  <li>
+    <i ng-click="zoomIn()" class="time-nav-btn fa fa-search-plus ng-scope"></i>


minor: I wouldn't mind tooltips on these buttons actually. 'move [forward|backward] in time', 'zoom out timeline', 'zoom in timeline'

tooltips added

thomasneirynck · 2016-10-06T20:25:59Z

src/ui/public/timepicker/kbn_global_timepicker.html

@@ -28,5 +31,6 @@
      <i aria-hidden="true" class="fa fa-clock-o"></i>
      <pretty-duration from="timefilter.time.from" to="timefilter.time.to"></pretty-duration>
    </button>
+    <i ng-click="foward()" class="time-nav-btn fa fa-arrow-right ng-scope"></i>
  </li>
 </ul>


sorry to hijack this PR for something a little broader ;)

With the 4 new additions, the top toolbar now looks like the above. From a UI perspective, would it make sense to better isolate the timepicker tools from the "new/save/open/share" buttons. Both are unrelated thematically. Putting a divisor in between/shading background differently/ .... might make the grouping a little clearer. @alt74, @cjcenizal, thoughts (?)

I think that, ultimately, we should rebuild the timepicker component so that these arrow buttons are part of the dropdown that appears when you click the time range. I don't think the user needs to have them visible at all times. However, this will take some time to implement, and I don't think we should hold up this PR for that.

I think the immediate problem is that the "back" arrow blends in with the + and - icons. We can create the separation you mention by moving them both to the right side, and letting the time range itself create visual separation:

As an additional benefit, the user doesn't need to move the mouse as far when navigating back and forward in succession.

What do you think?

I agree that the timepicker controls should be obviously separate from the rest of the menu.
I've added a separator as I think it's the simplest and less intrusive solution.

I prefer the arrow buttons being either side of the date range as it's obvious that you'll be moving either side of the displayed range.
But it doesn't make too much difference either way. So I'm happy to be overridden.

Ultimately I think these controls should remain in the navigation bar and not inside the dropdown. They provide a fast way of navigating through time and made a noticeable difference when we originally added them to the Prelert app.

I think this looks good. I'm not 100% confident about it but that's because I haven't used it as much as you have. Once I've had the chance to use it for awhile I'm sure I'll either be on board or be able to provide some better suggestions. :)

IMHO, the arrows work better on either side of the range, for the reasons mentioned by @jgowdyelastic

cjcenizal · 2016-10-10T19:18:30Z

src/ui/public/styles/base.less

@@ -232,6 +232,11 @@ a {
    background-color: transparent;
    border-radius: 0;
  }
+
+  i.time-nav-btn {


This can just be .time-nav-btn because using the element selector increases the selector's specificity unnecessarily.

agreed. element selector removed

cjcenizal · 2016-10-10T19:50:58Z

src/ui/public/timepicker/kbn_global_timepicker.html

@@ -28,5 +31,6 @@
      <i aria-hidden="true" class="fa fa-clock-o"></i>
      <pretty-duration from="timefilter.time.from" to="timefilter.time.to"></pretty-duration>
    </button>
+    <i ng-click="foward()" class="time-nav-btn fa fa-arrow-right ng-scope"></i>
  </li>
 </ul>


I think that, ultimately, we should rebuild the timepicker component so that these arrow buttons are part of the dropdown that appears when you click the time range. I don't think the user needs to have them visible at all times. However, this will take some time to implement, and I don't think we should hold up this PR for that.

I think the immediate problem is that the "back" arrow blends in with the + and - icons. We can create the separation you mention by moving them both to the right side, and letting the time range itself create visual separation:

As an additional benefit, the user doesn't need to move the mouse as far when navigating back and forward in succession.

What do you think?

tbragin · 2016-10-23T19:29:24Z

Could I ask for some screenshots or a quick animated gif in this PR? Will help reviewers that don't have time to check it out to at least have an idea where in the interface these new buttons are.

thomasneirynck · 2016-10-24T05:12:32Z

src/ui/public/styles/base.less

@@ -218,6 +218,8 @@ a {
 }

 .navbar-timepicker {
+  border-left: 1px solid @kibanaGray4;
+


+1 for me, I think it looks good.

thomasneirynck · 2016-10-24T05:16:13Z

@tanya this was buried in one of those "outdated" collapsing sections, but here is a screenshot to what it looks like: #8546 (comment). The arrows move the time forward and backward with the size of the current interval, and the magnifiers zoom out and or zoom in the time with a factor of two

kimjoar · 2016-10-24T05:33:15Z

src/ui/public/timepicker/kbn_global_timepicker.js

@@ -25,6 +26,71 @@ UiModules
      $rootScope.toggleRefresh = () => {
        timefilter.refreshInterval.pause = !timefilter.refreshInterval.pause;
      };
+
+      // travel forward in time based on the interval between from and to
+      $scope.foward = function () {


typo, foward -> forward

ha, embarrassing....
i've corrected this, thanks

kimjoar · 2016-10-24T05:52:15Z

I find the icons confusing, but there are tooltips so it's probably good enough. However, I think we should add better descriptions to the tooltips. For example, I think Zoom in and Zoom out are unclear about what you're actually doing (Maybe something along the lines of Double the timespan — or maybe we already have some other word we use for the timepicker range(?)).

Also, to me something looks off about the spacing between all the timepicker controls. I think it needs tighter spacing between the controls, plus I think it should have the same amount of padding on each side of the new border.

ppisljar · 2016-10-24T08:42:24Z

this looks really great!

icons are quite clear to me, also positioning makes sense. Zoom in makes more sense to be than Double the timespan ... but i agree we could come up with a better tooltip.

alignment and spacing between elements don't look right to me:

padding on icons (10px) is different that on timepicker button (15px)
timepicker button has line-height defined as 20px (should be 16 as everything else in this toolbar)
icon inside timepicker button sets vertical-align: middle (button does the same which misaligns things)
button has 1px trans border, which misaligns things
there is some additional spacing on the left and right (3px) and i have no idea where it comes from.

Bargs · 2016-11-04T21:07:56Z

I'll echo what @tbragin said, some screenshots and a gif in the description, along with some extra explanation of the details of how this is supposed to behave would be very helpful.

lukasolson · 2016-11-29T19:16:30Z

Replaced by #9253.

### Closes elastic#8546

jgowdyelastic added release_note:enhancement review v5.1.0 v6.0.0 labels Oct 5, 2016

jgowdyelastic force-pushed the add-time-nav-buttons branch from b01b1aa to c591bd7 Compare October 5, 2016 13:07

jgowdyelastic changed the base branch from 5.x to master October 5, 2016 13:09

epixa reviewed Oct 5, 2016

View reviewed changes

thomasneirynck self-assigned this Oct 5, 2016

LeeDr added the tests_needed label Oct 6, 2016

thomasneirynck reviewed Oct 6, 2016

View reviewed changes

epixa removed the review label Oct 8, 2016

cjcenizal reviewed Oct 10, 2016

View reviewed changes

jgowdyelastic force-pushed the add-time-nav-buttons branch from 254c731 to 2f7f251 Compare October 11, 2016 12:28

alt74 added the Team:Platform-Design Team Label for Kibana Design Team. Support the Analyze group of plugins. label Oct 11, 2016

thomasneirynck reviewed Oct 24, 2016

View reviewed changes

kimjoar reviewed Oct 24, 2016

View reviewed changes

jgowdyelastic added 4 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

jgowdyelastic force-pushed the add-time-nav-buttons branch from 2f7f251 to efad64d Compare October 24, 2016 10:23

tbragin added Feature:Timepicker Timepicker :Discovery labels Nov 4, 2016

Bargs self-assigned this Nov 4, 2016

ppisljar removed the v5.1.0 label Nov 21, 2016

Bargs assigned lukasolson and unassigned Bargs Nov 22, 2016

lukasolson mentioned this pull request Nov 29, 2016

Add time navigation buttons #9253

Merged

lukasolson closed this Nov 29, 2016

jimgoodwin added the v6.0.0-rc1 label Sep 28, 2017

daveyholler added a commit to daveyholler/kibana that referenced this pull request Nov 7, 2024

Removes the extra "connector" from the API key name

f9a5e75

### Closes elastic#8546

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add time navigation buttons #8546

Add time navigation buttons #8546

jgowdyelastic commented Oct 5, 2016 •

edited by epixa

Loading

epixa commented Oct 5, 2016

epixa commented Oct 5, 2016

jgowdyelastic commented Oct 5, 2016

epixa commented Oct 5, 2016

epixa Oct 5, 2016

jgowdyelastic Oct 5, 2016

LeeDr commented Oct 6, 2016

dimitris-athanasiou commented Oct 6, 2016

thomasneirynck left a comment

thomasneirynck Oct 6, 2016 •

edited

Loading

thomasneirynck Oct 6, 2016

thomasneirynck Oct 6, 2016

thomasneirynck Oct 6, 2016

thomasneirynck Oct 6, 2016

jgowdyelastic Oct 11, 2016

thomasneirynck Oct 6, 2016

cjcenizal Oct 10, 2016 •

edited

Loading

jgowdyelastic Oct 11, 2016

cjcenizal Oct 11, 2016

thomasneirynck Oct 11, 2016

cjcenizal Oct 10, 2016

jgowdyelastic Oct 11, 2016

cjcenizal Oct 10, 2016 •

edited

Loading

tbragin commented Oct 23, 2016

thomasneirynck Oct 24, 2016

thomasneirynck commented Oct 24, 2016

kimjoar Oct 24, 2016

jgowdyelastic Oct 24, 2016

kimjoar commented Oct 24, 2016

ppisljar commented Oct 24, 2016 •

edited

Loading

Bargs commented Nov 4, 2016

lukasolson commented Nov 29, 2016

		@@ -2,6 +2,7 @@ import moment from 'moment';
		import UiModules from 'ui/modules';
		import chromeNavControlsRegistry from 'ui/registry/chrome_nav_controls';

Add time navigation buttons #8546

Add time navigation buttons #8546

Conversation

jgowdyelastic commented Oct 5, 2016 • edited by epixa Loading

epixa commented Oct 5, 2016

epixa commented Oct 5, 2016

jgowdyelastic commented Oct 5, 2016

epixa commented Oct 5, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LeeDr commented Oct 6, 2016

dimitris-athanasiou commented Oct 6, 2016

thomasneirynck left a comment

Choose a reason for hiding this comment

thomasneirynck Oct 6, 2016 • edited Loading

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 Oct 10, 2016 • edited Loading

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 Oct 10, 2016 • edited Loading

Choose a reason for hiding this comment

tbragin commented Oct 23, 2016

Choose a reason for hiding this comment

thomasneirynck commented Oct 24, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kimjoar commented Oct 24, 2016

ppisljar commented Oct 24, 2016 • edited Loading

Bargs commented Nov 4, 2016

lukasolson commented Nov 29, 2016

jgowdyelastic commented Oct 5, 2016 •

edited by epixa

Loading

thomasneirynck Oct 6, 2016 •

edited

Loading

cjcenizal Oct 10, 2016 •

edited

Loading

cjcenizal Oct 10, 2016 •

edited

Loading

ppisljar commented Oct 24, 2016 •

edited

Loading