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.

Sign up for GitHub

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

Accessibility: Dismiss BlockMover tooltips on escape key press. #15578

Closed
wants to merge 22 commits into from
Closed

Accessibility: Dismiss BlockMover tooltips on escape key press. #15578

Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
ae832dc
Accessibility: dismiss BlockMover tooltips on escape key press.
nicolad May 11, 2019
dc86b93
Accessibility: Make tooltips dismissable onMouseDown.
nicolad May 11, 2019
9a61a03
Merge branch 'master' into accessibility/improve-tooltips-behavior
nicolad May 12, 2019
861351c
Merge branch 'master' into accessibility/improve-tooltips-behavior
nicolad May 13, 2019
ceae20d
Merge branch 'master' into accessibility/improve-tooltips-behavior
nicolad May 14, 2019
62b163d
Merge branch 'master' into accessibility/improve-tooltips-behavior
nicolad May 14, 2019
60bbc6c
Imported KeyboardShortcuts into Tooltip.
nicolad May 20, 2019
8ae4d23
Fixed KeyboardShortcuts import and wrapped children with this.
nicolad May 20, 2019
4379cf3
Updated the import and wrapped popover content.
nicolad May 20, 2019
919831f
Wrapped correct elements.
nicolad May 25, 2019
f0962b3
test-unit:update summary --- 7 snapshots updated from 2 test suites.
nicolad May 25, 2019
9741f77
unit-tests temp fix
nicolad May 25, 2019
ee9b1a5
Conflicts fixed.
nicolad Jun 18, 2019
0331775
Merge branch 'master' of github.com:WordPress/gutenberg into accessib…
nicolad Jun 30, 2019
67cfdf7
Merge branch 'master' of github.com:WordPress/gutenberg into accessib…
nicolad Jul 24, 2019
b23a066
Removed KeyboardShortcuts and checked escape key press.
nicolad Jul 24, 2019
2dfaf5b
Updated snapshots.
nicolad Jul 24, 2019
10e7a32
Merge branch 'master' into accessibility/improve-tooltips-behavior
nicolad Sep 14, 2019
a6f7be2
Moved destructuring back.
nicolad Sep 14, 2019
22ef4e2
Merge branch 'master' of github.com:WordPress/gutenberg into accessib…
nicolad Oct 8, 2019
68dd831
Removed obsolete onMouseEnter and onMouseLeave handlers.
nicolad Oct 8, 2019
87321bd
Conflicts fixed.
nicolad Jun 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions packages/components/src/color-palette/test/__snapshots__/index.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,7 @@ exports[`ColorPalette should render a dynamic toolbar of colors 1`] = `
onBlur={[Function]}
onClick={[Function]}
onFocus={[Function]}
onKeyDown={[Function]}
onMouseDown={[Function]}
onMouseEnter={[Function]}
onMouseLeave={[Function]}
Expand Down Expand Up @@ -361,6 +362,7 @@ exports[`ColorPalette should render a dynamic toolbar of colors 1`] = `
onBlur={[Function]}
onClick={[Function]}
onFocus={[Function]}
onKeyDown={[Function]}
onMouseDown={[Function]}
onMouseEnter={[Function]}
onMouseLeave={[Function]}
Expand Down Expand Up @@ -399,6 +401,7 @@ exports[`ColorPalette should render a dynamic toolbar of colors 1`] = `
onBlur={[Function]}
onClick={[Function]}
onFocus={[Function]}
onKeyDown={[Function]}
onMouseDown={[Function]}
onMouseEnter={[Function]}
onMouseLeave={[Function]}
Expand Down
6 changes: 6 additions & 0 deletions packages/components/src/color-picker/test/__snapshots__/index.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ exports[`ColorPicker should commit changes to all views on blur 1`] = `
onBlur={[Function]}
onClick={[Function]}
onFocus={[Function]}
onKeyDown={[Function]}
onMouseDown={[Function]}
onMouseEnter={[Function]}
onMouseLeave={[Function]}
Expand Down Expand Up @@ -322,6 +323,7 @@ exports[`ColorPicker should commit changes to all views on keyDown = DOWN 1`] =
onBlur={[Function]}
onClick={[Function]}
onFocus={[Function]}
onKeyDown={[Function]}
onMouseDown={[Function]}
onMouseEnter={[Function]}
onMouseLeave={[Function]}
Expand Down Expand Up @@ -496,6 +498,7 @@ exports[`ColorPicker should commit changes to all views on keyDown = ENTER 1`] =
onBlur={[Function]}
onClick={[Function]}
onFocus={[Function]}
onKeyDown={[Function]}
onMouseDown={[Function]}
onMouseEnter={[Function]}
onMouseLeave={[Function]}
Expand Down Expand Up @@ -670,6 +673,7 @@ exports[`ColorPicker should commit changes to all views on keyDown = UP 1`] = `
onBlur={[Function]}
onClick={[Function]}
onFocus={[Function]}
onKeyDown={[Function]}
onMouseDown={[Function]}
onMouseEnter={[Function]}
onMouseLeave={[Function]}
Expand Down Expand Up @@ -844,6 +848,7 @@ exports[`ColorPicker should only update input view for draft changes 1`] = `
onBlur={[Function]}
onClick={[Function]}
onFocus={[Function]}
onKeyDown={[Function]}
onMouseDown={[Function]}
onMouseEnter={[Function]}
onMouseLeave={[Function]}
Expand Down Expand Up @@ -1018,6 +1023,7 @@ exports[`ColorPicker should render color picker 1`] = `
onBlur={[Function]}
onClick={[Function]}
onFocus={[Function]}
onKeyDown={[Function]}
onMouseDown={[Function]}
onMouseEnter={[Function]}
onMouseLeave={[Function]}
Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/icon-button/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ function IconButton( props, ref ) {
// the children are empty and...
( ! children || ( isArray( children ) && ! children.length ) ) &&
// the tooltip is not explicitly disabled.
false !== tooltip
tooltip !== false
Copy link
Member Author

Choose a reason for hiding this comment

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

Readability improvement.

Copy link
Member

Choose a reason for hiding this comment

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

Noting that this will probably conflict with changes currently proposed in #19193 . Regardless if it's an improvement, I might suggest to remove it here, since it's not entirely relevant (or close in proximity) to the intended changes.

)
);

Expand Down
9 changes: 8 additions & 1 deletion packages/components/src/tooltip/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
cloneElement,
concatChildren,
} from '@wordpress/element';
import { ESCAPE } from '@wordpress/keycodes';

/**
* Internal dependencies
Expand Down Expand Up @@ -89,6 +90,11 @@ class Tooltip extends Component {
return;
}

// If pressed key is escape, no further actions are needed.
Copy link
Member

Choose a reason for hiding this comment

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

I'm a bit confused by this. What actions have been taken at this point in the event handler when pressing Escape?

if ( event.keyCode === ESCAPE ) {
return;
}

// A focus event will occur as a result of a mouse click, but it
// should be disambiguated between interacting with the button and
// using an explicit focus shift as a cue to display the tooltip.
Expand Down Expand Up @@ -157,10 +163,11 @@ class Tooltip extends Component {
return cloneElement( child, {
onMouseEnter: this.createToggleIsOver( 'onMouseEnter', true ),
onMouseLeave: this.createToggleIsOver( 'onMouseLeave' ),
onMouseDown: this.createSetIsMouseDown( true ),
onClick: this.createToggleIsOver( 'onClick' ),
onKeyDown: this.createToggleIsOver( 'onKeyDown' ),
onFocus: this.createToggleIsOver( 'onFocus' ),
onBlur: this.createToggleIsOver( 'onBlur' ),
onMouseDown: this.createSetIsMouseDown( true ),
children: concatChildren(
child.props.children,
isOver && (
Expand Down
1 change: 1 addition & 0 deletions packages/components/src/tooltip/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
.components-tooltip.components-popover {
z-index: z-index(".components-tooltip");
cursor: default;

&::before {
border-color: transparent;
Expand Down