Skip to content

Jetpack: Use settings search term as default module configuration URL#10668

Merged
brbrr merged 7 commits intomasterfrom
update/configure-url-to-jetpack-settings
Dec 7, 2018
Merged

Jetpack: Use settings search term as default module configuration URL#10668
brbrr merged 7 commits intomasterfrom
update/configure-url-to-jetpack-settings

Conversation

@brbrr
Copy link
Copy Markdown
Contributor

@brbrr brbrr commented Nov 19, 2018

Changes proposed in this Pull Request:

  • Updates configure_url to redirect to Jetpack Settings search term by default. Before this - it was redirecting to legacy pre-react configuration pages.

This change required by #10611 to provide flawless UX.

Testing instructions:

  • Go to wp-admin/admin.php?page=jetpack_modules
  • Find a module with 'Configure' button.
  • Make sure it redirects to wp-admin/admin.php?page=jetpack#/settings?term=$MODULE
  • try some other modules. Some of them might redirect

Proposed changelog entry for your changes:

  • Admin Page: Module configuration URLs now redirects to the relevant section in Jetpack settings

@brbrr brbrr added [Status] Needs Review This PR is ready for review. Admin Page React-powered dashboard under the Jetpack menu labels Nov 19, 2018
@brbrr brbrr self-assigned this Nov 19, 2018
@brbrr brbrr requested a review from a team November 19, 2018 15:51
@jetpackbot
Copy link
Copy Markdown
Collaborator

jetpackbot commented Nov 19, 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

@brbrr brbrr mentioned this pull request Nov 19, 2018
Merged
@brbrr
Copy link
Copy Markdown
Contributor Author

brbrr commented Nov 19, 2018

It worth to mention that this PR (as-is) re-defines a configuration URL, which means that any custom redirects such as https://github.com/Automattic/jetpack/blob/master/modules/publicize.php#L56 will stop working.

I've started working on cleaning up "old" code and overriding these URLs for specific modules (#10669 ), but it might be better to include these overrides in this PR.

@brbrr brbrr force-pushed the update/configure-url-to-jetpack-settings branch from 27b9332 to b7ff278 Compare November 19, 2018 16:39
@jeherve jeherve added [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. [Status] In Progress and removed [Status] Needs Review This PR is ready for review. labels Nov 29, 2018
jeherve
jeherve previously requested changes Nov 29, 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.

While this works well, it causes issues for the modules that do not have any configuration options in the Jetpack dashboard.

  • Custom CSS redirects to the customizer, where one can use the feature.
  • Widgets redirects to the widgets dashboard page.
  • Infinite Scroll has more options in Settings > Reading than in the JP dashboard.

Before we flip the switch, we should consider what options would become harder to access with that change.

@matticbot
Copy link
Copy Markdown
Contributor

matticbot commented Dec 3, 2018

D21714-code. (newly created revision)

@brbrr
Copy link
Copy Markdown
Contributor Author

brbrr commented Dec 3, 2018

@jeherve good point. I went through all the configurable modules and updated some of the configuration URLs which miss some configuration options in Jetpack settings. I left untouched sharing-buttons & publicize, but I am not sure about that since Jetpack settings option redirects to WP.com for further configuration, which isn't really good UX.

@brbrr brbrr added [Status] Needs Review This PR is ready for review. and removed [Status] In Progress [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Dec 4, 2018
@brbrr brbrr requested a review from a team December 5, 2018 08:29
@jeherve jeherve added [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. and removed [Status] Needs Review This PR is ready for review. labels Dec 6, 2018
jeherve
jeherve requested 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.

This tests well. I just have one tiny suggestion for the Widgets module.

@brbrr brbrr added [Status] Needs Review This PR is ready for review. and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Dec 6, 2018
@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.

Looking good. 👍

@brbrr brbrr merged commit a356d87 into master Dec 7, 2018
@ghost ghost removed the [Status] Ready to Merge Go ahead, you can push that green button! label Dec 7, 2018
@brbrr brbrr deleted the update/configure-url-to-jetpack-settings branch December 7, 2018 07:02
jeherve added a commit that referenced this pull request Dec 19, 2018
@jeherve
Changelog: add #10668
acaf7d6
jeherve added a commit that referenced this pull request Jan 3, 2019
@jeherve
Changelog: add #10668
ecda8fd
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

@jeherve jeherve jeherve approved these changes

Assignees

@brbrr brbrr

Labels

Admin Page React-powered dashboard under the Jetpack menu Touches WP.com Files

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@brbrr @jetpackbot @matticbot @jeherve