-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
[SearchProfiler] SearchProfiler to NP #48795
Merged
jloleysens
merged 42 commits into
elastic:master
from
jloleysens:np-migration/searchprofiler
Oct 31, 2019
Merged
[SearchProfiler] SearchProfiler to NP #48795
jloleysens
merged 42 commits into
elastic:master
from
jloleysens:np-migration/searchprofiler
Oct 31, 2019
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- highlight details (with render test) - searchprofiler tabs - (wip) profile tree - (wip) shard details
💔 Build Failed |
💔 Build Failed |
💔 Build Failed |
…und into individual files (wip)
…ts off to 100.0...)
jloleysens
changed the title
[WiP][SearchProfiler] SearchProfiler to NP
[SearchProfiler] SearchProfiler to NP
Oct 24, 2019
💔 Build Failed |
💔 Build Failed |
@elasticmachine merge upstream |
💔 Build Failed |
@elasticmachine merge upstream |
💔 Build Failed |
Retest |
💔 Build Failed |
@elasticmachine merge upstream |
💔 Build Failed |
💔 Build Failed |
miukimiu
approved these changes
Oct 31, 2019
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We agreed to merge this PR and create a new PR with UX/UI improvements from the issue #49772
💔 Build Failed |
💚 Build Succeeded |
jloleysens
added a commit
to jloleysens/kibana
that referenced
this pull request
Oct 31, 2019
* Added the following components: - highlight details (with render test) - searchprofiler tabs - (wip) profile tree - (wip) shard details * First iteration of ProfileTree component (needs render test) * Remove space * ProfileTree render test * Add profile tree test to git index * First iteration of editor component with render test * First iteration of nearly functional public * Fix highlight_details_flyout render test * Move NP directory to public and fix import issue created by directly importing FormattedMessage * Rendering and looking more normal * Fix type issues and fix a11y for ace editor * Added ability to do profile requests again and render into UI (styling WiP) * Fix props in editor test * Added empty tree placeholder component (with test), moved styling around into individual files (wip) * Fix path * Lots of style updates and added util for determining visible children (+test) * Re-add missing badge and make it slightly wider (otherwise 100.00% cuts off to 100.0...) * Delete legacy public! * SCSS refactor + fix for re-rendering editor * UI and server updates after license checks * [skip ci] Add server np_ready code * fix i18n * Re-enable error annotations * Minor UX improvements (focus editor after failed request and no tabindex for textarea without active license) Added some spaces to make code more readable * Removed xpackMain from ServerShim Updated use of notifications -> notifications.toasts from np core setup Removed TODO for using core.application.register (not available for legacy apps) * Added placeholder component for loading state and implemented useReducer * Refactor actions * Changes after PR feedback: - TS for unsafe utils test fixtures - Safer use of .selfTime (no more NaN) - Sentence case where applicable - Cleaned up TODOs - Fix styling issue with percentage on badges of profile tree - Refactor name of profile hook (now useRequestProfile) - Fixed copy paste issue in highlight flyout `Total time` -> `Self time` - Restyled the profile button to be fill and not take up the full horizontal space - Removed the `Type` input from the profiler query section * Removed .type from backend and cleanup translations * Disable responsive UI layout for now * Remove buggy error annotation code * - Refactored percentage badge to own component - Updated styles after testing on IE11 - Updated styles after testing on Safari - Chrome and FF worked on this commit * Update missing i18n and fix use ace ui keyboard hook * Update useEffect dependencies array for editor component * Use absolute path to dev tools app (to fix CI) * Remove file extensions * Re-add missing data-test-subj
jloleysens
added a commit
that referenced
this pull request
Nov 1, 2019
* [SearchProfiler] SearchProfiler to NP (#48795) * Added the following components: - highlight details (with render test) - searchprofiler tabs - (wip) profile tree - (wip) shard details * First iteration of ProfileTree component (needs render test) * Remove space * ProfileTree render test * Add profile tree test to git index * First iteration of editor component with render test * First iteration of nearly functional public * Fix highlight_details_flyout render test * Move NP directory to public and fix import issue created by directly importing FormattedMessage * Rendering and looking more normal * Fix type issues and fix a11y for ace editor * Added ability to do profile requests again and render into UI (styling WiP) * Fix props in editor test * Added empty tree placeholder component (with test), moved styling around into individual files (wip) * Fix path * Lots of style updates and added util for determining visible children (+test) * Re-add missing badge and make it slightly wider (otherwise 100.00% cuts off to 100.0...) * Delete legacy public! * SCSS refactor + fix for re-rendering editor * UI and server updates after license checks * [skip ci] Add server np_ready code * fix i18n * Re-enable error annotations * Minor UX improvements (focus editor after failed request and no tabindex for textarea without active license) Added some spaces to make code more readable * Removed xpackMain from ServerShim Updated use of notifications -> notifications.toasts from np core setup Removed TODO for using core.application.register (not available for legacy apps) * Added placeholder component for loading state and implemented useReducer * Refactor actions * Changes after PR feedback: - TS for unsafe utils test fixtures - Safer use of .selfTime (no more NaN) - Sentence case where applicable - Cleaned up TODOs - Fix styling issue with percentage on badges of profile tree - Refactor name of profile hook (now useRequestProfile) - Fixed copy paste issue in highlight flyout `Total time` -> `Self time` - Restyled the profile button to be fill and not take up the full horizontal space - Removed the `Type` input from the profiler query section * Removed .type from backend and cleanup translations * Disable responsive UI layout for now * Remove buggy error annotation code * - Refactored percentage badge to own component - Updated styles after testing on IE11 - Updated styles after testing on Safari - Chrome and FF worked on this commit * Update missing i18n and fix use ace ui keyboard hook * Update useEffect dependencies array for editor component * Use absolute path to dev tools app (to fix CI) * Remove file extensions * Re-add missing data-test-subj * Remove template/index.html
5 tasks
cjcenizal
added
Feature:NP Migration
Team:Kibana Management
Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more
labels
Apr 22, 2020
Pinging @elastic/es-ui (Team:Elasticsearch UI) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Feature:Dev Tools
Feature:NP Migration
Feature:Search Profiler
Team:Kibana Management
Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more
v7.6.0
v8.0.0
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Search profiler new platform migration.
General
Public
Screenshots old
Screenshots new
New states (w/ new copy)
Server
How to review
The focus here has been on re-architecting the existing NG app into React+TS+EUI.
State Management
Because the app doesn't have much app state we are not introducing any more heavy state management tools (just React Context for sharing more general dependencies).
UI
There are some custom SCSS shenanigans going on here, primarily the profile tree visualization. I've also organised the SCSS files by component and containers. So the styles directory more or less mirrors the TS files.
UX
It's important that we, for the most part preserve pre-existing behaviour:
Check that the UI updates as expected after:
1. A request is made
2. An index (or sub component) is expanded or collapsed. It would be really good to test with deep nesting here (what are good queries we can test with here?)
3. Test weird edge case combinations. E.g., What if we send a request with the flyout open?
Where possible we want to improve UX and UI. Suggestions from design or ES UI are welcome!UI/UX improvements will probably be done on a separate PR