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.

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

Theme Switcher: Default to using the redesigned theme for pages #313

Merged
merged 3 commits into from
Aug 16, 2023

Conversation

ryelle
Copy link
Contributor

@ryelle ryelle commented Aug 12, 2023

This will reverse the logic for loading the redesign theme, so that pages are opted-out of the new theme, rather than opted in. It also makes the page check case-insensitive. This fixes some edge cases where technically-valid page URLs are used, but the theme switcher fails to load the correct template.

For example, the following URLs should now load the correct theme & content.

  • Capitalized slugs: wordpress.org/About (or Blocks, as the original issue found, but there's a workaround for that page in place now)
  • Slugs with dots: wordpress.org/download/releases/6.3/ (IIRC this was reported in slack but not followed up on)
  • Arbitrary dashes: wordpress.org/download/beta----nightly/
  • More dots: wordpress.org/download/beta....nightly/

Ideally these URLs should redirect to the real canonical URL, but since they are functional links, the theme should at least not be broken. There are core tickets for the arbitrary dashes and dots in permalinks (which also applies to the 6.3 issue despite seeming related to the post title).

See #245 (comment), https://meta.trac.wordpress.org/ticket/7204

How to test the changes in this Pull Request:

  1. Check out the branch, test locally or sync to sandbox (mu-plugins/main-network/theme-switcher.php)
  2. Try visiting any of the example URLs
  3. The page should load the new theme and show the correct content
  4. Try other valid pages, they should continue to work
  5. Visit /hosting/ or another "old theme" page
  6. It should still use the old theme & content

@ryelle ryelle added the [Component] Backend Anything wp-admin or PHP-related label Aug 12, 2023
@ryelle ryelle self-assigned this Aug 12, 2023
Copy link
Contributor

@adamwoodnz adamwoodnz left a comment

Choose a reason for hiding this comment

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

Works for me.

Wondering if we need all of the truthy conditions above anymore, now that more pages will already get the new theme by not being in the exception list, for example /search/ and maybe preview links?

@ryelle ryelle force-pushed the update/theme-switcher-default branch from 3f468b3 to 455253b Compare August 15, 2023 17:52
@ryelle
Copy link
Contributor Author

ryelle commented Aug 15, 2023

Good idea, that does simplify the function. Looks like we don't need the search one, but I kept preview so that those links will always use the new theme.

Copy link
Contributor

@adamwoodnz adamwoodnz left a comment

Choose a reason for hiding this comment

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

:shipit: Still LGTM

@ryelle ryelle merged commit b44b16d into trunk Aug 16, 2023
2 checks passed
@ryelle ryelle deleted the update/theme-switcher-default branch August 16, 2023 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Component] Backend Anything wp-admin or PHP-related
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants