Skip to content

Adds 'nofollow' setting to Button block#54110

Merged
getdave merged 9 commits intoWordPress:trunkfrom
bangank36:add/nofollow-for-button-link
Sep 21, 2023
Merged

Adds 'nofollow' setting to Button block#54110
getdave merged 9 commits intoWordPress:trunkfrom
bangank36:add/nofollow-for-button-link

Conversation

@bangank36
Copy link
Copy Markdown
Contributor

@bangank36 bangank36 commented Sep 1, 2023 •
edited
Loading

What?

Add the ability to toggle the nofollow for Button's link.

Partial implementation of #54091
Prior art for Rich text link #53945

Why?

Check #53945 (comment)

How?

  • Place the nofollow option in the Advanced settings area
  • Update the linkValue for persisting the Advanced settings state

Testing Instructions

  • Open Post Editor
  • Add new Button block
  • Add a link to the Button
  • Click Edit link
  • Toggle the opensInNewTab and nofollow
  • Check and confirm whether the rel and target attributes has been inserted correctly
opensInNewWindow nofollow Generated
✅ ❌ target="_blank" rel="noreferrer noopener"
❌ ✅ target="" rel="nofollow"
✅ ✅ target="_blank" rel="noreferrer noopener nofollow"
❌ ❌ target="" rel=""

Testings

  • Verify and update these e2e tests
    npm run test:e2e:playwright -- test/e2e/specs/editor/blocks/links.spec.js
    npm run test:e2e:playwright -- test/e2e/specs/editor/blocks/buttons.spec.js

  • Insert new unit testing for updateLinkAttributes
    npm run test:unit -- packages/block-library/src/button

Testing Instructions for Keyboard

Screenshots or screencast

Kapture.2023-09-01.at.19.39.59.mp4

cc: @getdave

bangank36
bangank36 commented Sep 1, 2023
View reviewed changes
@getdave getdave added [Block] Buttons Affects the Buttons Block [Type] Feature New feature to highlight in changelogs. [Feature] Link Editing Link components (LinkControl, URLInput) and integrations (RichText link formatting) labels Sep 1, 2023
@bangank36 bangank36 marked this pull request as ready for review September 2, 2023 01:50
@bangank36 bangank36 force-pushed the add/nofollow-for-button-link branch from f22ca0c to 3137c93 Compare September 2, 2023 06:12
alexstine
alexstine reviewed Sep 3, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@alexstine alexstine left a comment

Choose a reason for hiding this comment

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

One note below.

@getdave getdave mentioned this pull request Sep 5, 2023
Closed
@scruffian
Copy link
Copy Markdown
Contributor

scruffian commented Sep 7, 2023

This is looking good, just needs a rebase I think.

getdave

This comment was marked as outdated.

Sign in to view

getdave
getdave previously requested changes Sep 7, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@getdave getdave left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I'm going to block this PR because I"m not confident that we should be adding to the API of this component.

Let's discuss this further and get a wider range of opinions before we proceed.

Thank you 🙇

getdave
getdave reviewed Sep 7, 2023
View reviewed changes
getdave
getdave reviewed Sep 7, 2023
View reviewed changes
getdave
getdave reviewed Sep 7, 2023
View reviewed changes
@bangank36 bangank36 force-pushed the add/nofollow-for-button-link branch from 8ae1504 to 08d44c6 Compare September 8, 2023 02:58
@bangank36 bangank36 requested a review from getdave September 8, 2023 02:59
getdave
getdave reviewed Sep 8, 2023
View reviewed changes
getdave
getdave reviewed Sep 8, 2023
View reviewed changes
getdave
getdave reviewed Sep 8, 2023
View reviewed changes
@bangank36 bangank36 requested a review from getdave September 13, 2023 00:01
getdave
getdave reviewed Sep 13, 2023
View reviewed changes
getdave
getdave reviewed Sep 13, 2023
View reviewed changes
getdave
getdave reviewed Sep 13, 2023
View reviewed changes
@bangank36 bangank36 force-pushed the add/nofollow-for-button-link branch from 2626796 to dbe9b5f Compare September 14, 2023 03:38
@bangank36
Copy link
Copy Markdown
Contributor Author

bangank36 commented Sep 20, 2023

Hi @getdave, just a friendly nudge to request your review of the latest changes on my PR when you have a moment. Thanks!

getdave
getdave reviewed Sep 20, 2023
View reviewed changes
getdave
getdave reviewed Sep 20, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@getdave getdave left a comment

Choose a reason for hiding this comment

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

This is very close. Thank you for your patience.

I think upon reflection some unit tests would help add confidence here. They will also lay a solid foundation for a later refactoring.

@bangank36 bangank36 force-pushed the add/nofollow-for-button-link branch from 6df0f71 to 7895ebd Compare September 21, 2023 03:56
@bangank36 bangank36 requested a review from getdave September 21, 2023 04:07
getdave
getdave reviewed Sep 21, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@getdave getdave left a comment

Choose a reason for hiding this comment

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

Great work here. A few more tweaks and we should be in a good place.

@bangank36 bangank36 requested a review from getdave September 21, 2023 09:26
getdave
getdave reviewed Sep 21, 2023
View reviewed changes
getdave
getdave approved these changes Sep 21, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@getdave getdave left a comment

Choose a reason for hiding this comment

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

Thank you for all the work on this.

LGTM 🚀

@getdave
Copy link
Copy Markdown
Contributor

getdave commented Sep 21, 2023

Note to WP 6.4 leads. I don't believe we need to backport this PR. Happy to be advised otherwise if folks find it to be sufficiently "blessed".

@getdave getdave merged commit 191fd0f into WordPress:trunk Sep 21, 2023
@github-actions github-actions bot added this to the Gutenberg 16.8 milestone Sep 21, 2023
@bangank36 bangank36 mentioned this pull request Sep 21, 2023
Open
This was referenced Sep 25, 2023
Merged
Draft
@skorasaurus skorasaurus mentioned this pull request Jun 11, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@getdave getdave getdave approved these changes

@alexstine alexstine alexstine left review comments

@kevin940726 kevin940726 Awaiting requested review from kevin940726

@Mamaduka Mamaduka Awaiting requested review from Mamaduka

@ajitbohra ajitbohra Awaiting requested review from ajitbohra ajitbohra is a code owner

@ellatrix ellatrix Awaiting requested review from ellatrix

@fluiddot fluiddot Awaiting requested review from fluiddot

@dcalhoun dcalhoun Awaiting requested review from dcalhoun

@scruffian scruffian Awaiting requested review from scruffian

Assignees

@bangank36 bangank36

Labels

[Block] Buttons Affects the Buttons Block [Feature] Link Editing Link components (LinkControl, URLInput) and integrations (RichText link formatting) [Type] Feature New feature to highlight in changelogs.

Projects

None yet

Milestone

Gutenberg 16.8

Development

Successfully merging this pull request may close these issues.

4 participants

@bangank36 @scruffian @getdave @alexstine