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

Don't use target="_blank" unless needed #25730

Merged
merged 6 commits into from
Nov 17, 2020
Merged

Conversation

aristath
Copy link
Member

We have target="_blank" added to read-more buttons, category links, and also as the default on post-titles. These are internal links to the site and should not open in a new tab, it's not what the visitor would expect.
This also ties in a11y - see https://www.w3.org/TR/WCAG20-TECHS/G200.html
Useful article: When to use target="_blank"

Types of changes

Remove target="_blank" rel="noreferrer noopener" from categories, post-titles, latest-posts.

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.

@github-actions
Copy link

github-actions bot commented Sep 30, 2020

Size Change: -13 B (0%)

Total Size: 1.2 MB

Filename Size Change
build/block-library/index.js 144 kB -13 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.54 kB 0 B
build/api-fetch/index.js 3.35 kB 0 B
build/autop/index.js 2.72 kB 0 B
build/blob/index.js 668 B 0 B
build/block-directory/index.js 8.59 kB 0 B
build/block-directory/style-rtl.css 943 B 0 B
build/block-directory/style.css 942 B 0 B
build/block-editor/index.js 130 kB 0 B
build/block-editor/style-rtl.css 11 kB 0 B
build/block-editor/style.css 10.9 kB 0 B
build/block-library/editor-rtl.css 8.93 kB 0 B
build/block-library/editor.css 8.93 kB 0 B
build/block-library/style-rtl.css 7.69 kB 0 B
build/block-library/style.css 7.69 kB 0 B
build/block-library/theme-rtl.css 741 B 0 B
build/block-library/theme.css 741 B 0 B
build/block-serialization-default-parser/index.js 1.77 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 47.6 kB 0 B
build/components/index.js 170 kB 0 B
build/components/style-rtl.css 15.4 kB 0 B
build/components/style.css 15.4 kB 0 B
build/compose/index.js 9.63 kB 0 B
build/core-data/index.js 12.1 kB 0 B
build/data-controls/index.js 684 B 0 B
build/data/index.js 8.63 kB 0 B
build/date/index.js 31.9 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 569 B 0 B
build/dom/index.js 4.43 kB 0 B
build/edit-navigation/index.js 10.6 kB 0 B
build/edit-navigation/style-rtl.css 868 B 0 B
build/edit-navigation/style.css 871 B 0 B
build/edit-post/index.js 306 kB 0 B
build/edit-post/style-rtl.css 6.37 kB 0 B
build/edit-post/style.css 6.35 kB 0 B
build/edit-site/index.js 21.6 kB 0 B
build/edit-site/style-rtl.css 3.8 kB 0 B
build/edit-site/style.css 3.81 kB 0 B
build/edit-widgets/index.js 26.6 kB 0 B
build/edit-widgets/style-rtl.css 3.09 kB 0 B
build/edit-widgets/style.css 3.09 kB 0 B
build/editor/editor-styles-rtl.css 480 B 0 B
build/editor/editor-styles.css 482 B 0 B
build/editor/index.js 42.6 kB 0 B
build/editor/style-rtl.css 3.85 kB 0 B
build/editor/style.css 3.84 kB 0 B
build/element/index.js 4.45 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 7.49 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 1.74 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.54 kB 0 B
build/is-shallow-equal/index.js 709 B 0 B
build/keyboard-shortcuts/index.js 2.39 kB 0 B
build/keycodes/index.js 1.85 kB 0 B
build/list-reusable-blocks/index.js 3.02 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.12 kB 0 B
build/notices/index.js 1.69 kB 0 B
build/nux/index.js 3.27 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.45 kB 0 B
build/primitives/index.js 1.35 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/reusable-blocks/index.js 3.06 kB 0 B
build/rich-text/index.js 13 kB 0 B
build/server-side-render/index.js 2.61 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.24 kB 0 B
build/url/index.js 4.06 kB 0 B
build/viewport/index.js 1.75 kB 0 B
build/warning/index.js 1.13 kB 0 B
build/wordcount/index.js 1.23 kB 0 B

compressed-size-action

Copy link
Contributor

@ntsekouras ntsekouras left a comment

Choose a reason for hiding this comment

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

Looks good! Thanks 👍

@jasmussen
Copy link
Contributor

Hmm. I suppose this does allow us to not have that heavy "open in new tab" icon, right?

But I wonder — in some of these cases, the open externally feels like it is there so as to not interfere with the document being edited. If you have unsaved changes and click one of these links it'll prompt you. Which maybe is fine?

@aristath
Copy link
Member Author

in some of these cases, the open externally feels like it is there so as to not interfere with the document being edited. If you have unsaved changes and click one of these links it'll prompt you. Which maybe is fine?

Yeah, I don't see what the problem with that would be... In general I feel we should find a better way to handle content links inside the editor, but that is a bigger topic not related to this PR.

@ntsekouras
Copy link
Contributor

But I wonder — in some of these cases, the open externally feels like it is there so as to not interfere with the document being edited. If you have unsaved changes and click one of these links it'll prompt you. Which maybe is fine?

I'm a bit torn on this one too, but I feel the goal is to achieve better wysiwyg behavior, no? If you do that now, it will open a new tab that will interrupt you in some way too and is inconsistent with front-end that opens in same tab.

@jasmussen
Copy link
Contributor

Oh I see, this is for content. For some reason I was thinking of these:

Screenshot 2020-09-30 at 10 40 13

Screenshot 2020-09-30 at 10 40 29

This seems okay to me as a trivial step, though I think there's a larger discussion to be had about whether those links should be clickable at all. Inside a reusable block, links are not clickable:

Screenshot 2020-09-30 at 10 44 37

And even normal hyperlinks aren't clickable — they open a popover, sure, but aren't directly clickable:

Screenshot 2020-09-30 at 10 44 49

An argument could be made that either links inside query blocks such as Latest Posts should be be visual only and blocked entirely just like inside a reusable block. Or, especially if aspects of these links need be configured, they open a popover just like normal hyperlinks.

Finally there's "Browse mode", which I picture being a 3rd tool in the tools dropdown. This tool lets you click any link and navigate to its editable counterpart inside the editor. For example if you click the "About" link in a navigation menu, you go and edit that page.

@aristath
Copy link
Member Author

@jasmussen ah you're right, the categories one was what you posted above.
Reverted that change, so that leaves only the latest-posts change 👍

@aristath aristath merged commit 270f6eb into master Nov 17, 2020
@aristath aristath deleted the fix/links-review-_blank branch November 17, 2020 12:42
@github-actions github-actions bot added this to the Gutenberg 9.5 milestone Nov 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants