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
+18 −1
Closed

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

Show file tree
Hide file tree
Changes from 7 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
4 changes: 4 additions & 0 deletions packages/block-editor/src/components/block-mover/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ export class BlockMover extends Component {
aria-disabled={ isFirst }
onFocus={ this.onFocus }
onBlur={ this.onBlur }
onMouseEnter={ this.onFocus }
Copy link
Member

Choose a reason for hiding this comment

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

Could you explain this change?

Copy link
Member Author

Choose a reason for hiding this comment

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

This PR is based on @afercia 's PR
Maybe @afercia might help here since I've iterated over his initial work?

Copy link
Contributor

Choose a reason for hiding this comment

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

Honestly I can't remember why I made that change. That was one year ago 😬 Sorry!

Copy link
Member Author

Choose a reason for hiding this comment

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

@aduth @afercia, FYI - I have reverted these changes.

onMouseLeave={ this.onBlur }
/>
<IconDragHandle
className="editor-block-mover__control block-editor-block-mover__control"
Expand All @@ -85,6 +87,8 @@ export class BlockMover extends Component {
aria-disabled={ isLast }
onFocus={ this.onFocus }
onBlur={ this.onBlur }
onMouseEnter={ this.onFocus }
onMouseLeave={ this.onBlur }
/>
<span id={ `block-editor-block-mover__up-description-${ instanceId }` } className="editor-block-mover__description block-editor-block-mover__description">
{
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 @@ -45,7 +45,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
3 changes: 3 additions & 0 deletions 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 { KeyboardShortcuts } from '@wordpress/components';
Copy link
Member Author

Choose a reason for hiding this comment

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

@aduth, this is a little bit strange, just by importing KeyboardShortcuts I do get that nasty error:
Fatal error: Allowed memory size of 268435456 bytes exhausted ...
Am I doing something wrong here?
Peek 2019-05-20 16-57

Copy link
Contributor

Choose a reason for hiding this comment

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

you can't import something from the same package using the @wordpress/* syntax, just use relative paths.

Copy link
Member

Choose a reason for hiding this comment

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

Discussed in Slack thread (link requires registration):

https://wordpress.slack.com/archives/C02QB2JS7/p1557863367231300


/**
* Internal dependencies
Expand Down Expand Up @@ -104,7 +105,9 @@ class Tooltip extends Component {
return cloneElement( child, {
onMouseEnter: this.createToggleIsOver( 'onMouseEnter', true ),
onMouseLeave: this.createToggleIsOver( 'onMouseLeave' ),
onMouseDown: this.createToggleIsOver( 'onMouseDown' ),
onClick: this.createToggleIsOver( 'onClick' ),
onKeyDown: this.createToggleIsOver( 'onKeyDown' ),
onFocus: this.createToggleIsOver( 'onFocus' ),
onBlur: this.createToggleIsOver( 'onBlur' ),
children: concatChildren(
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