RFC: Remove subpath URLQueryParam from settings pages.
#1031
Conversation
Use WC navigation features instead.
tomalec
added
type: technical debt
|
@tomalec , the reason why we use
You can enable the WC Navigation feature in WC Settings > Advanced > Features. |
|
I managed to achieve that by using It was quite hidden in the docs, so I found it by debugging it live 🤦♂️ |
in the new WC Navigation. Addresses #1031 (comment).
|
Highlighting the subpages in legacy menu is covered by #1038 |
| * @param {Object} [props.params = {}] `path` params extracted by React `<Router>`. | ||
| * @param {string} [props.params.subpath] Sub path of a nested settings page. |
| // Highlight this menu item for other subpages. | ||
| 'matchExpression' => '/google/settings(/[^/]+)?', |
There was a problem hiding this comment.
TIL: matchExpression. Great find! 🙌
|
Mental note - before we go merging will this impact any funnels we have set up or direct links we may have (in the codebase or docs) |
Yes, it will. All So the That's why I still have on my todo list:
|
|
@jconroy does the above make it a no-go? or maybe it's ok, but we should change everything at once, to update the docs and funnels only once, to minimize the inconvenience, for users of those. |
| @@ -88,7 +88,7 @@ protected function get_connect_callback(): callable { | |||
| return function( Request $request ) { | |||
| try { | |||
| $next = $request->get_param( 'next' ); | |||
| $path = $next === 'setup-mc' ? '/google/setup-mc' : '/google/settings&subpath=/reconnect-accounts'; | |||
| $path = $next === 'setup-mc' ? '/google/setup-mc' : '/google/settings/reconnect-accounts'; | |||
There was a problem hiding this comment.
📝
Will conflict with this commit in PR #1069
(This is a follow-up from #1025 (comment))
Changes proposed in this Pull Request:
Remove
subpathURLQueryParam from settings pages.Use WC navigation features instead.
This way we have
.~/utils/url.js, as we no longer need to construct a query object, but just usepathparam.subpathis empty?"What's left/missing/to be done later:
wc_admin_register_pagecalls insrc/Menu/Settings.phpsubpathor similar param.[ ] 📜(not sure if possible in WC-plaform) DRY/reduce duplication, of pages configs in-> Cleanup navigation related code #1037src/Menu/*.phps,js/src/index.jsandjs/src/utils/url.jsScreenshots:
Detailed test instructions:
/wp-admin/admin.php?page=wc-admin&path=%2Fgoogle%2Fsettings(full page reloads instead of SPA-morph, is an unrelated issue Impaired UX/A11Y of internal button links. #1027)
Changelog entry
//cc @eason9487 @ecgan