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

Jump to bottom

Replace SEO settings nofollow toggle with rel text widget #23682

Merged
merged 5 commits into from
Jul 16, 2020
Merged

Replace SEO settings nofollow toggle with rel text widget #23682

merged 5 commits into from
Jul 16, 2020

Conversation

adamziel
Copy link
Contributor

@adamziel adamziel commented Jul 3, 2020 •
edited
Loading

Description

Fixes #23153 by removing SEO settings footgun from the navigation link block

How has this been tested?

  1. Don't apply this PR just yet
  2. Go to post editor
  3. Add a navigation block with navigation link inside
  4. Select navigation link
  5. Enable the "Add nofollow to link" toggle
  6. Create another navigation link with that toggle unselected
  7. Save the post
  8. Confirm that on the actual website
  9. Save the post check the website, confirm the link is there, confirm that ref="nofollow" is NOT there (yup, it's actually broken :D)
  10. Apply this PR
  11. Refresh the website, confirm that the link now has rel="nofollow"
  12. Go to the editor, confirm the inspector allows you to type in an arbitrary rel.
  13. Confirm that particular menu item already has the inspector field pre-filled with nofollow.
  14. Confirm you're able to set rel to an arbitrary value and that it's reflected on subsequent refresh of both the editor and the actual website.

Zrzut ekranu 2020-07-7 o 16 24 41

Zrzut ekranu 2020-07-7 o 16 23 33

Types of changes

Non-breaking change

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.

@adamziel adamziel self-assigned this Jul 3, 2020
@adamziel adamziel added the [Block] Navigation Affects the Navigation Block label Jul 3, 2020
@adamziel adamziel marked this pull request as ready for review July 3, 2020 13:53
@github-actions
Copy link

github-actions bot commented Jul 3, 2020 •
edited
Loading

Size Change: +10.5 kB (0%)

Total Size: 1.14 MB

Filename Size Change
build/annotations/index.js 3.67 kB +45 B (1%)
build/api-fetch/index.js 3.39 kB -8 B (0%)
build/autop/index.js 2.82 kB +1 B
build/block-directory/index.js 7.67 kB +513 B (6%) 🔍
build/block-editor/index.js 115 kB +5.92 kB (5%) 🔍
build/block-editor/style-rtl.css 10.8 kB +93 B (0%)
build/block-editor/style.css 10.8 kB +90 B (0%)
build/block-library/editor-rtl.css 7.58 kB -44 B (0%)
build/block-library/editor.css 7.57 kB -46 B (0%)
build/block-library/index.js 132 kB +1.81 kB (1%)
build/block-library/style-rtl.css 7.77 kB -16 B (0%)
build/block-library/style.css 7.77 kB -12 B (0%)
build/block-serialization-default-parser/index.js 1.88 kB -3 B (0%)
build/block-serialization-spec-parser/index.js 3.1 kB -5 B (0%)
build/blocks/index.js 48.2 kB +90 B (0%)
build/components/index.js 199 kB +314 B (0%)
build/components/style-rtl.css 15.9 kB +32 B (0%)
build/components/style.css 15.8 kB +28 B (0%)
build/compose/index.js 9.67 kB +22 B (0%)
build/core-data/index.js 11.5 kB +21 B (0%)
build/data/index.js 8.46 kB +12 B (0%)
build/date/index.js 5.38 kB -86 B (1%)
build/dom/index.js 3.23 kB +38 B (1%)
build/edit-navigation/index.js 10.8 kB +859 B (7%) 🔍
build/edit-navigation/style-rtl.css 1.08 kB +59 B (5%) 🔍
build/edit-navigation/style.css 1.08 kB +59 B (5%) 🔍
build/edit-post/index.js 304 kB +379 B (0%)
build/edit-post/style-rtl.css 5.6 kB +30 B (0%)
build/edit-post/style.css 5.6 kB +30 B (0%)
build/edit-site/index.js 16.8 kB +116 B (0%)
build/edit-site/style-rtl.css 3.04 kB +11 B (0%)
build/edit-site/style.css 3.04 kB +12 B (0%)
build/edit-widgets/index.js 9.35 kB +22 B (0%)
build/editor/index.js 45.1 kB +261 B (0%)
build/editor/style-rtl.css 3.78 kB +4 B (0%)
build/editor/style.css 3.78 kB +5 B (0%)
build/element/index.js 4.65 kB -2 B (0%)
build/format-library/index.js 7.72 kB -10 B (0%)
build/hooks/index.js 2.13 kB +1 B
build/is-shallow-equal/index.js 709 B -1 B
build/keyboard-shortcuts/index.js 2.51 kB +1 B
build/list-reusable-blocks/index.js 3.12 kB +2 B (0%)
build/media-utils/index.js 5.32 kB +21 B (0%)
build/notices/index.js 1.79 kB +1 B
build/nux/index.js 3.4 kB -1 B
build/plugins/index.js 2.56 kB +1 B
build/primitives/index.js 1.4 kB -98 B (6%) ✅
build/priority-queue/index.js 789 B +1 B
build/redux-routine/index.js 2.85 kB -4 B (0%)
build/rich-text/index.js 13.9 kB -108 B (0%)
build/server-side-render/index.js 2.71 kB +37 B (1%)
build/shortcode/index.js 1.7 kB +1 B
build/token-list/index.js 1.27 kB -3 B (0%)
build/url/index.js 4.06 kB -4 B (0%)
build/viewport/index.js 1.85 kB -1 B
build/warning/index.js 1.13 kB -4 B (0%)
build/wordcount/index.js 1.17 kB -1 B
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/style-rtl.css 944 B 0 B
build/block-directory/style.css 945 B 0 B
build/block-library/theme-rtl.css 728 B 0 B
build/block-library/theme.css 729 B 0 B
build/data-controls/index.js 1.29 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 569 B 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/escape-html/index.js 733 B 0 B
build/format-library/style-rtl.css 547 B 0 B
build/format-library/style.css 548 B 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.56 kB 0 B
build/keycodes/index.js 1.94 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/nux/style-rtl.css 671 B 0 B
build/nux/style.css 668 B 0 B

compressed-size-action

@noisysocks
Copy link
Member

Instead of removing the SEO settings panel let's replace it with the Button block's Link settings panel. See #23153 (comment).

@adamziel adamziel changed the title Remove SEO settings from the navigation link block Replace SEO settings nofollow toggle with rel text widget Jul 7, 2020
@adamziel
Copy link
Contributor Author

adamziel commented Jul 7, 2020

@noisysocks all good notes! I updated the PR to reflect your proposal.

@adamziel adamziel requested review from nerrad and ntwb as code owners July 7, 2020 14:43
@adamziel adamziel mentioned this pull request Jul 7, 2020
Closed
@adamziel
Copy link
Contributor Author

adamziel commented Jul 7, 2020

I still need to fix the PHP unit test here, but otherwise it's good for reviews.

@draganescu
Copy link
Contributor

I tested this and found a problem:

  1. on master create a navigation block with two links
  2. on on of the links set BOTH open in new tab and the not working no follow in seo settings
  3. save and publish
  4. change to this PR
  5. refresh the front end -> the nofollow rel is there
  6. edit the post -> notice the link is missing the label!

@adamziel
Copy link
Contributor Author

@draganescu really good catch and a simple mistake on my end - it's fixed now, thank you!

draganescu
draganescu approved these changes Jul 16, 2020
View reviewed changes
Copy link
Contributor

@draganescu draganescu left a comment

Choose a reason for hiding this comment

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

Tested and works as described.

@draganescu draganescu dismissed noisysocks’s stale review July 16, 2020 14:00

All changes have been implemented - I hope

@draganescu draganescu merged commit 0ac9074 into master Jul 16, 2020
@draganescu draganescu deleted the update/remove-seo-settings-from-navigation-link-block branch July 16, 2020 14:00
@github-actions github-actions bot added this to the Gutenberg 8.6 milestone Jul 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@draganescu draganescu draganescu approved these changes

@noisysocks noisysocks noisysocks left review comments

@tellthemachines tellthemachines Awaiting requested review from tellthemachines

@ajitbohra ajitbohra Awaiting requested review from ajitbohra

@Soean Soean Awaiting requested review from Soean

@talldan talldan Awaiting requested review from talldan

@nerrad nerrad Awaiting requested review from nerrad

@ntwb ntwb Awaiting requested review from ntwb

Assignees

@adamziel adamziel

Labels
[Block] Navigation Affects the Navigation Block
Projects
None yet
Milestone
Gutenberg 8.6
Development

Successfully merging this pull request may close these issues.

Remove the SEO settings block from the nav block
3 participants
@adamziel @noisysocks @draganescu