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

Persist tooltips on hover #23959

Merged
merged 1 commit into from
Jul 15, 2020
Merged

Persist tooltips on hover #23959

merged 1 commit into from
Jul 15, 2020

Conversation

ntsekouras
Copy link
Contributor

Description

Fixes: #23958

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

@ntsekouras ntsekouras added [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Type] Regression Related to a regression in the latest release Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta labels Jul 15, 2020
@ntsekouras ntsekouras self-assigned this Jul 15, 2020
@@ -17,9 +17,6 @@
font-size: 12px;
box-shadow: none;

// This prevents quickly hovering the tooltip from blocking hovering something else.
pointer-events: none;
Copy link
Contributor

Choose a reason for hiding this comment

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

@jasmussen do you remember why we add this? (It was added on the original G2 branch).

Copy link
Contributor

Choose a reason for hiding this comment

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

From the comment, it looks like it was added to make the tooltip "invisible" to a cursor so a user wouldn't accidentally click it when trying to click something else in the UI or on the canvas.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think Michael is right, yes. If you hover a button and see a tooltip, that might cover a button immediately above it, and if you quickly click there to click that button, you should be able to.

@ItsJonQ
Copy link

ItsJonQ commented Jul 15, 2020

Tested locally, and it looks like it resolves the issue! Tooltips are able to persist.

It looks like it was added during the G2 design refresh. I feel like there's a reason for this, but I'm lacking the context.
I know the Toolbar was iterated on a bunch during that phase. Perhaps this change was once needed, but not so much anymore?

Dropdowns, Top Toolbar mode, and typical mouse-based navigation still work.

I'm comfortable with this fix @ntsekouras !

🚀

@github-actions
Copy link

Size Change: -20 B (0%)

Total Size: 1.14 MB

Filename Size Change
build/components/style-rtl.css 15.8 kB -10 B (0%)
build/components/style.css 15.8 kB -10 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.67 kB 0 B
build/api-fetch/index.js 3.39 kB 0 B
build/autop/index.js 2.82 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 7.67 kB 0 B
build/block-directory/style-rtl.css 944 B 0 B
build/block-directory/style.css 945 B 0 B
build/block-editor/index.js 115 kB 0 B
build/block-editor/style-rtl.css 10.8 kB 0 B
build/block-editor/style.css 10.8 kB 0 B
build/block-library/editor-rtl.css 7.6 kB 0 B
build/block-library/editor.css 7.59 kB 0 B
build/block-library/index.js 132 kB 0 B
build/block-library/style-rtl.css 7.77 kB 0 B
build/block-library/style.css 7.77 kB 0 B
build/block-library/theme-rtl.css 728 B 0 B
build/block-library/theme.css 729 B 0 B
build/block-serialization-default-parser/index.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 48.2 kB 0 B
build/components/index.js 199 kB 0 B
build/compose/index.js 9.67 kB 0 B
build/core-data/index.js 11.5 kB 0 B
build/data-controls/index.js 1.29 kB 0 B
build/data/index.js 8.45 kB 0 B
build/date/index.js 5.38 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 569 B 0 B
build/dom/index.js 3.23 kB 0 B
build/edit-navigation/index.js 10.8 kB 0 B
build/edit-navigation/style-rtl.css 1.08 kB 0 B
build/edit-navigation/style.css 1.08 kB 0 B
build/edit-post/index.js 304 kB 0 B
build/edit-post/style-rtl.css 5.61 kB 0 B
build/edit-post/style.css 5.61 kB 0 B
build/edit-site/index.js 16.8 kB 0 B
build/edit-site/style-rtl.css 3.04 kB 0 B
build/edit-site/style.css 3.04 kB 0 B
build/edit-widgets/index.js 9.35 kB 0 B
build/edit-widgets/style-rtl.css 2.45 kB 0 B
build/edit-widgets/style.css 2.45 kB 0 B
build/editor/editor-styles-rtl.css 537 B 0 B
build/editor/editor-styles.css 539 B 0 B
build/editor/index.js 45.1 kB 0 B
build/editor/style-rtl.css 3.78 kB 0 B
build/editor/style.css 3.78 kB 0 B
build/element/index.js 4.65 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 7.72 kB 0 B
build/format-library/style-rtl.css 547 B 0 B
build/format-library/style.css 548 B 0 B
build/hooks/index.js 2.13 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.56 kB 0 B
build/is-shallow-equal/index.js 709 B 0 B
build/keyboard-shortcuts/index.js 2.51 kB 0 B
build/keycodes/index.js 1.94 kB 0 B
build/list-reusable-blocks/index.js 3.12 kB 0 B
build/list-reusable-blocks/style-rtl.css 476 B 0 B
build/list-reusable-blocks/style.css 476 B 0 B
build/media-utils/index.js 5.32 kB 0 B
build/notices/index.js 1.79 kB 0 B
build/nux/index.js 3.4 kB 0 B
build/nux/style-rtl.css 671 B 0 B
build/nux/style.css 668 B 0 B
build/plugins/index.js 2.56 kB 0 B
build/primitives/index.js 1.4 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/rich-text/index.js 13.9 kB 0 B
build/server-side-render/index.js 2.71 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.27 kB 0 B
build/url/index.js 4.06 kB 0 B
build/viewport/index.js 1.85 kB 0 B
build/warning/index.js 1.13 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

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

I tested this as well and it seems to work great, I can't remember why this has been added.

@ntsekouras ntsekouras merged commit 11a0cd5 into master Jul 15, 2020
@ntsekouras ntsekouras deleted the fix/persist-tooltip-on-hover branch July 15, 2020 16:56
@github-actions github-actions bot added this to the Gutenberg 8.6 milestone Jul 15, 2020
@afercia
Copy link
Contributor

afercia commented Jul 15, 2020

Thanks for fixing this so quickly. This should be included in WP 5.5 though, what is the best way to make sure that happens? @youknowriad

@youknowriad
Copy link
Contributor

yes, It has the "backport to Core" label, it means it will be backported before the next WordPress 5.5 beta.

@ellatrix ellatrix mentioned this pull request Jul 20, 2020
6 tasks
ellatrix pushed a commit that referenced this pull request Jul 20, 2020
@youknowriad youknowriad removed the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Jul 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Type] Regression Related to a regression in the latest release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Accessibility regression: Tooltips don't persist on hover any longer
6 participants