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

Implement multi-theme system #28131

Closed
wants to merge 7 commits into from

Conversation

carlomanf
Copy link

@carlomanf carlomanf commented Jan 12, 2021

Description

Closes #26950
Closes #26546
Closes Automattic/wp-calypso#48145

In recent months, there have been whispers around the future possibility of multiple themes being active, templates being "themeless," etc. This branch is an implementation of that.

The idea behind this implementation is there can only be one active theme at a time, but the wp_theme taxonomy can be used to link up individual templates / template parts with one or more themes at a time.

This branch does a few things:

  • Removes the theme name from the REST API endpoint, solving duplicates and meaning that only one template / template part can be requested per slug. Templates / template parts not linked with the active theme are not accessible. (That's why it's called the "active" theme.)

  • Removes the theme attribute from the template part block.

  • When switching theme, users are given one chance to link any old templates / template parts to their new theme. (After the new theme is activated, anything they didn't link will become inaccessible until the old theme is re-activated.)

  • If the user elects to link a template to the new theme, any of its template parts are automatically linked too.

Planned additions to this branch:

  • Linked templates / template parts need to be stored in the database even if they previously were not. (This could be considered a down-side of the approach, since the user may not have made any changes.)

  • If the user tries to re-activate a previously active theme, plus link a template / template part that would create a slug conflict, they are shown an error message and can't activate the theme until they de-select the one that is creating the conflict. (Don't know if this is the best solution.)

  • When an inactive theme is deleted, its taxonomy term is deleted as well. Any templates / template parts that only belonged to that theme alone, are deleted.

Maybe will attempt:

  • Link templates / template parts from inactive themes into the active theme, without switching.

  • Unlink a template / template part from the current theme but leave it linked with its other themes.

Not planned for this branch to attempt, but maybe should still be handled:

  • Handle stylesheets, global styles, etc.

  • Warn the user when deleting a template / template part that is linked to multiple themes.

  • Let the user clone a template / template part from their old theme, and link the clone to the new theme rather than the original. (This would give the user more control but might create a lot of slug conflicts.)

  • Unit tests.

  • Make the UI nicer.

Types of changes

Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

@github-actions github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Jan 12, 2021
@carlomanf carlomanf marked this pull request as ready for review January 13, 2021 03:02
@carlomanf carlomanf changed the title Draft Implement multi-theme system Jan 13, 2021
@aristath aristath self-requested a review January 13, 2021 13:35
@Addison-Stavlo
Copy link
Contributor

Removes the theme name from the REST API endpoint, solving duplicates and meaning that only one template / template part can be requested per slug.

When switching theme, users are given one chance to link any old templates / template parts to their new theme. (After the new theme is activated, anything they didn't link will become inaccessible until the old theme is re-activated.)

Would the combination of this mean that if I linked the header template part over to the new theme I am switching to, that I would not be able to access the header that theme provides? If so it seems a bit unfriendly as users may want to carry over template parts but still have the option to preview/use what is provided by the newly activated theme (as opposed to being forced to decide between the two at the time of theme switch).

@gziolo gziolo requested a review from youknowriad January 13, 2021 19:33
@gziolo gziolo added [Feature] Full Site Editing [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. labels Jan 13, 2021
@carlomanf
Copy link
Author

Would the combination of this mean that if I linked the header template part over to the new theme I am switching to, that I would not be able to access the header that theme provides? If so it seems a bit unfriendly as users may want to carry over template parts but still have the option to preview/use what is provided by the newly activated theme (as opposed to being forced to decide between the two at the time of theme switch).

@Addison-Stavlo Thanks for your interest. This scenario would be handled by the following point, which is not yet implemented at the time of writing:

  • If the user tries to re-activate a previously active theme, plus link a template / template part that would create a slug conflict, they are shown an error message and can't activate the theme until they de-select the one that is creating the conflict. (Don't know if this is the best solution.)

In your scenario, the theme switch would be impeded because the new theme already includes its own "header."

It's certainly conceivable that a user might want to use one "header" on their front-page for example, and another "header" on their other pages, but I'm not sure there is an easy solution to that.

The problem with reinstating the theme reference into the endpoint is it would make it extremely difficult to list out the "active" template-parts (in the site editor, for example) without listing out every template-part from every theme.

I'm open to suggestions if you have any.

@carlomanf
Copy link
Author

The problem with reinstating the theme reference into the endpoint is it would make it extremely difficult to list out the "active" template-parts (in the site editor, for example) without listing out every template-part from every theme.

Just want to be clear, this point applies to the master branch as well, and shouldn't be considered a limitation of this branch.

Base automatically changed from master to trunk March 1, 2021 15:45
@aristath
Copy link
Member

Is this something we still want to pursue? If yes, then this needs rebasing 👍

@youknowriad
Copy link
Contributor

IMO we need to make choices, I don't think we should implement this for FSE v1 to avoid extra complexity and potential new issues/confusion but I think it's definitely something to pursue for later.

@carlomanf
Copy link
Author

I agree that it's a "later" feature, and always have, but I submitted this branch more for the purpose of illustrating a point.

The trunk implementation of the template endpoint has the format themename//templatename. This branch requires it to be just templatename, because in a multi-theme environment, the client-side will not know which theme to fetch from and requires the endpoint to determine it.

If this feature is planned for later on, then a required change in the endpoint format should be setting off alarm bells right now. The endpoint is already available for use in its current format, and it could cause headaches for plugin developers if it needs to be changed later. In other words, I agree that now is not the time to ship this feature, but I do think that now is the time to discuss whether the template endpoint format is appropriate and forward compatible.

@gziolo gziolo added the [Type] Technical Prototype Offers a technical exploration into an idea as an example of what's possible label Mar 17, 2021
@carlomanf
Copy link
Author

Closing, because I don't plan to add any more commits and I think the prototype has served its purpose.

@carlomanf carlomanf closed this Apr 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Themes Questions or issues with incorporating or styling blocks in a theme. First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository [Type] Technical Prototype Offers a technical exploration into an idea as an example of what's possible
Projects
None yet
5 participants