[Canvas] Adds doc links and keyboard shortcut cheatsheat to help menu #31335

cqliu1 · 2019-02-16T00:24:28Z

This moves doc links from the workpad loader to the global nav. This also adds a keyboard shortcut cheatsheet. These shortcuts are dynamically generated from .../lib/keymap.js and displays the shortcuts corresponding to the client OS.

Global Nav Help Menu

Mac Shortcuts

Windows Shortcuts (same shortcuts for Linux and any other OS)

I'll add more shortcuts available in the layout engine in a future PR.

elasticmachine · 2019-02-16T00:24:30Z

Pinging @elastic/kibana-canvas

elasticmachine · 2019-02-16T01:16:27Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

monfera · 2019-02-16T08:14:10Z

x-pack/plugins/canvas/public/components/workpad_header/workpad_header.js

@@ -80,7 +80,7 @@ export const WorkpadHeader = ({
            <EuiFlexItem grow={false}>
              <FullscreenControl>
                {({ toggleFullscreen }) => (
-                  <EuiToolTip position="bottom" content="Toggle fullscreen mode">
+                  <EuiToolTip position="bottom" content="Enter fullscreen mode">


monfera · 2019-02-16T08:15:36Z

x-pack/plugins/canvas/public/components/workpad_page/index.js

+  withHandlers(({ commit }) => ({
+    groupElements: () =>
+      commit('actionEvent', {
+        event: 'group',


monfera · 2019-02-16T08:16:30Z

x-pack/plugins/canvas/public/lib/keymap.js

+    },
+    GROUP: {
+      osx: ['g'],
+      windows: ['g'],


does it need 'G' and 'U' too?

monfera · 2019-02-16T08:22:23Z

x-pack/plugins/canvas/public/lib/keymap.js

+      osx: ['u'],
+      windows: ['u'],
+      linux: ['u'],
+      other: ['u'],


If all platforms have the same key, would a single line of other cover it as a fallback, or is there some other defaulting? I'm not saying enlisting it is not nice, eg. for code documentation purpose.

It looks like react-shortcuts handles both cases. Both lowercase and uppercase keys (i.e with caps lock on) trigger the shortcuts.
Note: this was a response meant for the comment above

If all platforms have the same key, would a single line of other cover it as a fallback, or is there some other defaulting? I'm not saying enlisting it is not nice, eg. for code documentation purpose.

I needed the objects to be a specific shape. A single line would've worked too, but I wanted the shortcut to with the shape

{ help: 'description', osx: ['shortcut'], windows: ['shortcut'], linux: ['shortcut'], other: ['shortcut'], }

because I needed the shortcut to be an object that I could add a help attribute to that is compatible with what react-shortcuts expects as a valid keymap.

monfera · 2019-02-16T08:25:06Z

x-pack/plugins/canvas/public/components/workpad_page/event_handlers.js

-const handleKeyPress = (commit, e, isEditable) => {
-  const { key, target } = e;
-  const upcaseKey = key && key.toUpperCase();
-  if (isEditable && !isTextInput(target) && 'GU'.indexOf(upcaseKey) !== -1) {


I see isTextInput is disused, is it the case that the new handler already isn't active when the input target is a text input tag?

I tested typing in an advanced filter and it appears that the handler doesn't correctly handle the event target being an input control. I'll add the check to the key handler.

monfera · 2019-02-16T08:31:38Z

x-pack/plugins/canvas/public/components/workpad_page/event_handlers.js

-      return false;
-  }
-};
-
 const modifierKey = key => ['KeyALT', 'KeyCONTROL'].indexOf(keyCode(key)) > -1;

 const handleKeyDown = (commit, e, isEditable) => {


I haven't cleaned up some code here because the expectation was that, we're going to add back the initially present layout keyboard shortcuts for moving/rotating/resizing elements, which were initially fully handled in aero. Since we decided that keystrokes should be none of aero's concern, I recently removed the processing code, and I think this handleKeyDown is no longer doing anything useful so could be removed. May be extra safe to search for keyboardEvent under aeroelastic/. A bit tangential to your PR but you're reworking a good bunch of key handling so removing this vestige may free you up for whatever this may have blocked (or just remove a few lines).

I checked, and there aren't any instances of keyboardEvent in the aeroelastic code. I'll go ahead and remove the key event handlers

monfera · 2019-02-16T08:35:34Z

With so many nicely documented shortcuts by now, a context menu will be very useful too :-)

elasticmachine · 2019-02-19T07:34:35Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

ryankeairns

This is awesome, very helpful!

I had some typography and copy changes that I've put into a PR: cqliu1#1

Once those are in, then I'll be 👍 , Thanks!

elasticmachine · 2019-02-19T17:28:15Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

alexfrancoeur · 2019-02-19T18:35:56Z

Great work @cqliu1! Some feedback below.

The docs button text looks much larger in comparison to the rest of Canvas. Is this specific to EUI? Maybe @ryankeairns typography changes addressed this.

I'm guessing so, but are the commands conditional based on OS?

I still see the docs link in the list view. Was the removal of that link meant to be part of this PR as well?

As this feature was snuck in so fast, I'm not sure what the expectations are. Are we meant to continually link out to general Kibana docs. I'd imagine so, I think it's helpful. That being said, i the primary action always meant to be the in context documentation? Would love to hear @cchaos & @ryankeairns thoughts here. FWIW I think the screenshot below makes sense, just want to make sure we're consistent with other implementations.

ryankeairns · 2019-02-19T20:50:40Z

@alexfrancoeur @cqliu1

The font size in the support menu is set globally, outside of Canvas. We're opting to use a smaller font size in the Canvas "chrome", thus the difference.
We should remove that 'Docs' link from the workpad loader.
The way the support menu works is that the top section (including the 'Go to documentation' button) is baked in for everybody. Plugin developers determine what appears below that section which in this case is the Canvas specific text and two buttons.

The original PR for this support menu demonstrated plain text links, however the component was designed to accept whatever content is supplied. In talking with Catherine, the global nature/placement of the support button may not feel like a logical place to find Canvas docs so we discussed emphasizing our links visually - thus the buttons. It's tough, we may be setting an undesirable precedent using buttons here, but then the links feel visually insufficient on their own.

For that reason, I was ok with the buttons, but perhaps a better design solution is needed in the near future.

cchaos · 2019-02-20T04:15:24Z

@ryankeairns , The top section of the help popover is actually optional (on by default). You can opt to remove it by setting useDefaultContent = false

kibana/src/legacy/ui/public/chrome/directives/header_global_nav/components/header_help_menu.tsx

Line 51 in 8aada89

useDefaultContent?: boolean;

elasticmachine · 2019-02-20T17:36:47Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-02-20T18:12:34Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

cqliu1 · 2019-02-20T19:35:46Z

I added one minor change. I changed the arrow keys to arrow symbols in the cheatsheet.

elasticmachine · 2019-02-20T20:09:36Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-02-20T20:18:00Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

alexfrancoeur

LGTM

Fix: reintroduce additional call on keyboard event

elasticmachine · 2019-02-21T18:47:15Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

…elastic#31335) * Added docs link and keyboard shortcuts to global help menu * Fixed tooltip * Removed aeroelastic keyboard event handlers * typography and copy changes * Removed doc links from workpad manager * Added input target check to workpad page keyhandler * Fixed ungrouping * Displays arrow symbols instead of arrow key word * Removed tabIndex * Fix: reintroduce additional call on keyboard event

…#31335) (#31728) * Added docs link and keyboard shortcuts to global help menu * Fixed tooltip * Removed aeroelastic keyboard event handlers * typography and copy changes * Removed doc links from workpad manager * Added input target check to workpad page keyhandler * Fixed ungrouping * Displays arrow symbols instead of arrow key word * Removed tabIndex * Fix: reintroduce additional call on keyboard event

…#31335) (#31727) * Added docs link and keyboard shortcuts to global help menu * Fixed tooltip * Removed aeroelastic keyboard event handlers * typography and copy changes * Removed doc links from workpad manager * Added input target check to workpad page keyhandler * Fixed ungrouping * Displays arrow symbols instead of arrow key word * Removed tabIndex * Fix: reintroduce additional call on keyboard event

…elastic#31335) * Added docs link and keyboard shortcuts to global help menu * Fixed tooltip * Removed aeroelastic keyboard event handlers * typography and copy changes * Removed doc links from workpad manager * Added input target check to workpad page keyhandler * Fixed ungrouping * Displays arrow symbols instead of arrow key word * Removed tabIndex * Fix: reintroduce additional call on keyboard event

…elastic#31335) * Fixed tooltip in fullscreen_control * Removed aeroelastic keyboard event handlers * Added input target check to workpad page keyhandler * Fixed ungrouping * Removed tabIndex * Fix: reintroduce additional call on keyboard event

…#31335) (#31729) * Fixed tooltip in fullscreen_control * Removed aeroelastic keyboard event handlers * Added input target check to workpad page keyhandler * Fixed ungrouping * Removed tabIndex * Fix: reintroduce additional call on keyboard event

cqliu1 added review v7.0.0 Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas v8.0.0 v6.7.0 v7.2.0 labels Feb 16, 2019

cqliu1 requested review from ryankeairns and alexfrancoeur February 16, 2019 00:24

cqliu1 requested a review from a team as a code owner February 16, 2019 00:24

monfera reviewed Feb 16, 2019

View reviewed changes

cqliu1 force-pushed the canvas/help-menu branch from 5f39b82 to df70bfc Compare February 19, 2019 06:42

ryankeairns requested changes Feb 19, 2019

View reviewed changes

cqliu1 and others added 5 commits February 20, 2019 10:09

Added docs link and keyboard shortcuts to global help menu

87bb9f7

Fixed tooltip

6c84053

Removed aeroelastic keyboard event handlers

ba58542

typography and copy changes

36aa689

Removed doc links from workpad manager

16813e0

cqliu1 force-pushed the canvas/help-menu branch from f417d5c to 16813e0 Compare February 20, 2019 17:33

cqliu1 added 2 commits February 20, 2019 11:20

Added input target check to workpad page keyhandler

46bfc5e

Fixed ungrouping

870d4f2

Removed tabIndex

7b2fa27

ryankeairns approved these changes Feb 20, 2019

View reviewed changes

alexfrancoeur approved these changes Feb 21, 2019

View reviewed changes

monfera and others added 2 commits February 21, 2019 15:49

Fix: reintroduce additional call on keyboard event

daaa871

Merge pull request #2 from monfera/cqliu1-canvas/help-menu

f6b0c13

Fix: reintroduce additional call on keyboard event

cqliu1 merged commit 0ad6345 into elastic:master Feb 21, 2019

cqliu1 deleted the canvas/help-menu branch February 21, 2019 18:56

cqliu1 mentioned this pull request Feb 21, 2019

[7.x] [Canvas] Adds doc links and keyboard shortcut cheatsheat to help menu (#31335) #31727

Merged

This was referenced Feb 21, 2019

[7.0] [Canvas] Adds doc links and keyboard shortcut cheatsheat to help menu (#31335) #31728

Merged

[6.7] [Canvas] Adds doc links and keyboard shortcut cheatsheat to help menu (#31335) #31729

Merged

cqliu1 removed the v6.7.0 label Feb 21, 2019

monfera mentioned this pull request Feb 22, 2019

[6.7] [Canvas] Adds doc links and keyboard shortcut cheatsheat to help menu (#31335) #31786

Closed

monfera mentioned this pull request Feb 25, 2019

[Accessibility][Layout Engine] Suggestion: Layer Selector #25505

Closed

cqliu1 added v5.6.7 v6.7.0 and removed v5.6.7 labels Feb 26, 2019

monfera mentioned this pull request Mar 18, 2019

[Canvas] No dummy commit for keyboard events #33414

Merged

rayafratkina added the release_note:enhancement label Mar 22, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Canvas] Adds doc links and keyboard shortcut cheatsheat to help menu #31335

[Canvas] Adds doc links and keyboard shortcut cheatsheat to help menu #31335

cqliu1 commented Feb 16, 2019

elasticmachine commented Feb 16, 2019

elasticmachine commented Feb 16, 2019

monfera Feb 16, 2019

monfera Feb 16, 2019

monfera Feb 16, 2019

monfera Feb 16, 2019

cqliu1 Feb 20, 2019 •

edited

Loading

cqliu1 Feb 21, 2019 •

edited

Loading

monfera Feb 16, 2019

cqliu1 Feb 20, 2019

monfera Feb 16, 2019

cqliu1 Feb 20, 2019

monfera commented Feb 16, 2019

elasticmachine commented Feb 19, 2019

ryankeairns left a comment

elasticmachine commented Feb 19, 2019

alexfrancoeur commented Feb 19, 2019

ryankeairns commented Feb 19, 2019

cchaos commented Feb 20, 2019

elasticmachine commented Feb 20, 2019

elasticmachine commented Feb 20, 2019

cqliu1 commented Feb 20, 2019

elasticmachine commented Feb 20, 2019

elasticmachine commented Feb 20, 2019

alexfrancoeur left a comment

elasticmachine commented Feb 21, 2019

[Canvas] Adds doc links and keyboard shortcut cheatsheat to help menu #31335

[Canvas] Adds doc links and keyboard shortcut cheatsheat to help menu #31335

Conversation

cqliu1 commented Feb 16, 2019

Global Nav Help Menu

Mac Shortcuts

Windows Shortcuts (same shortcuts for Linux and any other OS)

elasticmachine commented Feb 16, 2019

elasticmachine commented Feb 16, 2019

💚 Build Succeeded

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

cqliu1 Feb 20, 2019 • edited Loading

Choose a reason for hiding this comment

cqliu1 Feb 21, 2019 • 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

monfera commented Feb 16, 2019

elasticmachine commented Feb 19, 2019

💔 Build Failed

ryankeairns left a comment

Choose a reason for hiding this comment

elasticmachine commented Feb 19, 2019

💔 Build Failed

alexfrancoeur commented Feb 19, 2019

ryankeairns commented Feb 19, 2019

cchaos commented Feb 20, 2019

elasticmachine commented Feb 20, 2019

💔 Build Failed

elasticmachine commented Feb 20, 2019

💔 Build Failed

cqliu1 commented Feb 20, 2019

elasticmachine commented Feb 20, 2019

💚 Build Succeeded

elasticmachine commented Feb 20, 2019

💚 Build Succeeded

alexfrancoeur left a comment

Choose a reason for hiding this comment

elasticmachine commented Feb 21, 2019

💚 Build Succeeded

cqliu1 Feb 20, 2019 •

edited

Loading

cqliu1 Feb 21, 2019 •

edited

Loading