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

Add Opens in new Tab control into Link Preview #53566

Merged
merged 9 commits into from
Aug 15, 2023

Conversation

getdave
Copy link
Contributor

@getdave getdave commented Aug 11, 2023

What?

Adds the ability to toggle the Opens in new tab link setting from the preview step of the Link UI.

Also keeps the Link UI open when toggling the checkbox. This has been deferred until a unrelated bug can be resolved.

Why?

User feedback is that toggling this control is extremely common and should be made easier. Exposing it in the preview will enable this whilst not cluttering the initial link creation step.

How?

Add render slot into the bottom of the Link Preview component and expose the LinkSettings control there exposing only the opensInNewTab setting.

When toggled the onChange of the component will be called with only that value present.

Testing Instructions

  • New Post
  • Add text
  • Create link on text
  • When submitted see the "preview" is immediately displayed
  • See the Opens in new tab checkbox in the preview.
  • Toggle the checkbox. See the preview remains open/active.
  • Click away from link.
  • Click back to link. Check that Opens in new tab settings was retained.
  • Click "Edit" on link. Check Opens in new tab setting matches in edit mode.
  • Toggle the setting in edit mode and Save.
  • Check that the setting shown on preview is correct.

Testing Instructions for Keyboard

Screenshots or screencast

Screen.Capture.on.2023-08-11.at.12-58-46.mp4

@getdave getdave requested a review from richtabor August 11, 2023 11:18
@getdave getdave self-assigned this Aug 11, 2023
@getdave getdave added [Type] Enhancement A suggestion for improvement. [Feature] Link Editing Link components (LinkControl, URLInput) and integrations (RichText link formatting) labels Aug 11, 2023
@github-actions
Copy link

github-actions bot commented Aug 11, 2023

Size Change: +2.22 kB (0%)

Total Size: 1.51 MB

Filename Size Change
build/block-editor/index.min.js 212 kB +1.41 kB (+1%)
build/block-editor/style-rtl.css 14.9 kB +30 B (0%)
build/block-editor/style.css 14.9 kB +24 B (0%)
build/block-library/blocks/button/style-rtl.css 629 B +5 B (+1%)
build/block-library/blocks/button/style.css 628 B +5 B (+1%)
build/block-library/blocks/cover/style-rtl.css 1.63 kB +22 B (+1%)
build/block-library/blocks/cover/style.css 1.62 kB +23 B (+1%)
build/block-library/blocks/freeform/editor-rtl.css 2.61 kB +2 B (0%)
build/block-library/blocks/image/style-rtl.css 1.42 kB -3 B (0%)
build/block-library/blocks/image/style.css 1.41 kB -9 B (-1%)
build/block-library/blocks/image/view-interactivity.min.js 1.83 kB +368 B (+25%) 🚨
build/block-library/blocks/media-text/style-rtl.css 505 B -2 B (0%)
build/block-library/blocks/media-text/style.css 503 B -2 B (0%)
build/block-library/blocks/navigation/editor-rtl.css 2.26 kB +1 B (0%)
build/block-library/blocks/navigation/editor.css 2.26 kB +1 B (0%)
build/block-library/blocks/pullquote/theme-rtl.css 168 B +1 B (+1%)
build/block-library/blocks/pullquote/theme.css 168 B +1 B (+1%)
build/block-library/blocks/search/style-rtl.css 607 B -1 B (0%)
build/block-library/blocks/search/style.css 607 B -1 B (0%)
build/block-library/blocks/social-links/style-rtl.css 1.44 kB +1 B (0%)
build/block-library/blocks/social-links/style.css 1.43 kB +1 B (0%)
build/block-library/blocks/table/editor-rtl.css 432 B -1 B (0%)
build/block-library/blocks/table/editor.css 432 B -1 B (0%)
build/block-library/blocks/table/style-rtl.css 639 B -6 B (-1%)
build/block-library/blocks/table/style.css 639 B -5 B (-1%)
build/block-library/editor-rtl.css 12.1 kB +2 B (0%)
build/block-library/editor.css 12.1 kB +1 B (0%)
build/block-library/index.min.js 203 kB +21 B (0%)
build/block-library/style-rtl.css 13.8 kB +31 B (0%)
build/block-library/style.css 13.8 kB +24 B (0%)
build/block-library/theme-rtl.css 688 B +2 B (0%)
build/block-library/theme.css 693 B +2 B (0%)
build/commands/index.min.js 15.3 kB +4 B (0%)
build/components/index.min.js 245 kB +93 B (0%)
build/components/style-rtl.css 11.8 kB -1 B (0%)
build/components/style.css 11.8 kB -1 B (0%)
build/core-commands/index.min.js 2.58 kB +141 B (+6%) 🔍
build/edit-post/index.min.js 35.8 kB +78 B (0%)
build/edit-post/style-rtl.css 7.61 kB +18 B (0%)
build/edit-post/style.css 7.6 kB +18 B (0%)
build/edit-site/index.min.js 91.1 kB +25 B (0%)
build/edit-site/style-rtl.css 13.2 kB -17 B (0%)
build/edit-site/style.css 13.2 kB -19 B (0%)
build/edit-widgets/index.min.js 16.9 kB +8 B (0%)
build/edit-widgets/style-rtl.css 4.53 kB +1 B (0%)
build/edit-widgets/style.css 4.53 kB +1 B (0%)
build/editor/index.min.js 45.5 kB +9 B (0%)
build/editor/style-rtl.css 3.53 kB -28 B (-1%)
build/editor/style.css 3.52 kB -28 B (-1%)
build/format-library/index.min.js 7.56 kB -33 B (0%)
build/private-apis/index.min.js 958 B +7 B (+1%)
build/reusable-blocks/index.min.js 2.7 kB -9 B (0%)
build/rich-text/index.min.js 11 kB +4 B (0%)
ℹ️ View Unchanged
Filename Size
build/a11y/index.min.js 955 B
build/annotations/index.min.js 2.69 kB
build/api-fetch/index.min.js 2.28 kB
build/autop/index.min.js 2.1 kB
build/blob/index.min.js 451 B
build/block-directory/index.min.js 7.01 kB
build/block-directory/style-rtl.css 1.02 kB
build/block-directory/style.css 1.02 kB
build/block-editor/content-rtl.css 4.26 kB
build/block-editor/content.css 4.25 kB
build/block-editor/default-editor-styles-rtl.css 381 B
build/block-editor/default-editor-styles.css 381 B
build/block-library/blocks/archives/editor-rtl.css 61 B
build/block-library/blocks/archives/editor.css 60 B
build/block-library/blocks/archives/style-rtl.css 90 B
build/block-library/blocks/archives/style.css 90 B
build/block-library/blocks/audio/editor-rtl.css 150 B
build/block-library/blocks/audio/editor.css 150 B
build/block-library/blocks/audio/style-rtl.css 122 B
build/block-library/blocks/audio/style.css 122 B
build/block-library/blocks/audio/theme-rtl.css 126 B
build/block-library/blocks/audio/theme.css 126 B
build/block-library/blocks/avatar/editor-rtl.css 116 B
build/block-library/blocks/avatar/editor.css 116 B
build/block-library/blocks/avatar/style-rtl.css 104 B
build/block-library/blocks/avatar/style.css 104 B
build/block-library/blocks/block/editor-rtl.css 305 B
build/block-library/blocks/block/editor.css 305 B
build/block-library/blocks/button/editor-rtl.css 584 B
build/block-library/blocks/button/editor.css 582 B
build/block-library/blocks/buttons/editor-rtl.css 337 B
build/block-library/blocks/buttons/editor.css 337 B
build/block-library/blocks/buttons/style-rtl.css 332 B
build/block-library/blocks/buttons/style.css 332 B
build/block-library/blocks/calendar/style-rtl.css 239 B
build/block-library/blocks/calendar/style.css 239 B
build/block-library/blocks/categories/editor-rtl.css 113 B
build/block-library/blocks/categories/editor.css 112 B
build/block-library/blocks/categories/style-rtl.css 124 B
build/block-library/blocks/categories/style.css 124 B
build/block-library/blocks/code/editor-rtl.css 53 B
build/block-library/blocks/code/editor.css 53 B
build/block-library/blocks/code/style-rtl.css 121 B
build/block-library/blocks/code/style.css 121 B
build/block-library/blocks/code/theme-rtl.css 124 B
build/block-library/blocks/code/theme.css 124 B
build/block-library/blocks/columns/editor-rtl.css 108 B
build/block-library/blocks/columns/editor.css 108 B
build/block-library/blocks/columns/style-rtl.css 421 B
build/block-library/blocks/columns/style.css 421 B
build/block-library/blocks/comment-author-avatar/editor-rtl.css 125 B
build/block-library/blocks/comment-author-avatar/editor.css 125 B
build/block-library/blocks/comment-content/style-rtl.css 92 B
build/block-library/blocks/comment-content/style.css 92 B
build/block-library/blocks/comment-template/style-rtl.css 199 B
build/block-library/blocks/comment-template/style.css 198 B
build/block-library/blocks/comments-pagination-numbers/editor-rtl.css 123 B
build/block-library/blocks/comments-pagination-numbers/editor.css 121 B
build/block-library/blocks/comments-pagination/editor-rtl.css 222 B
build/block-library/blocks/comments-pagination/editor.css 209 B
build/block-library/blocks/comments-pagination/style-rtl.css 235 B
build/block-library/blocks/comments-pagination/style.css 231 B
build/block-library/blocks/comments-title/editor-rtl.css 75 B
build/block-library/blocks/comments-title/editor.css 75 B
build/block-library/blocks/comments/editor-rtl.css 840 B
build/block-library/blocks/comments/editor.css 839 B
build/block-library/blocks/comments/style-rtl.css 637 B
build/block-library/blocks/comments/style.css 636 B
build/block-library/blocks/cover/editor-rtl.css 647 B
build/block-library/blocks/cover/editor.css 650 B
build/block-library/blocks/details/editor-rtl.css 65 B
build/block-library/blocks/details/editor.css 65 B
build/block-library/blocks/details/style-rtl.css 98 B
build/block-library/blocks/details/style.css 98 B
build/block-library/blocks/embed/editor-rtl.css 293 B
build/block-library/blocks/embed/editor.css 293 B
build/block-library/blocks/embed/style-rtl.css 410 B
build/block-library/blocks/embed/style.css 410 B
build/block-library/blocks/embed/theme-rtl.css 126 B
build/block-library/blocks/embed/theme.css 126 B
build/block-library/blocks/file/editor-rtl.css 316 B
build/block-library/blocks/file/editor.css 316 B
build/block-library/blocks/file/style-rtl.css 280 B
build/block-library/blocks/file/style.css 281 B
build/block-library/blocks/file/view-interactivity.min.js 317 B
build/block-library/blocks/file/view.min.js 375 B
build/block-library/blocks/footnotes/style-rtl.css 201 B
build/block-library/blocks/footnotes/style.css 199 B
build/block-library/blocks/freeform/editor.css 2.61 kB
build/block-library/blocks/gallery/editor-rtl.css 947 B
build/block-library/blocks/gallery/editor.css 952 B
build/block-library/blocks/gallery/style-rtl.css 1.53 kB
build/block-library/blocks/gallery/style.css 1.53 kB
build/block-library/blocks/gallery/theme-rtl.css 108 B
build/block-library/blocks/gallery/theme.css 108 B
build/block-library/blocks/group/editor-rtl.css 654 B
build/block-library/blocks/group/editor.css 654 B
build/block-library/blocks/group/style-rtl.css 57 B
build/block-library/blocks/group/style.css 57 B
build/block-library/blocks/group/theme-rtl.css 78 B
build/block-library/blocks/group/theme.css 78 B
build/block-library/blocks/heading/style-rtl.css 76 B
build/block-library/blocks/heading/style.css 76 B
build/block-library/blocks/html/editor-rtl.css 336 B
build/block-library/blocks/html/editor.css 337 B
build/block-library/blocks/image/editor-rtl.css 834 B
build/block-library/blocks/image/editor.css 833 B
build/block-library/blocks/image/theme-rtl.css 126 B
build/block-library/blocks/image/theme.css 126 B
build/block-library/blocks/latest-comments/style-rtl.css 357 B
build/block-library/blocks/latest-comments/style.css 357 B
build/block-library/blocks/latest-posts/editor-rtl.css 213 B
build/block-library/blocks/latest-posts/editor.css 212 B
build/block-library/blocks/latest-posts/style-rtl.css 478 B
build/block-library/blocks/latest-posts/style.css 478 B
build/block-library/blocks/list/style-rtl.css 88 B
build/block-library/blocks/list/style.css 88 B
build/block-library/blocks/media-text/editor-rtl.css 266 B
build/block-library/blocks/media-text/editor.css 263 B
build/block-library/blocks/more/editor-rtl.css 431 B
build/block-library/blocks/more/editor.css 431 B
build/block-library/blocks/navigation-link/editor-rtl.css 712 B
build/block-library/blocks/navigation-link/editor.css 711 B
build/block-library/blocks/navigation-link/style-rtl.css 115 B
build/block-library/blocks/navigation-link/style.css 115 B
build/block-library/blocks/navigation-submenu/editor-rtl.css 296 B
build/block-library/blocks/navigation-submenu/editor.css 295 B
build/block-library/blocks/navigation/style-rtl.css 2.23 kB
build/block-library/blocks/navigation/style.css 2.22 kB
build/block-library/blocks/navigation/view-interactivity.min.js 988 B
build/block-library/blocks/navigation/view-modal.min.js 2.85 kB
build/block-library/blocks/navigation/view.min.js 469 B
build/block-library/blocks/nextpage/editor-rtl.css 395 B
build/block-library/blocks/nextpage/editor.css 395 B
build/block-library/blocks/page-list/editor-rtl.css 401 B
build/block-library/blocks/page-list/editor.css 401 B
build/block-library/blocks/page-list/style-rtl.css 175 B
build/block-library/blocks/page-list/style.css 175 B
build/block-library/blocks/paragraph/editor-rtl.css 174 B
build/block-library/blocks/paragraph/editor.css 174 B
build/block-library/blocks/paragraph/style-rtl.css 279 B
build/block-library/blocks/paragraph/style.css 281 B
build/block-library/blocks/post-author/style-rtl.css 175 B
build/block-library/blocks/post-author/style.css 176 B
build/block-library/blocks/post-comments-form/editor-rtl.css 96 B
build/block-library/blocks/post-comments-form/editor.css 96 B
build/block-library/blocks/post-comments-form/style-rtl.css 508 B
build/block-library/blocks/post-comments-form/style.css 508 B
build/block-library/blocks/post-date/style-rtl.css 61 B
build/block-library/blocks/post-date/style.css 61 B
build/block-library/blocks/post-excerpt/editor-rtl.css 71 B
build/block-library/blocks/post-excerpt/editor.css 71 B
build/block-library/blocks/post-excerpt/style-rtl.css 141 B
build/block-library/blocks/post-excerpt/style.css 141 B
build/block-library/blocks/post-featured-image/editor-rtl.css 588 B
build/block-library/blocks/post-featured-image/editor.css 586 B
build/block-library/blocks/post-featured-image/style-rtl.css 319 B
build/block-library/blocks/post-featured-image/style.css 319 B
build/block-library/blocks/post-navigation-link/style-rtl.css 153 B
build/block-library/blocks/post-navigation-link/style.css 153 B
build/block-library/blocks/post-template/editor-rtl.css 99 B
build/block-library/blocks/post-template/editor.css 98 B
build/block-library/blocks/post-template/style-rtl.css 314 B
build/block-library/blocks/post-template/style.css 314 B
build/block-library/blocks/post-terms/style-rtl.css 96 B
build/block-library/blocks/post-terms/style.css 96 B
build/block-library/blocks/post-time-to-read/style-rtl.css 69 B
build/block-library/blocks/post-time-to-read/style.css 69 B
build/block-library/blocks/post-title/style-rtl.css 100 B
build/block-library/blocks/post-title/style.css 100 B
build/block-library/blocks/preformatted/style-rtl.css 125 B
build/block-library/blocks/preformatted/style.css 125 B
build/block-library/blocks/pullquote/editor-rtl.css 135 B
build/block-library/blocks/pullquote/editor.css 135 B
build/block-library/blocks/pullquote/style-rtl.css 335 B
build/block-library/blocks/pullquote/style.css 335 B
build/block-library/blocks/query-pagination-numbers/editor-rtl.css 122 B
build/block-library/blocks/query-pagination-numbers/editor.css 121 B
build/block-library/blocks/query-pagination/editor-rtl.css 221 B
build/block-library/blocks/query-pagination/editor.css 211 B
build/block-library/blocks/query-pagination/style-rtl.css 302 B
build/block-library/blocks/query-pagination/style.css 299 B
build/block-library/blocks/query-title/style-rtl.css 63 B
build/block-library/blocks/query-title/style.css 63 B
build/block-library/blocks/query/editor-rtl.css 450 B
build/block-library/blocks/query/editor.css 449 B
build/block-library/blocks/quote/style-rtl.css 222 B
build/block-library/blocks/quote/style.css 222 B
build/block-library/blocks/quote/theme-rtl.css 223 B
build/block-library/blocks/quote/theme.css 226 B
build/block-library/blocks/read-more/style-rtl.css 132 B
build/block-library/blocks/read-more/style.css 132 B
build/block-library/blocks/rss/editor-rtl.css 149 B
build/block-library/blocks/rss/editor.css 149 B
build/block-library/blocks/rss/style-rtl.css 289 B
build/block-library/blocks/rss/style.css 288 B
build/block-library/blocks/search/editor-rtl.css 178 B
build/block-library/blocks/search/editor.css 178 B
build/block-library/blocks/search/theme-rtl.css 114 B
build/block-library/blocks/search/theme.css 114 B
build/block-library/blocks/search/view.min.js 631 B
build/block-library/blocks/separator/editor-rtl.css 146 B
build/block-library/blocks/separator/editor.css 146 B
build/block-library/blocks/separator/style-rtl.css 234 B
build/block-library/blocks/separator/style.css 234 B
build/block-library/blocks/separator/theme-rtl.css 194 B
build/block-library/blocks/separator/theme.css 194 B
build/block-library/blocks/shortcode/editor-rtl.css 323 B
build/block-library/blocks/shortcode/editor.css 323 B
build/block-library/blocks/site-logo/editor-rtl.css 754 B
build/block-library/blocks/site-logo/editor.css 754 B
build/block-library/blocks/site-logo/style-rtl.css 203 B
build/block-library/blocks/site-logo/style.css 203 B
build/block-library/blocks/site-tagline/editor-rtl.css 86 B
build/block-library/blocks/site-tagline/editor.css 86 B
build/block-library/blocks/site-title/editor-rtl.css 116 B
build/block-library/blocks/site-title/editor.css 116 B
build/block-library/blocks/site-title/style-rtl.css 57 B
build/block-library/blocks/site-title/style.css 57 B
build/block-library/blocks/social-link/editor-rtl.css 184 B
build/block-library/blocks/social-link/editor.css 184 B
build/block-library/blocks/social-links/editor-rtl.css 674 B
build/block-library/blocks/social-links/editor.css 673 B
build/block-library/blocks/spacer/editor-rtl.css 348 B
build/block-library/blocks/spacer/editor.css 348 B
build/block-library/blocks/spacer/style-rtl.css 48 B
build/block-library/blocks/spacer/style.css 48 B
build/block-library/blocks/table/theme-rtl.css 146 B
build/block-library/blocks/table/theme.css 146 B
build/block-library/blocks/tag-cloud/style-rtl.css 251 B
build/block-library/blocks/tag-cloud/style.css 253 B
build/block-library/blocks/template-part/editor-rtl.css 403 B
build/block-library/blocks/template-part/editor.css 403 B
build/block-library/blocks/template-part/theme-rtl.css 101 B
build/block-library/blocks/template-part/theme.css 101 B
build/block-library/blocks/term-description/style-rtl.css 111 B
build/block-library/blocks/term-description/style.css 111 B
build/block-library/blocks/text-columns/editor-rtl.css 95 B
build/block-library/blocks/text-columns/editor.css 95 B
build/block-library/blocks/text-columns/style-rtl.css 166 B
build/block-library/blocks/text-columns/style.css 166 B
build/block-library/blocks/verse/style-rtl.css 99 B
build/block-library/blocks/verse/style.css 99 B
build/block-library/blocks/video/editor-rtl.css 552 B
build/block-library/blocks/video/editor.css 555 B
build/block-library/blocks/video/style-rtl.css 185 B
build/block-library/blocks/video/style.css 185 B
build/block-library/blocks/video/theme-rtl.css 126 B
build/block-library/blocks/video/theme.css 126 B
build/block-library/classic-rtl.css 179 B
build/block-library/classic.css 179 B
build/block-library/common-rtl.css 1.1 kB
build/block-library/common.css 1.1 kB
build/block-library/editor-elements-rtl.css 75 B
build/block-library/editor-elements.css 75 B
build/block-library/elements-rtl.css 54 B
build/block-library/elements.css 54 B
build/block-library/reset-rtl.css 478 B
build/block-library/reset.css 478 B
build/block-serialization-default-parser/index.min.js 1.12 kB
build/block-serialization-spec-parser/index.min.js 2.87 kB
build/blocks/index.min.js 51.2 kB
build/commands/style-rtl.css 863 B
build/commands/style.css 857 B
build/compose/index.min.js 12.1 kB
build/core-data/index.min.js 16.8 kB
build/customize-widgets/index.min.js 12 kB
build/customize-widgets/style-rtl.css 1.46 kB
build/customize-widgets/style.css 1.45 kB
build/data-controls/index.min.js 640 B
build/data/index.min.js 8.38 kB
build/date/index.min.js 17.8 kB
build/deprecated/index.min.js 451 B
build/dom-ready/index.min.js 324 B
build/dom/index.min.js 4.64 kB
build/edit-post/classic-rtl.css 544 B
build/edit-post/classic.css 545 B
build/element/index.min.js 4.82 kB
build/escape-html/index.min.js 537 B
build/format-library/style-rtl.css 554 B
build/format-library/style.css 553 B
build/hooks/index.min.js 1.55 kB
build/html-entities/index.min.js 448 B
build/i18n/index.min.js 3.58 kB
build/interactivity/index.min.js 10.4 kB
build/is-shallow-equal/index.min.js 527 B
build/keyboard-shortcuts/index.min.js 1.64 kB
build/keycodes/index.min.js 1.87 kB
build/list-reusable-blocks/index.min.js 2.2 kB
build/list-reusable-blocks/style-rtl.css 836 B
build/list-reusable-blocks/style.css 836 B
build/media-utils/index.min.js 2.9 kB
build/notices/index.min.js 948 B
build/nux/index.min.js 1.99 kB
build/nux/style-rtl.css 735 B
build/nux/style.css 732 B
build/patterns/index.min.js 2.7 kB
build/patterns/style-rtl.css 240 B
build/patterns/style.css 240 B
build/plugins/index.min.js 1.79 kB
build/preferences-persistence/index.min.js 1.84 kB
build/preferences/index.min.js 1.24 kB
build/primitives/index.min.js 943 B
build/priority-queue/index.min.js 1.52 kB
build/react-i18n/index.min.js 615 B
build/react-refresh-entry/index.min.js 9.47 kB
build/react-refresh-runtime/index.min.js 7.31 kB
build/redux-routine/index.min.js 2.7 kB
build/reusable-blocks/style-rtl.css 243 B
build/reusable-blocks/style.css 243 B
build/router/index.min.js 1.78 kB
build/server-side-render/index.min.js 1.94 kB
build/shortcode/index.min.js 1.39 kB
build/style-engine/index.min.js 1.85 kB
build/sync/index.min.js 53.8 kB
build/token-list/index.min.js 582 B
build/url/index.min.js 3.73 kB
build/vendors/inert-polyfill.min.js 2.48 kB
build/vendors/react-dom.min.js 41.8 kB
build/vendors/react.min.js 4.02 kB
build/viewport/index.min.js 958 B
build/warning/index.min.js 268 B
build/widgets/index.min.js 7.16 kB
build/widgets/style-rtl.css 1.15 kB
build/widgets/style.css 1.16 kB
build/wordcount/index.min.js 1.02 kB

compressed-size-action

@getdave

This comment was marked as resolved.

@getdave getdave marked this pull request as ready for review August 11, 2023 12:00
@ricjcs
Copy link

ricjcs commented Aug 11, 2023

Excellent! That's the best solution.

It basically stays as it was before and additionally has advanced options.

Now yes, that solves the problem.

Thank you very much for listening to users and for the work!

@richtabor
Copy link
Member

There's potential here. The popover moving to the upper left corner is odd; not sure what's going on there. The checkbox also does not save if you edit it within that state. But if you click on the link again, it does.

CleanShot.2023-08-11.at.10.17.37.mp4

@richtabor
Copy link
Member

And when is-preview, .block-editor-link-control__setting padding should be 20px 8px 8px 0.

CleanShot 2023-08-11 at 10 23 02

@getdave
Copy link
Contributor Author

getdave commented Aug 11, 2023

That is very odd. I hadn't noticed that on my small screen. Will take a look.

@richtabor
Copy link
Member

That is very odd. I hadn't noticed that on my small screen. Will take a look.

It's actually doing it in your video—moving to the top left. But it's not as noticeable at those dimensions.

@elgato91
Copy link

Compared to the previous interface, the current one is pejorative. It is no longer clear or simple and introduces additional steps, actions, and waiting time to perform required tasks. We should thoroughly investigate the reasons behind this change. What are the benefits it brings?

Furthermore, we should also consider that while we frequently discuss the concept of "open in a new window" it is equally important to implement controls for the "no follow" attribute. This attribute holds significant importance in terms of SEO compliance, aligning with recent Google guidelines pertaining to sponsored links, affiliations, and similar aspects.

Additionally, the checkboxes appear to represent a design evolution from the previous toggle switches. Moreover, what advantages does the anchor text field in the advanced section bring?

@getdave
Copy link
Contributor Author

getdave commented Aug 14, 2023

Thanks for your questions. Usually we'd keep the feedback related to the proposal in the PR but as this relates to wider concerns I'll address some of your queries to enable you to better review this specific solution in context.

Compared to the previous interface, the current one is pejorative.

Are you referring to the UI proposed in this PR?

It is no longer clear or simple...

Can you go deeper here? It sounds a little subjective.

To me the UI itself appears to be very simple when first creating a link (witness lack of other controls). It could therefore reasonably be argued that showing additional controls here would make the interface more complex. What specifically is not clear/simple in the steps shown in this PR?

...and introduces additional steps, actions, and waiting time to perform required tasks.

There is a valid discussion to be had here. What would your proposal be? I assume it would be to show the Opens in new tab control immediately at link creation step once the user has entered a value into the URL input?

...it is equally important to implement controls for the "no follow" attribute

The work we are doing is geared towards allowing more controls such as this. However we cannot continue to expand the number of controls shown on the first step indefinitely. This is precisely the background to why we moved the Opens in new tab.

Consider that other contributors may suggest other controls which - from their perspective - are equally important to them and before we know it the UI would be incredibly complex. Therefore as contributors we have to make difficult decisions to deprioritise the majority of controls to avoid overwhelming the user.

Our current thinking is that whilst Opens in new tab could be a special case (considering how long it's been in WordPress), all other controls should be moved into the Advanced area. Whilst it might be important for one set of users, for another set this control would simply add additional complexity.

With the auto-saving of the expanded state of this control already merged, this seems like a reasonable compromise solution.

Implementing No follow will be something we may do once the UI has reached a stable state.

Additionally, the checkboxes appear to represent a design evolution from the previous toggle switches

The checkboxes are an evolution and one that was undertaken in a deliberate manner based on user feedback.

The reason checkboxes are used is because they imply a need to submit a change. We had numerous reports of confusion with the previous toggle-based interface where changes to settings would immediately cause the value to be submitted in unexpected ways. This is why we moved to a checkbox and also requiring the user to submit their changes. Obviously if we choose to pursue this PR then that distinction is less clear cut but I think it still works. Do you? If not why.

what advantages does the anchor text field in the advanced section bring?

I didn't follow this. If it's not directly related to the change in this particular PR I would recommend raising a new Issue documenting any problems you've experienced. Many thanks.

@getdave
Copy link
Contributor Author

getdave commented Aug 14, 2023

Re: incorrect placement of popover. This seems to be because on first render the popover's anchor is not the link <a> but rather then entire <p> paragraph block. When you click again the popover's anchor is updated to render to the <a> tag.

See graphic below which shows the console logging of first and then second clicks:

Screen Shot 2023-08-14 at 12 35 06

Interesting when you first create a link the popover knows how to position itself correctly relative to the selection so now looking at how we loose that information once the link is created.

@elgato91
Copy link

Thanks, Dave. Your answers are greatly appreciated. I am very sorry that I discovered the new Link UI issue only after being impacted by it because I would have gladly contributed since its inception. Obviously, the choice between switches or checkboxes works well in any case, and I think you are right to favor what gives greater stability.

Regarding the design and concept of the Link UI, I believe there is some reasoning to be done on the matter — reasoning that is not subjective but based on factual issues. The before-and-after comparisons, in my view, are between the old UI and the new UI. The old UI was well-designed because it did its job: inserting a link, possibly searching for an existing page and linking to it, and marking the link with the two most important tags: opening in new tab and adding the nofollow tag.

While "open in new tab" is obviously used and abused even by amateur users, the latter is extremely important for anyone who understands the rudiments of SEO. Publishers, bloggers, marketers, and so on need (and it is a fundamental requirement of the Google Guidelines) to be able to add the nofollow tag very frequently — whenever there is a commercial relationship (affiliation or other) and whenever they are linking to a resource they don't fully trust. In the professional context, most links are often "nofollow", and it is also the most time-sensitive context since any additional action (click) requires more effort (and time) from the content creator.

This is not a subjective matter; a simple web search or reading the Google guidelines would reveal it. The old UI also allowed the sponsored tag to be included, strictly following the new Google guidelines. However, it currently appears that nofollow and sponsored can work equally well for Google.

To me the UI itself appears to be very simple when first creating a link (witness lack of other controls). It could therefore reasonably be argued that showing additional controls here would make the interface more complex. What specifically is not clear/simple in the steps shown in this PR?

Coming to that, I think that's the point. A minimal interface does not equate to an easier to use interface. You're just moving vital controls elsewhere, sweeping the dust under the carpet. This, while aesthetically it looks like a simplification, has actually significantly increased the complexity of the interface.

Consider that other contributors may suggest other controls which - from their perspective - are equally important to them and before we know it the UI would be incredibly complex. Therefore as contributors we have to make difficult decisions to deprioritise the majority of controls to avoid overwhelming the user.

There's no making up to say what's the best way to handle this, the old UI did it brilliantly! The reasoning made for the design of the old UI is still valid: inserting links and immediately choosing to mark them with "open in a new tab" and "no follow" are the essential controls. It's something that has always existed in WP.

@getdave
Copy link
Contributor Author

getdave commented Aug 14, 2023

@elgato91 Thanks for your thoughts. To keep this PR's comments on track I'm going to recommend you/we re-post your comment on the Issue which I found which tracks the "No follow" requirement.

In terms of UI design I think this discussion can continue on #50891.

Thanks again for your feedback and contributing to the project.

@getdave getdave mentioned this pull request Aug 14, 2023
13 tasks
@getdave
Copy link
Contributor Author

getdave commented Aug 14, 2023

My new thinking re: this bug is that we loose the "selection" in the paragraph block when a link is submitted. That means the virtual anchor for the popover doesn't know where to "anchor" itself to.

This is in contrast to:

  • original link creation - you select the text and then adding a link causes the popover to be anchored to that selection range
  • the link preview - this is anchored to the <a> tag once that is available.

That's why we end up with a popover in the top left corner. I don't yet know how to fix that. Ideally when the link is submitted the <a> should already be in the document but it appears it isn't added until it's too late...

Update: 15th August

More deep diving here has revealed that the useLayoutEffect of useAnchor which is pretty much what determines where the popover is anchored is called once when the link creation popover UI is shown but is not called again when the link is submitted.

This means that the calculation to find the new anchor point based on the new <a> (the link format) within the rich text does not happen. This means that because we no longer have a text selection in the richtext, the popover is positioned relative to the entire rich text paragraph.

One "fix" I found is to call callback() immediately within the attach function:

function attach() {
ownerDocument.addEventListener( 'selectionchange', callback );
}

The strange thing is why callback() doesn't get called anyway by the event listener which is added by the attach function:

ownerDocument.addEventListener( 'selectionchange', callback );

Given that is based on the selectionchange event, you would assume that when the focus moves from the richtext to the popover the selection change event would trigger.

How this doesn't happen because the event listener is deregistered by the detach event which is called as soon as the richtext looses focus and moves to the LinkControl.

function detach() {
ownerDocument.removeEventListener( 'selectionchange', callback );
}

@richtabor richtabor merged commit 3d01a3f into trunk Aug 15, 2023
@richtabor richtabor deleted the try/add-opens-new-tab-to-link-preview branch August 15, 2023 14:43
@github-actions github-actions bot added this to the Gutenberg 16.5 milestone Aug 15, 2023
@TapiwaZvaks
Copy link

For me personally, the best option would actually be to default all links to open in a new tab.

I agree with almost everything you said, but on this point I disagree.

I don't think the default should be to open links in a new tab, however that process should be made easier. In my experience, most people prefer setting to open internal links in the same tab, and external links in a new tab. This for me is also what makes the most sense, opening external links in a new tab so that the visitor does not leave our website.

When making a website/posts we must think about the user and not just search engines/robots, opening all the links in a new tab does not seem to provide the best browsing experience in most cases.

Maybe someone will come up with a plugin or something.

There are plugins that allow you to configure this.

True enough. I have seen a lot of people stating this very same point, that external links should be opened in a new tab while internal links should be opened on the same page. I do disagree, however, that opening a link in a new tab necessarily makes for a bad user experience. I believe the opposite. If I come across a link while reading a post, and if I click on it, it doesn't mean I want to close the page that I was reading. It just means I want to reference some more information related to the subject, and I can do it in another tab.

@TapiwaZvaks
Copy link

One (supposed) signal of how good a web page is for search engines is the amount of time that visitors spent on that page. Therein lies the usefulness of opening a post in a new tab.

I think this is also subjective; there are arguments for and against new tab, like this one (against) from John Mueller, where John states "links are links". I tend to agree,

For me personally, the best option would actually be to default all links to open in a new tab. Maybe someone will come up with a plugin or something.

Maybe, but this would an lackluster experience for visitors. If every link you clicked on opened a new tab, you would end up with tens of tabs in your browser window, and you can't progress through them any longer.

Not saying there's a case when you do want them to open in a new tab (there is—hence the control within WordPress), but it likely shouldn't be the default experience for your readers. Anyhow, always conduct the research on your own accord and do what's best for your use case.

Again, thanks for the feedback. Hoping to get this merged, stepping closer to an ideal link editing flow for everyone. 🖤

Oh yes, John Mueller. He says one thing today and another tomorrow. In any case, you are right. I did mention that my response was mostly conjecture, but it was reasonable conjecture. Otherwise why would they have time spend on a page as a metric in Analytics? Why have bounce rate? If someone leaves immediately after clicking on an external link, they are essentially bouncing. In any case, don't know if those metrics are still there in GA4 but they were there in UA.

Maybe, but this would an lackluster experience for visitors. If every link you clicked on opened a new tab, you would end up with tens of tabs in your browser window, and you can't progress through them any longer.

I don't agree on this one. I believe opening in a new page is the most intuitive and usefully logical progression. If I am reading a how to article, do I want to have it closed because I have clicked on a link that's referenced in the article?

I agree it shouldn't be default, but it would be great if there was an option somewhere as an example where I could be given three options 1)open all external links in a new tab or 2) open all internal links in a new tab. I could then choose to either select both or 1. Or none.

@ricjcs
Copy link

ricjcs commented Aug 15, 2023

I believe opening in a new page is the most intuitive and usefully logical progression. If I am reading a how to article, do I want to have it closed because I have clicked on a link that's referenced in the article?

I understand your point of view and I've thought that way too. However, the website visitor can always choose to open a link in a new tab if that is their wish, so in my opinion it is more useful to force only external links to open in a new tab so that the person doesn't leave our website.

But if it's better one way or another, for this case I even think it's a little irrelevant. What matters is that the website creator can easily choose how he wants to define the links without having to do a lot of clicking.

This PR solves the problem that came up with the new WP. I'm happy about it.

I agree it shouldn't be default, but it would be great if there was an option somewhere as an example where I could be given three options 1)open all external links in a new tab or 2) open all internal links in a new tab. I could then choose to either select both or 1. Or none.

A few years ago I opened a ticket with this suggestion on "Trac". For reasons related to accessibility it was closed.

@TapiwaZvaks
Copy link

We are in agreement. The choice should be with the writer. And it needs to be an easy one. In any case, this has been an interesting interaction. Fruitful, at least for me. Yes, this link-in-new-tab issue will mostly be solved, but what about the "this is a sponsored link" option? All these things were easily accessible before the latest WordPress. I don't use it personally but @elgato91 thinks its in common use. They are all being replaced by this link preview. The link preview has no functional use. As mentioned, I spend my days typing posts. Each time I have to click six times I am drawn here and I have to chant in my head while typing responses "this is an open source project, stay polite, these guys are doing you a favor remain polite, these guys...." Lol. I am only half kidding

@elgato91
Copy link

@TapiwaZvaks I would like to provide a clearer explanation of my consideration on nofollow, for greater clarity in any future considerations.

Using "nofollow" to mark affiliate links is just one of the reasons (albeit a common one among professional bloggers) for using the nofollow tag. Actually, it should be used whenever you are linking to a source whose reputation you are not entirely certain about. This is because it offers a "safer" approach from an SEO perspective. Although today there are more granular controls not natively present in WP but easily addable (as those familiar with prevalent SEO plugins are aware, such as "rel=sponsored"), just a judicious use of "nofollow" suffices, according to the most recent Google guidelines.

Returning to the core issue, these controls are commonly added by major SEO plugins, so I wouldn't fret if they aren't in core. The real concern lies in the user interface's dynamics, which have shifted and, in doing so, have deviated from their main and original purpose. Consequently, it demands more effort to utilize these controls.

@ricjcs
Copy link

ricjcs commented Aug 15, 2023

I consider an option "nofollow" important and should be implemented. It's best not to depend on plugins for basic things. But in this case, although important, it doesn't seem to me to be massively used, so I wouldn't be shocked if it stayed in the "Advanced" drawer.

@elgato91
Copy link

@ricjcs Try checking the code of some serious site, you might discover that "nofollow" is utilized more frequently than you realize... :)

The non-use only concerns very amateur users and those who do not respect the SEO guidelines. From newspapers to professional blogs, nofollow usage is massive. In terms of importance and impact on professional user experience, I would consider these two controls to be equally important, carrying the same weight.

@ellatrix
Copy link
Member

This restores a change made in 6.3, right? Should we consider backporting this to a minor release? I'll add the label so it receives the attention to be discussed (not a confirmation that it will be backported).

@ellatrix ellatrix added the Backport to WP Minor Release Pull request that needs to be backported to a WordPress minor release label Aug 16, 2023
@EW0JY
Copy link

EW0JY commented Aug 25, 2023

Hi @getdave ,

I wanted to let you know about some major issues and bugs that I've noticed with this feature.

I've updated the Gutenberg plugin to 16.5 just now to test the new link preview feature and recorded this screen capture for you:

bug.-.link.preview.feature.not.working.as.expected.mp4

As you can see, it is not working as expected at all and has the following issues:

  1. the checkbox is non-responsive when clicking it.
  2. worse, after clicking away from the preview, each already existing link gets actually removed.
  3. after reinserting and finally managing to get the link to stay and the checkbox to be ticked, it's impossible to un-check it.

I've tested this many times and across different browsers and it's always the same.

I am not sure whether you've already been made aware of these issues and bugs. I just wanted to take the time to provide feedback anyway.

Also, I'm not familiar with Github and so I don't know if this is the correct place to provide this feedback - apologies if it isn't.
In that case, feel free to move it to the correct place.

Thank you!

@getdave
Copy link
Contributor Author

getdave commented Aug 25, 2023

@EW0JY I have tested again with a fresh environment using WP 6.3 and Gutenberg 16.5.0 and I cannot replicate that error.

@EW0JY
Copy link

EW0JY commented Aug 25, 2023

@getdave Thanks for your quick reply!

Well, I have no clue as to why it doesn't happen for you.

As you can see in that video, the error is definitely occurring for me.

I've tested it in incognito in Firefox as well as Chrome.

@getdave
Copy link
Contributor Author

getdave commented Aug 25, 2023

@EW0JY To help contributors I would recommend:

  • Posting details of your testing environment including Operating System, Browser, WP version, Gutenberg Plugin version.
  • Ensuring that you are running a clean install without Plugins and using a default Theme.

If the bug still manifests then we'll be a step closer.

I appreciate that's additional overhead but it's really the only way to narrow this down.

If you can still replicate then it will also help if this is posted as a new Issue as that will cause it to be picked up by Triage teams and thus get more testers involved 🙏

Thanks for your help here.

@TapiwaZvaks
Copy link

I was able to "almost" replicate the issue. Things work but they are a bit iffy. When I insert a link using ctrl + v, I am able to click on the checkbox and everything works fine. However, when I manually type in a link as @EW0JY did in the screenshot, things become a bit patchy. Clicking on the checkbox does nothing. However, when I click on the side and then click on the link one more time, I can see that the box is already checked.

@EW0JY
Copy link

EW0JY commented Aug 25, 2023

@getdave I understand, so I went through all the steps you recommended and did the following:

I installed a fresh WP version without any plugins and could confirm that the link feature works as expected.

So I cloned my production site and deactivated all plugins, after which the problem also disappeared.

Then I reactivated each plugin one by one until the problem reoccurred.

I can now definitely confirm that the problem is this:

The new link feature is incompatible with the YOAST SEO plugin.

That is the only plugin I have to deactivate to make the feature work as expected.

I also downgraded to a previous version of the yoast plugin, but even then it didn't work.

Since it's such a popular plugin and not feasible for me to stop using it, I hope that is not what you are going to suggest as a "solution".

Also, due to its popularity, a ton of people will also be affected by it.

Can you please install the yoast plugin, replicate the issue and make the link feature compatible with yoast?

Oh, my environment is Windows 10 home, latest Firefox and Chrome versions, WP 6.3 with the latest Twenty-Twentytwo theme and Gutenberg Plugin 16.5.

I hope that helps, thank you!

P.S.:

I've tested a bit more and was able to narrow it down to this toggle within the yoast plugin settings:

conflicting yoast setting

When I disable that toggle, your link preview feature works as expected.

In my specific case, I think it's a viable workaround as I don't need those "additional settings" enabled - for other people, this may be different.

I hope this helps,
Best wishes!

@ricjcs
Copy link

ricjcs commented Aug 25, 2023

I was able to reproduce the same issue. Without the Yoast plugin it works perfectly, with Yoast activated the reported problem happens, I also tested it with Rank Math, the same thing happens.

@getdave getdave removed the Backport to WP Minor Release Pull request that needs to be backported to a WordPress minor release label Aug 25, 2023
@draganescu
Copy link
Contributor

draganescu commented Aug 25, 2023

#53949 is fixing the bug described by @EW0JY and reproduced by @ricjcs . Thank you both for the great steps.

@EW0JY
Copy link

EW0JY commented Aug 25, 2023

@draganescu I can't seem to find #81960 when searching for it on Github - could you please link to it?

Thanks!

@draganescu
Copy link
Contributor

It's #53949 @EW0JY 🤕

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Link Editing Link components (LinkControl, URLInput) and integrations (RichText link formatting) Needs User Documentation Needs new user documentation [Type] Enhancement A suggestion for improvement.
Projects
Development

Successfully merging this pull request may close these issues.

9 participants