[UI Framework] Improve KuiContextMenu keyboard navigation UX (#14434)

* Refactor focus state logic to use the React lifecycle correctly. * Update KuiPopover snapshots. * Remove unnecessary isVisible prop from KuiContextMenu. * Allow user to both tab AND use the arrow keys for navigation. * Reinstate ability to tab and shift-tab to the title of KuiContextMenuPanel. * Release focus from Dashboard panel options KuiContextMenu by closing it when you select an option. * Update KuiContextMenu example to demonstrate best practice of closing the menu when an item is clicked. * Replace native transitionend event handler with onAnimationEnd React event handler.
elastic · Oct 25, 2017 · ec38367 · ec38367
1 parent 97e393d
commit ec38367
Show file tree

Hide file tree

Showing 20 changed files with 486 additions and 244 deletions.
diff --git a/src/core_plugins/kibana/public/dashboard/panel/panel_options_menu.js b/src/core_plugins/kibana/public/dashboard/panel/panel_options_menu.js
@@ -18,12 +18,30 @@ export class PanelOptionsMenu extends React.Component {
 
   closePopover = () => this.setState({ isPopoverOpen: false });
 
+  onEditPanel = () => {
+    this.closePopover();
+    this.props.onEditPanel();
+  };
+
+  onDeletePanel = () => {
+    this.closePopover();
+
+    if (this.props.onDeletePanel) {
+      this.props.onDeletePanel();
+    }
+  };
+
+  onToggleExpandPanel = () => {
+    this.closePopover();
+    this.props.onToggleExpandPanel();
+  };
+
   renderItems() {
     const items = [(
       <KuiContextMenuItem
         key="0"
         data-test-subj="dashboardPanelEditLink"
-        onClick={this.props.onEditPanel}
+        onClick={this.onEditPanel}
         icon={(
           <span
             aria-hidden="true"
@@ -37,7 +55,7 @@ export class PanelOptionsMenu extends React.Component {
       <KuiContextMenuItem
         key="1"
         data-test-subj="dashboardPanelExpandIcon"
-        onClick={this.props.onToggleExpandPanel}
+        onClick={this.onToggleExpandPanel}
         icon={(
           <span
             aria-hidden="true"
@@ -54,7 +72,7 @@ export class PanelOptionsMenu extends React.Component {
         <KuiContextMenuItem
           key="2"
           data-test-subj="dashboardPanelRemoveIcon"
-          onClick={this.props.onDeletePanel}
+          onClick={this.onDeletePanel}
           icon={(
             <span
               aria-hidden="true"

diff --git a/test/functional/apps/dashboard/_panel_controls.js b/test/functional/apps/dashboard/_panel_controls.js
@@ -104,6 +104,7 @@ export default function ({ getService, getPageObjects }) {
 
       describe('visualization object edit menu', () => {
         it('opens a visualization when edit link is clicked', async () => {
+          await testSubjects.click('dashboardPanelToggleMenuIcon');
           await PageObjects.dashboard.clickDashboardPanelEditLink();
           await PageObjects.header.waitUntilLoadingHasFinished();
           const currentUrl = await remote.getCurrentUrl();

diff --git a/ui_framework/dist/ui_framework.css b/ui_framework/dist/ui_framework.css
@@ -884,7 +884,13 @@ main {
 .kuiContextMenuPanel {
   width: 100%;
   visibility: visible;
-  background-color: #ffffff; }
+  background-color: #ffffff;
+  /**
+   * 1. Override global focus style.
+   */ }
+  .kuiContextMenuPanel:focus {
+    box-shadow: none;
+    /* 1 */ }
   .kuiContextMenuPanel.kuiContextMenuPanel-txInLeft {
     pointer-events: none;
     -webkit-animation: kuiContextMenuPanelTxInLeft 250ms cubic-bezier(0.694, 0.0482, 0.335, 1);

diff --git a/ui_framework/doc_site/src/views/context_menu/context_menu.js b/ui_framework/doc_site/src/views/context_menu/context_menu.js
@@ -41,7 +41,7 @@ export default class extends Component {
         icon: (
           <span className="kuiIcon fa-search" />
         ),
-        onClick: () => window.alert('Show fullscreen'),
+        onClick: () => { this.closePopover(); window.alert('Show fullscreen'); },
       }, {
         name: 'Share this dasbhoard',
         icon: <span className="kuiIcon fa-user" />,
@@ -51,11 +51,11 @@ export default class extends Component {
           items: [{
             name: 'PDF reports',
             icon: <span className="kuiIcon fa-user" />,
-            onClick: () => window.alert('PDF reports'),
+            onClick: () => { this.closePopover(); window.alert('PDF reports'); },
           }, {
             name: 'CSV reports',
             icon: <span className="kuiIcon fa-user" />,
-            onClick: () => window.alert('CSV reports'),
+            onClick: () => { this.closePopover(); window.alert('CSV reports'); },
           }, {
             name: 'Embed code',
             icon: <span className="kuiIcon fa-user" />,
@@ -95,38 +95,38 @@ export default class extends Component {
           }, {
             name: 'Permalinks',
             icon: <span className="kuiIcon fa-user" />,
-            onClick: () => window.alert('Permalinks'),
+            onClick: () => { this.closePopover(); window.alert('Permalinks'); },
           }],
         },
       }, {
         name: 'Edit / add panels',
         icon: <span className="kuiIcon fa-user" />,
-        onClick: () => window.alert('Edit / add panels'),
+        onClick: () => { this.closePopover(); window.alert('Edit / add panels'); },
       }, {
         name: 'Display options',
         icon: <span className="kuiIcon fa-user" />,
-        onClick: () => window.alert('Display options'),
+        onClick: () => { this.closePopover(); window.alert('Display options'); },
       }],
     };
 
     this.panels = flattenPanelTree(panelTree);
   }
 
-  onButtonClick() {
-    this.setState({
-      isPopoverOpen: !this.state.isPopoverOpen,
-    });
-  }
+  onButtonClick = () => {
+    this.setState(prevState => ({
+      isPopoverOpen: !prevState.isPopoverOpen,
+    }));
+  };
 
-  closePopover() {
+  closePopover = () => {
     this.setState({
       isPopoverOpen: false,
     });
-  }
+  };
 
   render() {
     const button = (
-      <KuiButton buttonType="basic" onClick={this.onButtonClick.bind(this)}>
+      <KuiButton buttonType="basic" onClick={this.onButtonClick}>
         Click me to load a context menu
       </KuiButton>
     );
@@ -135,14 +135,13 @@ export default class extends Component {
       <KuiPopover
         button={button}
         isOpen={this.state.isPopoverOpen}
-        closePopover={this.closePopover.bind(this)}
+        closePopover={this.closePopover}
         panelPaddingSize="none"
         withTitle
         anchorPosition="left"
       >
         <KuiContextMenu
           initialPanelId={0}
-          isVisible={this.state.isPopoverOpen}
           panels={this.panels}
         />
       </KuiPopover>

diff --git a/ui_framework/doc_site/src/views/context_menu/single_panel.js b/ui_framework/doc_site/src/views/context_menu/single_panel.js
@@ -18,21 +18,21 @@ export default class extends Component {
     };
   }
 
-  onButtonClick() {
-    this.setState({
-      isPopoverOpen: !this.state.isPopoverOpen,
-    });
-  }
+  onButtonClick = () => {
+    this.setState(prevState => ({
+      isPopoverOpen: !prevState.isPopoverOpen,
+    }));
+  };
 
-  closePopover() {
+  closePopover = () => {
     this.setState({
       isPopoverOpen: false,
     });
-  }
+  };
 
   render() {
     const button = (
-      <KuiButton buttonType="basic" onClick={this.onButtonClick.bind(this)}>
+      <KuiButton buttonType="basic" onClick={this.onButtonClick}>
         Click me to load a context menu
       </KuiButton>
     );
@@ -41,23 +41,23 @@ export default class extends Component {
       <KuiContextMenuItem
         key="A"
         icon={<span className="kuiIcon fa-user" />}
-        onClick={() => { window.alert('A'); }}
+        onClick={() => { this.closePopover(); window.alert('A'); }}
       >
         Option A
       </KuiContextMenuItem>
     ), (
       <KuiContextMenuItem
         key="B"
         icon={<span className="kuiIcon fa-user" />}
-        onClick={() => { window.alert('B'); }}
+        onClick={() => { this.closePopover(); window.alert('B'); }}
       >
         Option B
       </KuiContextMenuItem>
     ), (
       <KuiContextMenuItem
         key="C"
         icon={<span className="kuiIcon fa-user" />}
-        onClick={() => { window.alert('C'); }}
+        onClick={() => { this.closePopover(); window.alert('C'); }}
       >
         Option C
       </KuiContextMenuItem>
@@ -67,7 +67,7 @@ export default class extends Component {
       <KuiPopover
         button={button}
         isOpen={this.state.isPopoverOpen}
-        closePopover={this.closePopover.bind(this)}
+        closePopover={this.closePopover}
         panelPaddingSize="none"
         withTitle
         anchorPosition="left"

diff --git a/ui_framework/doc_site/src/views/popover/popover.js b/ui_framework/doc_site/src/views/popover/popover.js
@@ -37,6 +37,7 @@ export default class extends Component {
 
     return (
       <KuiPopover
+        isFocusable
         button={button}
         isOpen={this.state.isPopoverOpen}
         closePopover={this.closePopover.bind(this)}

diff --git a/ui_framework/doc_site/src/views/popover/popover_anchor_position.js b/ui_framework/doc_site/src/views/popover/popover_anchor_position.js
@@ -45,6 +45,7 @@ export default class extends Component {
     return (
       <div>
         <KuiPopover
+          isFocusable
           button={(
             <KuiButton buttonType="basic" onClick={this.onButtonClick1.bind(this)}>
               Popover anchored to the right.
@@ -60,6 +61,7 @@ export default class extends Component {
         &nbsp;
 
         <KuiPopover
+          isFocusable
           button={(
             <KuiButton buttonType="basic" onClick={this.onButtonClick2.bind(this)}>
               Popover anchored to the left.

diff --git a/ui_framework/doc_site/src/views/popover/popover_body_class_name.js b/ui_framework/doc_site/src/views/popover/popover_body_class_name.js
@@ -31,6 +31,7 @@ export default class extends Component {
   render() {
     return (
       <KuiPopover
+        isFocusable
         button={(
           <KuiButton buttonType="basic" onClick={this.onButtonClick.bind(this)}>
             Custom class

diff --git a/ui_framework/doc_site/src/views/popover/popover_panel_class_name.js b/ui_framework/doc_site/src/views/popover/popover_panel_class_name.js
@@ -31,6 +31,7 @@ export default class extends Component {
   render() {
     return (
       <KuiPopover
+        isFocusable
         button={(
           <KuiButton buttonType="basic" onClick={this.onButtonClick.bind(this)}>
             Turn padding off and apply a custom class

diff --git a/ui_framework/doc_site/src/views/popover/popover_with_title.js b/ui_framework/doc_site/src/views/popover/popover_with_title.js
@@ -41,6 +41,7 @@ export default class extends Component {
 
     return (
       <KuiPopover
+        isFocusable
         button={button}
         isOpen={this.state.isPopoverOpen}
         closePopover={this.closePopover.bind(this)}