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

Settings: introduce toggle for Site Verification #12130

Merged
merged 2 commits into from
Apr 25, 2019
Merged

Settings: introduce toggle for Site Verification #12130

merged 2 commits into from
Apr 25, 2019

Conversation

eliorivero
Copy link
Contributor

@eliorivero eliorivero commented Apr 23, 2019 •
edited
Loading

Fixes #12123

verification

Changes proposed in this Pull Request:

  • remove banner for Site verification
  • introduce searchable toggle that activates/deactivates Site verification
  • adds new prop disabled to GoogleVerificationServiceComponent
  • when Site verification is disabled, all fields and buttons are disabled

Testing instructions:

  • Go to Jetpack settings > Traffic
  • You should see the settings card
  • Toggle labeled "Activate verification tools" should work properly
  • search for the feature, you should find it

Captura de Pantalla 2019-04-23 a la(s) 15 53 47

Proposed changelog entry for your changes:

  • Part of module prioritization

@eliorivero eliorivero added [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. Admin Page React-powered dashboard under the Jetpack menu labels Apr 23, 2019
@eliorivero eliorivero added this to the 7.3 milestone Apr 23, 2019
@eliorivero eliorivero requested a review from a team April 23, 2019 18:58
@eliorivero eliorivero self-assigned this Apr 23, 2019
@jetpackbot
Copy link
Collaborator

jetpackbot commented Apr 23, 2019 •
edited
Loading

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Scheduled Jetpack release: May 7, 2019.
Scheduled code freeze: April 30, 2019

Generated by 🚫 dangerJS against ec356bf

jeherve
jeherve reviewed Apr 23, 2019
View reviewed changes
Copy link
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

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

It works well. I only have one minor comment.

_inc/client/traffic/verification-services.jsx Outdated Show resolved Hide resolved
@kraftbj kraftbj added [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Apr 23, 2019
@MichaelArestad
Copy link
Contributor

This is great. I think the label text is about all that could use a minor change. Either Jeremy's or my suggestion will work.

@eliorivero eliorivero added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. and removed [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! labels Apr 24, 2019
@jeherve jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Apr 25, 2019
jeherve
jeherve approved these changes Apr 25, 2019
View reviewed changes
Copy link
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

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

This works well for me now. 👍

oskosk
oskosk approved these changes Apr 25, 2019
View reviewed changes
Copy link
Contributor

@oskosk oskosk left a comment

Choose a reason for hiding this comment

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

LGTM!

@oskosk oskosk merged commit e77d969 into master Apr 25, 2019
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Apr 25, 2019
@oskosk oskosk deleted the add/settings-verification branch April 25, 2019 09:06
kraftbj added a commit that referenced this pull request Apr 26, 2019
@kraftbj
CL: #12130
3e097df
kraftbj added a commit that referenced this pull request Apr 30, 2019
@kraftbj
CL: #12130
6fc1540
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jeherve jeherve jeherve approved these changes

@oskosk oskosk oskosk approved these changes

@MichaelArestad MichaelArestad Awaiting requested review from MichaelArestad

@crunnells crunnells Awaiting requested review from crunnells

Assignees

@eliorivero eliorivero

Labels
Admin Page React-powered dashboard under the Jetpack menu [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it
Projects
None yet
Milestone
7.3
Development

Successfully merging this pull request may close these issues.

Add toggle to enable/disable site verification
7 participants
@eliorivero @jetpackbot @MichaelArestad @jeherve @oskosk @kraftbj @matticbot