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

Console to NP ready #43346

Merged
merged 42 commits into from
Sep 12, 2019
Merged

Conversation

jloleysens
Copy link
Contributor

@jloleysens jloleysens commented Aug 15, 2019

Summary

Actioning part of phase 1 of this roadmap item, read more details on future work there.

The goals with this PR are

  • to create an interface between brace/ace editor
  • make Console as NP ready as reasonably possible, challenges remain with directives likekbn-top-nav-v2. These are not going to be addressed here we will also need to remove top nav.
  • create a way for us to easily switch to another editor in future (parsing and autocomplete refactoring will not be done here however).

Current Todos before first major review:

  • General structure of Public w/ legacy brace + autocomplete 🤔
  • Refactor Resizer functionality
  • Refactor Play Button
  • Refactor Auto-completion
  • Refactor Docs opener
  • Refactor Storage
  • Refactor History Viewer
    • Move over history functionality
    • Move over history components
  • Refactor a11y ui ace keyboard service (probably a React hook?)
  • Welcome component
  • Help/Tutorial component
  • Settings
  • Replace top-nav-menu

Next:

  • Tests!

Server-side will be done in a separate PR -> NP (try to keep this quite simple for this PR as there is a lot going on already)

How to review

For the most part everything from the user perspective should be 100% intact. So if you've used Console before check that:

  • all of your historical interactions are still visible/listed under the history viewer
  • all of your settings are still as they were (e.g., font size)
  • width settings will be overridden
  • you shouldn't see the welcome screen again!
  • watch out for weird scrolling behaviour

Visible updates includes:

  • Shared screen width between panels is now proportional (not a fixed px width)
  • The top nav (with History, Settings and Help) is below the dev tools navigation :yey:

Additional things:

  • auto complete still behaves the same way (added one small fix here for undefined values sometimes coming through 🤦‍♂)
  • first time users should see the welcome panel

Release Note

Console has had the first of a series of refactoring to its underlying architecture done. This will put us in a stronger position to build new features and improvements moving forward. This refactor includes minor UI improvements but should leave the User Experience mostly unchanged.

Screenshot 2019-09-12 at 14 03 35

@jloleysens jloleysens added Feature:Console Dev Tools Console Feature Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more v7.5.0 labels Aug 15, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/es-ui

@jloleysens jloleysens changed the title Chore/console to np ready [Console] Console to NP ready Aug 15, 2019
@jloleysens jloleysens changed the title [Console] Console to NP ready Console to NP ready Aug 15, 2019
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

 Refactor Resizer functionality (panel component)
 Refactor Play Button
 Refactor Auto-completion
 Refactor Docs opener
 Refactor Storage
@elasticmachine
Copy link
Contributor

💔 Build Failed

@jloleysens jloleysens force-pushed the chore/console-to-np-ready branch from fcd7cf0 to a6c4045 Compare August 28, 2019 09:51
@elasticmachine
Copy link
Contributor

💔 Build Failed

- Pre-emptive changes to tests
- sense-history -> HistoryList
- removed unused kbn top nav v2 component
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

Also moved over and integrated remaining three react components
@elasticmachine
Copy link
Contributor

💔 Build Failed

Copy link
Contributor

@streamich streamich left a comment

Choose a reason for hiding this comment

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

I mostly looked at NP-ready setup, looks good, just added a couple of comments:

@jloleysens
Copy link
Contributor Author

Retest

@elasticmachine
Copy link
Contributor

💔 Build Failed

@spalger
Copy link
Contributor

spalger commented Sep 10, 2019

Please merge master

…-to-np-ready

* 'master' of github.com:elastic/kibana:
  Revert "Revert "[ci] compress jobs for CI stability" (elastic#44584)"
  add src/plugins to the list of plugin dirs to watch (elastic#45033)
  De-angularize and EUI-ficate Discover Context control elements (elastic#44474)
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@cjcenizal
Copy link
Contributor

cjcenizal commented Sep 10, 2019

@jloleysens The tabbing bug is fixed but now there's a new bug. 🤕 When I tab to the editor, hitting Enter doesn't enter editing mode. And if I click on the editor with the mouse to begin editing mode, hitting Escape doesn't exist editing mode and re-display the prompt.

image

Major fix for ace in IE11 and Edge browsers
@jloleysens
Copy link
Contributor Author

jloleysens commented Sep 11, 2019

@cjcenizal I was able to reproduce what you were describing - apologies for the oversight. I did some further testing and found an issue on IE11 after my styling changes so if you could take a look again at my last commit that would be great. Re-tested most functionality and I think it should be ready (if CI passes).

@elasticmachine
Copy link
Contributor

💔 Build Failed

…-to-np-ready

* 'master' of github.com:elastic/kibana: (25 commits)
  [ML] Fixes display of matching modules in index data visualizer (elastic#45261)
  [Console] Update indentation behaviour (elastic#45249)
  Convert value provided to PhraseValueInput to string to catch Exception (elastic#45259)
  [Region Map] Fix loading default vector map and base layer setting (elastic#43858)
  [ML] Fixing empty time range when cloning jobs (elastic#45286)
  [ML] Fixing wizard validation delay (elastic#45265)
  [Logs UI] Interpret finished analysis jobs as healthy (elastic#45268)
  [Console] SQL template with triple quote in completion (elastic#45248)
  [ML] Data Frames: Cards as links (elastic#45254)
  fix(code/frontend): should show updating instead of cloning when updating (elastic#45238)
  fix(code/frontend): fix document search result from (elastic#45236)
  disable another flaky suite (elastic#45323) (elastic#45330)
  disable flaky suite (elastic#45105)
  skip flaky suite (elastic#43069)
  skip flaky suite (elastic#45089)
  disable jest suite that has no enabled tests (elastic#44250)
  disable flaky test (elastic#45317)
  disable flaky test (elastic#45315)
  [DOCS] Creates developer folder (elastic#45280)
  [SIEM] Changes ML conditional links to use tabs, fixes a small bug with null filterQuery   (elastic#45218)
  ...
@elasticmachine
Copy link
Contributor

💔 Build Failed

…-to-np-ready

* 'master' of github.com:elastic/kibana:
  Upgrade EUI to 13.8.1 (elastic#45052)
  [ML] Add multi metric job wizard test (elastic#45279)
  [SIEM] Inject/apply KQL changed in refresh button (elastic#45065)
  [Graph] Type persistence (elastic#44985)
  Functional tests: convert more test/services to TS (elastic#45176)
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Contributor

@cjcenizal cjcenizal left a comment

Choose a reason for hiding this comment

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

Tested locally, code LGTM!

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@jloleysens jloleysens merged commit 5c2d0ca into elastic:master Sep 12, 2019
@jloleysens jloleysens deleted the chore/console-to-np-ready branch September 12, 2019 11:28
jloleysens added a commit to jloleysens/kibana that referenced this pull request Sep 12, 2019
*  General structure of Public w/ legacy brace + autocomplete 🤔
 Refactor Resizer functionality (panel component)
 Refactor Play Button
 Refactor Auto-completion
 Refactor Docs opener
 Refactor Storage

* First refactor of kbn ace keyboard mode to TS+React

* clean up unused props

* console_menu.js -> console_menu.tsx

* Remove unused file from quarantine and added fixed ui ace keyboard mode react hook

* - Refactored history and storage to app-wide services
- Pre-emptive changes to tests
- sense-history -> HistoryList
- removed unused kbn top nav v2 component

* A lot of cleanup, re-introduced editor resize checker, re-introduced history viewer as TS+React. `history` still needs refactoring.

* First iteration of tap nav menu, with history toggle working

* Lots of fixes
Also moved over and integrated remaining three react components

* Moved a lot of files around again, tidied up NP set up

* Replace angular directive

* Remove used code

* Re-order imports and move all ace dependencies to same location

* Remove more unused code

* Revise quarantined setup mocks

* Don't suggest 'undefined' or other null-like values in autocomplete

* Clean up api_server folder

* Re-add missing style

* Updated karma spec mock

* Fix editors cutting of at bottom of screen

* Refactor console editors into single components
Refactor a lot of business logic to main.tsx container
Minor renaming of variables for better readability

* Updated use of contexts with better error message
Fixed broken render sync cycles (using useCallback)
Fixed Main container render cycle (added missing deps to useEffect)
Fixed default input and removed auto indent from being called on init for already formatted text

* Updated test mocks

* Update to be more in line with NP conventions https://github.com/elastic/kibana/blob/master/src/core/CONVENTIONS.md

* Update console history when making new requests
Fixed spacing between editor and console history
Moved registration of keyboard commands to TS
Fixed setup_mocks.js after renaming app to application

* Clean up git merge conflict artifact

* Use updated NP interfaces

* More typings fixed after updating local project dependencies

* Removing some dependencies on KUI and font awesome from legacy editor

* Fix clear history not re-rendering
Refactor prop name to be more descriptive

* Simplify split_panel and add tests

* Fix accessibility tabbing behaviour for ace editor

* Refactor ConsoleEditor into two separate components
Remove unused changeCursor code
Remove unused textArea ref
Use default lodash debounce (remove unnecessary arg)

* Major a11y fix when tabbing
Major fix for ace in IE11 and Edge browsers

* Update comment
@jloleysens jloleysens added release_note:enhancement and removed release_note:skip Skip the PR/issue when compiling release notes labels Sep 12, 2019
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

jloleysens added a commit that referenced this pull request Sep 12, 2019
*  General structure of Public w/ legacy brace + autocomplete 🤔
 Refactor Resizer functionality (panel component)
 Refactor Play Button
 Refactor Auto-completion
 Refactor Docs opener
 Refactor Storage

* First refactor of kbn ace keyboard mode to TS+React

* clean up unused props

* console_menu.js -> console_menu.tsx

* Remove unused file from quarantine and added fixed ui ace keyboard mode react hook

* - Refactored history and storage to app-wide services
- Pre-emptive changes to tests
- sense-history -> HistoryList
- removed unused kbn top nav v2 component

* A lot of cleanup, re-introduced editor resize checker, re-introduced history viewer as TS+React. `history` still needs refactoring.

* First iteration of tap nav menu, with history toggle working

* Lots of fixes
Also moved over and integrated remaining three react components

* Moved a lot of files around again, tidied up NP set up

* Replace angular directive

* Remove used code

* Re-order imports and move all ace dependencies to same location

* Remove more unused code

* Revise quarantined setup mocks

* Don't suggest 'undefined' or other null-like values in autocomplete

* Clean up api_server folder

* Re-add missing style

* Updated karma spec mock

* Fix editors cutting of at bottom of screen

* Refactor console editors into single components
Refactor a lot of business logic to main.tsx container
Minor renaming of variables for better readability

* Updated use of contexts with better error message
Fixed broken render sync cycles (using useCallback)
Fixed Main container render cycle (added missing deps to useEffect)
Fixed default input and removed auto indent from being called on init for already formatted text

* Updated test mocks

* Update to be more in line with NP conventions https://github.com/elastic/kibana/blob/master/src/core/CONVENTIONS.md

* Update console history when making new requests
Fixed spacing between editor and console history
Moved registration of keyboard commands to TS
Fixed setup_mocks.js after renaming app to application

* Clean up git merge conflict artifact

* Use updated NP interfaces

* More typings fixed after updating local project dependencies

* Removing some dependencies on KUI and font awesome from legacy editor

* Fix clear history not re-rendering
Refactor prop name to be more descriptive

* Simplify split_panel and add tests

* Fix accessibility tabbing behaviour for ace editor

* Refactor ConsoleEditor into two separate components
Remove unused changeCursor code
Remove unused textArea ref
Use default lodash debounce (remove unnecessary arg)

* Major a11y fix when tabbing
Major fix for ace in IE11 and Edge browsers

* Update comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Console Dev Tools Console Feature Feature:NP Migration Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more v7.5.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants