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

Stacked headers and navigational search #72331

Merged
merged 99 commits into from
Sep 14, 2020
Merged

Conversation

myasonik
Copy link
Contributor

@myasonik myasonik commented Jul 17, 2020

Summary

This PR introduces two major features

1. Stacked (global) header

  • The stacked header design will be consistent across Elastic properties (i.e. Kibana, Cloud, and Dream Machine)
  • The new global header will include both navigational search (see next section) and app-specific controls/menus [1]

2. Navigational search

  • This is an MVP that returns apps and saved objects, today, with more content coming in the future [2]
  • Control (or Command on Mac) + / is the keyboard shortcut to activate the search input
  • Apps are listed first and assigned a higher score. This may change over time but navigation remains our primary use case
  • Future enhancements are being planned and tracked here

Closes #58049, closes #62010, closes #64541, closes #66464, and closes #68524.

Footnotes

[1] Note to app teams regarding menu relocation: there is a new header API that allows you to push app menus to the header via the shred TopNavMenu component. Several apps have already been converted; others are being evaluated as part of this PR

[2] The list of currently included objects can be found here. This link also describes how content gets included and what teams can do to add additional saved objects.


Preview

Screen Shot 2020-09-01 at 3 07 25 PM

In a future PR

Lots of stuff! If you have a feature request, please check our existing backlog first!
Navigational Search Project Backlog


Checklist

Delete any items that are not applicable to this PR.

For maintainers

@myasonik myasonik added release_note:enhancement release highlight Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v8.0.0 REASSIGN from Team:Core UI Deprecated label for old Core UI team v7.10.0 labels Jul 17, 2020
@alexfrancoeur

This comment has been minimized.

@alexfrancoeur

This comment has been minimized.

@ryankeairns
Copy link
Contributor

ryankeairns commented Jul 22, 2020

Quick piece of feedback: Firefrox (mac) already uses Cmd + K to focus the address bar. We'll likely need to come up with something other 🤔

@myasonik
Copy link
Contributor Author

We can... But likely anything we come up with will clash with something. That's largely why I've brought up the keyboard registry — to let people customize it to suit their tools.

As another data point, Slack (for web) uses cmd+k so there's at least precedent 🤷‍♂️ Though you can turn it off in Slack at a user level...

@ryankeairns

This comment has been minimized.

Copy link
Contributor

@cchaos cchaos left a comment

Choose a reason for hiding this comment

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

❤️ Loving all the old hack removals :) 👍 LGTM with just some added suggestions

src/core/public/rendering/_base.scss Outdated Show resolved Hide resolved
src/core/public/rendering/_base.scss Outdated Show resolved Hide resolved
src/plugins/timelion/public/_app.scss Outdated Show resolved Hide resolved
x-pack/plugins/maps/public/_main.scss Outdated Show resolved Hide resolved
@timroes
Copy link
Contributor

timroes commented Sep 14, 2020

@myasonik @ryankeairns I wanted to bring up the topic of international keyboard layouts here and would like to hear the thoughts we had around them during planning the shortcut? While the shortcut might be very easy to hit on a US layout (Ctrl + /) a lot of non US keyboard layouts might have the slash assigned to at least an Shift variation if not even a alt gr variation.

So e.g. to trigger this on a German keyboard you'll need to hit Ctrl + Shift + 7 and similar on other keyboard layouts.

Using a letter (like k or p) would have had the advantage of being always without any modifier key on (at least) any latin based keyboards. Also not sure if the situation for letters could have been improved using key codes instead of the key sign (for special chars I think it's not possible, because those keys might not even exist in specific hardware layouts).

In general I'd like if you could elaborate a bit on the discussions and thoughts about that here for posterity.

@ryankeairns
Copy link
Contributor

ryankeairns commented Sep 14, 2020

@myasonik @ryankeairns I wanted to bring up the topic of international keyboard layouts here and would like to hear the thoughts we had around them during planning the shortcut? While the shortcut might be very easy to hit on a US layout (Ctrl + /) a lot of non US keyboard layouts might have the slash assigned to at least an Shift variation if not even a alt gr variation.

So e.g. to trigger this on a German keyboard you'll need to hit Ctrl + Shift + 7 and similar on other keyboard layouts.

Using a letter (like k or p) would have had the advantage of being always without any modifier key on (at least) any latin based keyboards. Also not sure if the situation for letters could have been improved using key codes instead of the key sign (for special chars I think it's not possible, because those keys might not even exist in specific hardware layouts).

In general I'd like if you could elaborate a bit on the discussions and thoughts about that here for posterity.

The keyboard shortcut has changed more than once since we've started working on this feature. It has been challenging to find a solution that does not conflict with one of the major browsers or screen readers. Given the wide range of possible conflicts - which very per user - our belief is that the shortcut should be customizable.

Our thought process was to a) avoid single key shortcuts since those can conflict with screenreader software's way of navigating via keyboard b) avoid overriding existing browser shortcuts (which vary by browser) c) avoid global shortcuts (e.g. Cut/Copy/Paste/Save, etc.) and d) look for a common, existing pattern.

This leaves us with few alternatives for single-modifier combinations. github.com uses a / to activate its search feature while Slack uses it to toggle display of keyboard shortcuts... so this at least has some potential for familiarity. It may not be the same use, per se, but / is a common and findable key.

In addition to the advanced setting, we also have on our roadmap the creation of a global keyboard shortcut registry to avoid conflict within our own applications as shortcut usage continues to grow.

Copy link
Member

@spong spong left a comment

Choose a reason for hiding this comment

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

Security code changes LGTM! 👍

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

@kbn/optimizer bundle module count

id value diff baseline
core 558 -1 559
globalSearchBar 13 +13 -
kibanaReact 296 +1 295
painlessLab 43 -1 44
total +12

async chunks size

id value diff baseline
advancedSettings 63.8KB -2.2KB 65.9KB
canvas 1.4MB -467.0B 1.4MB
discover 431.2KB +106.0B 431.1KB
graph 1.4MB +87.0B 1.4MB
ingestManager 1.1MB -2.4KB 1.1MB
lens 26.0KB +130.0B 25.9KB
maps 3.3MB +331.0B 3.3MB
painlessLab 182.0KB -2.4KB 184.4KB
timelion 598.3KB -140.0B 598.5KB
visualize 697.8KB +129.0B 697.7KB
total -6.8KB

page load bundle size

id value diff baseline
apm 41.7KB +43.0B 41.7KB
canvas 1.3MB +45.0B 1.3MB
console 30.3KB -124.0B 30.4KB
core 1.2MB -7.0KB 1.2MB
dashboard 711.9KB +375.0B 711.5KB
devTools 166.1KB -128.0B 166.2KB
discover 229.9KB +591.0B 229.3KB
enterpriseSearch 22.5KB +87.0B 22.4KB
globalSearchBar 28.8KB +28.8KB -
graph 15.8KB +2.0B 15.8KB
infra 277.4KB +17.0B 277.3KB
ingestManager 468.4KB -4.0B 468.4KB
kibanaLegacy 232.6KB +47.0B 232.5KB
kibanaReact 648.7KB +480.0B 648.2KB
management 30.1KB -2.0B 30.1KB
maps 297.9KB +146.0B 297.8KB
ml 845.3KB +47.0B 845.3KB
navigation 164.7KB -860.0B 165.5KB
observability 51.8KB +32.0B 51.7KB
painlessLab 161.0KB +4.0B 161.0KB
searchprofiler 50.8KB -136.0B 50.9KB
securitySolution 793.0KB +46.0B 793.0KB
timelion 14.4KB -1.0B 14.4KB
uptime 25.1KB +8.0B 25.1KB
visualize 41.2KB +94.0B 41.1KB
total +22.7KB

distributable file count

id value diff baseline
default 45543 +4 45539

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@myasonik myasonik merged commit 61c4e6f into elastic:master Sep 14, 2020
@myasonik myasonik deleted the search-header branch September 14, 2020 19:32
gmmorris added a commit to gmmorris/kibana that referenced this pull request Sep 15, 2020
* master: (25 commits)
  [Security Solution] Add unit tests for Network search strategy (elastic#77416)
  [Alerting] Improves performance of the authorization filter in AlertsClient.find by skipping KQL parsing (elastic#77040)
  [Ingest Manager] Add route for package installation by upload (elastic#77044)
  [APM-UI][E2E] filter PRs from the uptime GH team (elastic#77359)
  [APM] Remove useLocation and some minor route improvements (elastic#76343)
  [Enterprise Search] Update enterpriseSearchRequestHandler to manage range of errors + add handleAPIErrors helper (elastic#77258)
  [SECURITY_SOLUTION] Task/hostname policy response ux updates (elastic#76444)
  Move remaining uses of serviceName away from urlParams (elastic#77248)
  [Lens] Move configuration popover to flyout (elastic#76046)
  [Ingest Manager] Manually build Fleet kuery with Node arguments (elastic#76589)
  skip flaky suite (elastic#59975)
  Neutral-naming in reporting plugin (elastic#77371)
  [Enterprise Search] Add UserIcon styles (elastic#77385)
  [RUM Dashboard] Added loading state to visitor breakdown pie charts (elastic#77201)
  [Ingest Manager] Fix polling for new agent action (elastic#77339)
  Remote cluster - Functional UI test to change the superuser to a test_user with limited role (elastic#77212)
  Stacked headers and navigational search (elastic#72331)
  [ML] DF Analytics creation wizard: Fixing field loading race condition (elastic#77326)
  [Monitoring] Handle no mappings found for sort and collapse fields (elastic#77099)
  Add Lens to Recently Accessed (elastic#77249)
  ...
gmmorris added a commit to gmmorris/kibana that referenced this pull request Sep 15, 2020
* master: (293 commits)
  Fix tsvb filter ration for table (elastic#77272)
  [Security Solution] Add unit tests for Network search strategy (elastic#77416)
  [Alerting] Improves performance of the authorization filter in AlertsClient.find by skipping KQL parsing (elastic#77040)
  [Ingest Manager] Add route for package installation by upload (elastic#77044)
  [APM-UI][E2E] filter PRs from the uptime GH team (elastic#77359)
  [APM] Remove useLocation and some minor route improvements (elastic#76343)
  [Enterprise Search] Update enterpriseSearchRequestHandler to manage range of errors + add handleAPIErrors helper (elastic#77258)
  [SECURITY_SOLUTION] Task/hostname policy response ux updates (elastic#76444)
  Move remaining uses of serviceName away from urlParams (elastic#77248)
  [Lens] Move configuration popover to flyout (elastic#76046)
  [Ingest Manager] Manually build Fleet kuery with Node arguments (elastic#76589)
  skip flaky suite (elastic#59975)
  Neutral-naming in reporting plugin (elastic#77371)
  [Enterprise Search] Add UserIcon styles (elastic#77385)
  [RUM Dashboard] Added loading state to visitor breakdown pie charts (elastic#77201)
  [Ingest Manager] Fix polling for new agent action (elastic#77339)
  Remote cluster - Functional UI test to change the superuser to a test_user with limited role (elastic#77212)
  Stacked headers and navigational search (elastic#72331)
  [ML] DF Analytics creation wizard: Fixing field loading race condition (elastic#77326)
  [Monitoring] Handle no mappings found for sort and collapse fields (elastic#77099)
  ...
myasonik pushed a commit that referenced this pull request Sep 15, 2020
Co-authored-by: Poff Poffenberger <poffdeluxe@gmail.com>
Co-authored-by: Ryan Keairns <contactryank@gmail.com>
Co-authored-by: pgayvallet <pierre.gayvallet@elastic.co>
Co-authored-by: cchaos <caroline.horn@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
REASSIGN from Team:Core UI Deprecated label for old Core UI team release highlight release_note:enhancement Team:APM All issues that need APM UI Team support Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Team:Fleet Team label for Observability Data Collection Fleet team Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v7.10.0 v8.0.0
Projects
None yet