Skip to content

Update Security section links on Jetpack Dashboard#10846

Merged
joanrho merged 2 commits intomasterfrom
update/jetpack-dashboard-security
Dec 6, 2018
Merged

Update Security section links on Jetpack Dashboard#10846
joanrho merged 2 commits intomasterfrom
update/jetpack-dashboard-security

Conversation

@joanrho
Copy link
Copy Markdown
Contributor

@joanrho joanrho commented Dec 6, 2018

Fixes #10687 and #10693

Changes proposed in this Pull Request:

  1. Change the wording of the "Manage security on WordPress.com" text link to "Manage security settings"
  2. Update destination of the above text link from https://wordpress.com/settings/security/SITE_URL (Calypso Settings) to SITE_URL/wp-admin/admin.php?page=jetpack#security (wp-admin Jetpack Settings)
  3. Remove gear icon.

BEFORE
before

AFTER
after

Testing instructions:

  • Check out this branch
  • Run yarn build
  • Visit the Jetpack dashboard in wp-admin on your Jetpack Sandbox site (or whatever site you're testing on)
  • Ensure the changes reflect the above "After" mockup and that the text link "Manage security settings" links to the wp-admin Jetpack Security settings page.

Proposed changelog entry for your changes:

  • Clarify and unify Security settings links on Jetpack Dashboard

@joanrho joanrho requested a review from a team December 6, 2018 02:47
@joanrho joanrho self-assigned this Dec 6, 2018
@joanrho joanrho added Enhancement Changes to an existing feature — removing, adding, or changing parts of it Admin Page React-powered dashboard under the Jetpack menu [Pri] Normal labels Dec 6, 2018
@jetpackbot
Copy link
Copy Markdown
Collaborator

jetpackbot commented Dec 6, 2018 •
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: January 10, 2019.
Scheduled code freeze: January 3, 2019

Generated by 🚫 dangerJS

@joanrho joanrho added the [Status] Needs Review This PR is ready for review. label Dec 6, 2018
@joanrho joanrho added this to the 6.9 milestone Dec 6, 2018
@joanrho
Copy link
Copy Markdown
Contributor Author

joanrho commented Dec 6, 2018

Note to @Automattic/jetpack-crew when you're testing this — I'll need your help fixing the bug where the /wp-admin/admin.php?page=jetpack#security link jumps to the middle part of the page instead of just the top of the page.

@joanrho joanrho requested a review from jeffgolenski December 6, 2018 02:57
@dereksmart
Copy link
Copy Markdown
Contributor

dereksmart commented Dec 6, 2018 •
edited
Loading

@joanrho It looks there are tests that expect these to be there, which is why the PR is failing currently. You're going to need to remove these two tests for the PR to pass: https://github.com/Automattic/jetpack/blob/master/_inc/client/components/dash-section-header/test/component.js#L53-L60

You can run that test before you commit with the command yarn test-adminpage.

I'll need your help fixing the bug where the /wp-admin/admin.php?page=jetpack#security link jumps to the middle part of the page instead of just the top of the page.

I think this issue has come up before. If there isn't one already logged, can you please create a new one in the repo and we can take care of it in a separate PR?

@joanrho
Copy link
Copy Markdown
Contributor Author

joanrho commented Dec 6, 2018

Thanks @dereksmart! Removed those tests.

I think this issue has come up before. If there isn't one already logged, can you please create a new one in the repo and we can take care of it in a separate PR?

Yep, it has, but the issue referenced the gear Gridicon, which we are now removing. Will open a fresh issue that we can address once we merge this one.

@joanrho joanrho mentioned this pull request Dec 6, 2018
Closed
@jeherve jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Dec 6, 2018
jeherve
jeherve approved these changes Dec 6, 2018
View reviewed changes
Copy link
Copy Markdown
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 for me, although the jump to the middle of the page is confusing, as you mentioned.

jeffgolenski
jeffgolenski approved these changes Dec 6, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@jeffgolenski jeffgolenski 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 great. :shipit:

@joanrho joanrho merged commit c824fae into master Dec 6, 2018
@ghost ghost removed the [Status] Ready to Merge Go ahead, you can push that green button! label Dec 6, 2018
@jeherve jeherve deleted the update/jetpack-dashboard-security branch December 7, 2018 10:08
jeherve added a commit that referenced this pull request Dec 19, 2018
@jeherve
Changelog: add #10846
f58923f
jeherve added a commit that referenced this pull request Jan 3, 2019
@jeherve
Changelog: add #10846
fd34641
jeherve added a commit that referenced this pull request Jan 3, 2019
@jeherve
Jetpack 6.9: Changelog and testing list (#10722)
8d91603 
* Add first version of the Changelog and testing list for 6.9

* Changelog: add #10710

* changelog: add #10538

* changelog: add #10741

* changelog: add #10749

* changelog: add #10664

* changelog: add #10224

* changelog: add #10788

* Changelog: add #10560

* Chanegelog: add #10812

* changelog: add #10556

* Changelog: add #10668

* Changelog: add #10846

* Changelog: add #10947

* Changelog: add #10962

* Changelog: add #10956

* Changelog: add #10940

* Changelog: add #10934

* Changelog: add #10912

* changelog: add #10866

* changelog: add #10924

* Changelog: add #10936

* Changelog: add #10833

* changelog: add #10867

* Changelog: add #10960

* Changelog: add #10888

* changelog: add #10840

* changelog: add #10972

* Changelog: add #10979

* changelog: add #10909

* Changelog: add #10958

* Changelog: add #10981

* Changelog: add #10564

* Changelog: add #10809

* Changelog: add #10982

* Changelog: add #10706

* Changelog: add #10978

* Changelog: add #10132

* Changelog: add #11022

* Changelog: add #11024

* Changelog: add #10875

* Changelog: add #11030

* Changelog: add #11053

* Changelog: add #10880

* Changelog: add #9359

* Changelog: add #11037

* Update block list

* Changelog: add #11060

* Changelog: add #10755

* changelog: add #11000

* Changelog: add #10786

* Changelog: add #10945

* Changelog: add #10597
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jeffgolenski jeffgolenski jeffgolenski approved these changes

@jeherve jeherve jeherve approved these changes

Assignees

@joanrho joanrho

Labels

Admin Page React-powered dashboard under the Jetpack menu Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Pri] Normal

Projects

None yet

Milestone

6.9

Development

Successfully merging this pull request may close these issues.

6 participants

@joanrho @jetpackbot @dereksmart @jeffgolenski @jeherve @matticbot