EUIfy Console - partially de-angularize and move custom views out of top_nav #39341

lizozom · 2019-06-20T13:20:18Z

Release note

Console's welcome panel, help panel, and settings form UIs have all been updated with a new look that is more consistent with the rest of Kibana's UI. Changes to the layout of these elements also improves the user experience of using Console on smaller screens.

Summary

This PR is part of the task of moving kbn_top_nav to new platform.
To do so, I wanted to narrow the functionality of kbn_top_nav, and specifically, stop using it for rendering inline custom views (examples below).

This PR focuses on the Sense (DevTools) app, which was the heaviest user of this functionality. I will create additional PRs for graph and timelion, that make smaller use of this ability.

I was also able to delete SenseTopNavController which was the only remaining custom TopNavController implementation

These are the UI changes in this PR

Welcome notification

Welcome notification used to be shown automatically (i.e. without the user clicking on any navigation button) as part of the navigation menu.
It now uses a flyout.
If it's closed by clicking on the X or anywhere on the screen, it will come back next time the user refreshes the window. However, if the user clicks on the 'Dismiss' button, it won't be shown again.
The only changes here are the display location and the text on the button, which I felt was more meaningful.

Help

Help used to be displayed inside a flyout.
There were no other changes to the UI.

Settings

Previously, DevTools settings were displayed inline, inside the navigation menu.
I moved them to a modal.

Important

Coincidentally, @cjcenizal merged two PRs yesterday, and I had to re-implement them partially. They are tested and seem to work correctly.

History

History console still opens inline, but not as part of the navigation menu, as I felt like moving it to a modal \ flyout worsens the UX.
I was able to achieve a small usability improvement, where if history is open, running queries are added immediately to the list (before you had to re-open the history to see changes).

Checklist

Use ~~strikethroughs~~ to remove checklist items you don't feel are applicable to this PR.

This was checked for cross-browser compatibility, including a check against IE11
Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
This was checked for keyboard-only and screenreader accessibility

Improve it's behavior - now it will update when open, when a query is run.

…op-nav/devtools

lizozom · 2019-06-20T13:22:03Z

@AlonaNadler @elastic/eui-design I would appreciate a review of the UI changes made here.

Please note that my goal was not to create the final perfect UI (as this app still needs to be Reactified and EUIficated), but to clear out top nav quickly, while getting some quick EUI wins.

…op-nav/devtools

elasticmachine · 2019-06-20T13:54:46Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-06-20T13:56:33Z

Pinging @elastic/es-ui

cchaos

Thanks @lizozom

I'm good with the settings moving to a modal because of the extra options that have been added. However, can you explain why you think moving the history to a flyout is worse UX than keeping it inline?

It currently shrinks the entire Console to less than half the window (though I guess that depends on your window size), and makes the user have to scroll a lot within the actual editor.

That is on a 17" macbook full height. The console pretty much becomes unusable if you have long searches.

src/legacy/core_plugins/console/public/src/components/help_panel.tsx

src/legacy/core_plugins/console/public/src/components/welcome_panel.tsx

src/legacy/core_plugins/console/public/src/directives/_history.scss

elasticmachine · 2019-06-20T16:19:01Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

cjcenizal

👏 👏 👏 I am so happy to see this change @lizozom! Thank you for doing this. This is a huge help for the ES UI New Platform migration effort.

I did a light code review and left a few comments. I will do a more in-depth review in a later round, during which I'll look more closely at the welcome screen code and the code that touches changes from my recent PRs, and also do a more thorough general QA pass.

In terms of Caroline's comments, I'm OK with not spending too many iterations on improving the UI and UX in this PR. The purpose of this PR is just to convert parts of the UI from Angular to React/EUI, and as long as we don't introduce any UI/UX regressions then we're good to go. In that sense, any UI/UX improvements we introduce are a nice bonus, but nothing we should block merging on. Once this PR is merged we will spend subsequent PRs on optimizing the UI and UX.

That said, I do think this PR introduces a small UI regression because there's no visual indication separating the history panel from the main content area. @cchaos Could you suggest a minimally invasive solution here? E.g. a CSS class with a bottom-border and a box-shadow? EDIT: I see that this same problem exists in master, except there's just more space between the menu and the editor. Can we add a <EuiSpacer /> at the bottom of the menu below the apply and cancel buttons, Liza?

Can we also remove the overlay from the welcome flyout? The welcome flyout makes many references to various parts of the UI, but the overlay obscures the UI which takes away necessary context.

Similarly for the Help menu, it would be nice to be able to make use of the menu while it's open, but the overlay makes that impossible. Can we remove the overlay here, too?

src/legacy/core_plugins/console/public/src/components/settings_modal.tsx

src/legacy/core_plugins/console/public/src/input.js

src/legacy/core_plugins/console/public/src/components/help_show_panel.tsx

cjcenizal · 2019-06-20T23:12:09Z

src/legacy/core_plugins/console/public/src/controllers/sense_controller.js

@@ -94,7 +114,9 @@ module.controller('SenseController', function SenseController(Private, $scope, $

  $scope.sendSelected = () => {
    input.focus();
-    input.sendCurrentRequestToES();
+    input.sendCurrentRequestToES(() => {
+      $scope.historyDirty = new Date().getTime();


Could you walk me through the role $scope.historyDirty plays in the UX?

It's used to force the sense-history to regenerate in case a user runs a query, while the history panel is open.

Ah, I see, thanks! Could we please rename it to lastRequestTimestamp, and add a comment on this line:

// History watches this value and will re-render itself when it changes, so that // the list of requests stays up-to-date as new requests are sent.

cjcenizal · 2019-06-22T00:00:22Z

I just did a QA pass and verified:

Keyboard shortcuts work (except for executing the selected request)
Gear menu options work
Executing multiple requests in sequence works
History panel works
Settings panel works including:
- Polling settings
- Manual refresh button
- Disabling triple quotes

…op-nav/devtools

lizozom · 2019-06-23T11:50:58Z

@cjcenizal I removed the overlay from the flyout.

However, I actually don't think that adding a spacer there will help anything, just take even more space off the screen. I loved the new design @cchaos suggested for the history panel, but I don't have enough time on this task to implement it. Could we just leave it as is?

Fixed all other comments!

…/kibana into newplatform/kbn-top-nav/devtools

lizozom · 2019-06-25T13:37:53Z

@cjcenizal done

elasticmachine · 2019-06-25T14:16:13Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-06-25T16:23:23Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

cjcenizal · 2019-06-26T01:52:27Z

Retest

cjcenizal

Woohoo! This looks great! I'm going to approve this PR but I did find a couple minor errors that I'd prefer we fix before merging.

Closing the Welcome panel with the X doesn't "dismiss" it

If you click this X in the top right corner the the local storage flag isn't set and the panel shows up again if you leave and come back to Console.

Font settings input gets `NaN`

If you clear this input then you get this warning in the console:

Font settings input and unmounted component

I'm not sure if the input is what's causing this warning, but if you change the font settings to another value and hit Save, you get this warning in the console:

src/legacy/core_plugins/console/public/src/helpers/get_top_nav.ts

src/legacy/core_plugins/console/public/src/controllers/sense_controller.js

src/legacy/core_plugins/console/public/src/helpers/welcome_show_panel.tsx

cuff-links · 2019-06-26T02:05:10Z

@lizozom I went ahead and ran through these changes and did not notice any regressions. I tested this on Firefox and Chrome and also tested the changes made due to feedback from others. Nice work! Getting a VM up and running to run through it on IE11.

elasticmachine · 2019-06-26T03:04:02Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

…op-nav/devtools

elasticmachine · 2019-06-26T09:44:10Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

lizozom · 2019-06-26T10:09:56Z

@cchaos could you review the final design?

lizozom · 2019-06-26T12:13:57Z

@lizozom I went ahead and ran through these changes and did not notice any regressions. I tested this on Firefox and Chrome and also tested the changes made due to feedback from others. Nice work! Getting a VM up and running to run through it on IE11.

@silne30 Waiting for your feedback!

cuff-links

No regressions on IE11. Thanks for your patience. :)

@silne30

PR reviewed for regressions by @silne30 while @cchaos is out on vacation.

…av (elastic#39341) * Sense settings modal * Deleted comments * DevTools help flyout * Settings UI improvements * Improve file naming * Rename helpShowPanel * Added TS to help * React welcome flyout + removed it from top nav * Move history tool outside of top nav Improve it's behavior - now it will update when open, when a query is run. * ts fix * Code review fixes * Code review fixes * Fixed translations * Code review changes - autoclose panels * deleted unused useEffect * fix z-order of components * type check fixes * Final code review fixes

elasticmachine · 2019-06-26T19:22:36Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

…av (#39341) (#39705) * Sense settings modal * Deleted comments * DevTools help flyout * Settings UI improvements * Improve file naming * Rename helpShowPanel * Added TS to help * React welcome flyout + removed it from top nav * Move history tool outside of top nav Improve it's behavior - now it will update when open, when a query is run. * ts fix * Code review fixes * Code review fixes * Fixed translations * Code review changes - autoclose panels * deleted unused useEffect * fix z-order of components * type check fixes * Final code review fixes

Liza Katz added 10 commits June 19, 2019 18:50

Sense settings modal

b22e36b

Deleted comments

ec5d87a

DevTools help flyout

d7a153b

Settings UI improvements

8c51d50

Improve file naming

a2501b3

Rename helpShowPanel

e75dd04

Added TS to help

462aa35

React welcome flyout + removed it from top nav

2948e47

Move history tool outside of top nav

2e57a6f

Improve it's behavior - now it will update when open, when a query is run.

Merge remote-tracking branch 'upstream/master' into newplatform/kbn-t…

34879a7

…op-nav/devtools

lizozom self-assigned this Jun 20, 2019

Merge remote-tracking branch 'upstream/master' into newplatform/kbn-t…

b78d8d9

…op-nav/devtools

cjcenizal added Feature:Console Dev Tools Console Feature Feature:Dev Tools Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more labels Jun 20, 2019

cchaos previously requested changes Jun 20, 2019

View reviewed changes

lizozom added the Feature:New Platform label Jun 20, 2019

ts fix

ad40321

cjcenizal suggested changes Jun 20, 2019

View reviewed changes

lizozom marked this pull request as ready for review June 21, 2019 09:32

Liza Katz added 3 commits June 23, 2019 14:44

Code review fixes

f15acdb

Code review fixes

dca7b78

Merge remote-tracking branch 'upstream/master' into newplatform/kbn-t…

2026b92

…op-nav/devtools

Merge branch 'newplatform/kbn-top-nav/devtools' of github.com:lizozom…

3ea04af

…/kibana into newplatform/kbn-top-nav/devtools

fix z-order of components

979cee4

type check fixes

787f9d1

cjcenizal requested a review from cuff-links June 25, 2019 16:52

cjcenizal approved these changes Jun 26, 2019

View reviewed changes

Liza Katz added 3 commits June 26, 2019 11:17

Final code review fixes

02d8bad

Merge remote-tracking branch 'upstream/master' into newplatform/kbn-t…

5fd54dc

…op-nav/devtools

Merge remote-tracking branch 'upstream/master' into newplatform/kbn-t…

3f90b57

…op-nav/devtools

lukeelmers mentioned this pull request Jun 26, 2019

App Architecture roadmap for New Platform migration #31968

Closed

cuff-links approved these changes Jun 26, 2019

View reviewed changes

lizozom merged commit 2ac2058 into elastic:master Jun 26, 2019

lizozom mentioned this pull request Jun 26, 2019

[7.x] DevTools - partially de-angularize and move custom views out of top_nav (#39341) #39705

Merged

lizozom mentioned this pull request Jun 30, 2019

Replace kbn_top_nav with a new React component #39981

Closed

11 tasks

cjcenizal changed the title ~~DevTools - partially de-angularize and move custom views out of top_nav~~ EUIfy Console - partially de-angularize and move custom views out of top_nav Jul 14, 2019

cjcenizal added v7.3.0 v8.0.0 labels Jul 14, 2019

cjcenizal mentioned this pull request Jul 14, 2019

Fix minor Console regressions introduced during EUIfication. #41089

Merged

cjcenizal mentioned this pull request Jul 23, 2019

[DOCS] Updates Console doc #41371

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

EUIfy Console - partially de-angularize and move custom views out of top_nav #39341

EUIfy Console - partially de-angularize and move custom views out of top_nav #39341

lizozom commented Jun 20, 2019 •

edited by cjcenizal

Loading

lizozom commented Jun 20, 2019

elasticmachine commented Jun 20, 2019

elasticmachine commented Jun 20, 2019

cchaos left a comment

elasticmachine commented Jun 20, 2019

cjcenizal left a comment •

edited

Loading

cjcenizal Jun 20, 2019

lizozom Jun 23, 2019

cjcenizal Jun 25, 2019

cjcenizal commented Jun 22, 2019 •

edited

Loading

lizozom commented Jun 23, 2019 •

edited

Loading

lizozom commented Jun 25, 2019

elasticmachine commented Jun 25, 2019

elasticmachine commented Jun 25, 2019

cjcenizal commented Jun 26, 2019

cjcenizal left a comment •

edited

Loading

cuff-links commented Jun 26, 2019

elasticmachine commented Jun 26, 2019

elasticmachine commented Jun 26, 2019

lizozom commented Jun 26, 2019

lizozom commented Jun 26, 2019 •

edited

Loading

cuff-links left a comment

elasticmachine commented Jun 26, 2019

EUIfy Console - partially de-angularize and move custom views out of top_nav #39341

EUIfy Console - partially de-angularize and move custom views out of top_nav #39341

Conversation

lizozom commented Jun 20, 2019 • edited by cjcenizal Loading

Release note

Summary

Welcome notification

Help

Settings

Important

History

Checklist

lizozom commented Jun 20, 2019

elasticmachine commented Jun 20, 2019

💔 Build Failed

elasticmachine commented Jun 20, 2019

cchaos left a comment

Choose a reason for hiding this comment

elasticmachine commented Jun 20, 2019

💔 Build Failed

cjcenizal left a comment • edited Loading

Choose a reason for hiding this comment

cjcenizal Jun 20, 2019

Choose a reason for hiding this comment

lizozom Jun 23, 2019

Choose a reason for hiding this comment

cjcenizal Jun 25, 2019

Choose a reason for hiding this comment

cjcenizal commented Jun 22, 2019 • edited Loading

lizozom commented Jun 23, 2019 • edited Loading

lizozom commented Jun 25, 2019

elasticmachine commented Jun 25, 2019

💔 Build Failed

elasticmachine commented Jun 25, 2019

💔 Build Failed

cjcenizal commented Jun 26, 2019

cjcenizal left a comment • edited Loading

Choose a reason for hiding this comment

Closing the Welcome panel with the X doesn't "dismiss" it

Font settings input gets NaN

Font settings input and unmounted component

cuff-links commented Jun 26, 2019

elasticmachine commented Jun 26, 2019

💚 Build Succeeded

elasticmachine commented Jun 26, 2019

💚 Build Succeeded

lizozom commented Jun 26, 2019

lizozom commented Jun 26, 2019 • edited Loading

cuff-links left a comment

Choose a reason for hiding this comment

elasticmachine commented Jun 26, 2019

💚 Build Succeeded

lizozom commented Jun 20, 2019 •

edited by cjcenizal

Loading

cjcenizal left a comment •

edited

Loading

cjcenizal commented Jun 22, 2019 •

edited

Loading

lizozom commented Jun 23, 2019 •

edited

Loading

cjcenizal left a comment •

edited

Loading

Font settings input gets `NaN`

lizozom commented Jun 26, 2019 •

edited

Loading