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

Full Site Editing Design Picker: Enable Arbutus theme #58695

Merged
merged 3 commits into from
Dec 2, 2021

Conversation

jeyip
Copy link
Contributor

@jeyip jeyip commented Dec 1, 2021

Changes proposed in this Pull Request

  • Update available-designs-config.json to include new Arbutus block theme.
  • Arbutus is a selectable theme in the /new FSE beta onboarding design picker

Screenshots

Screen Shot 2021-11-30 at 5 20 56 PM

Testing instructions

  • Apply branch to local calypso dev environment
  • Visit http://calypso.localhost:3000/new/beta
  • Enroll in the FSE beta
  • Pick a free test domain
  • Verify that Arbutus is a selectable theme
  • Create a site with Arbutus enabled and smoke test the theme

Related to # #58663

@jeyip jeyip added [Goal] Full Site Editing [Feature] Design Picker Picking themes and designs during onboarding. labels Dec 1, 2021
@jeyip jeyip self-assigned this Dec 1, 2021
@github-actions
Copy link

github-actions bot commented Dec 1, 2021

@matticbot
Copy link
Contributor

matticbot commented Dec 1, 2021

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

App Entrypoints (~38 bytes added 📈 [gzipped])

name                 parsed_size           gzip_size
entry-gutenboarding       +353 B  (+0.0%)      +38 B  (+0.0%)

Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used.

Sections (~26 bytes added 📈 [gzipped])

name             parsed_size           gzip_size
signup                +353 B  (+0.1%)      +26 B  (+0.0%)
jetpack-connect       +353 B  (+0.0%)      +26 B  (+0.0%)
accept-invite         +353 B  (+0.1%)      +26 B  (+0.0%)

Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to.

Legend

What is parsed and gzip size?

Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory.
Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

@matticbot matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Dec 1, 2021
@jeyip
Copy link
Contributor Author

jeyip commented Dec 1, 2021

Note:

These changes also require updates to the Arbutus and Russell themes.

Currently, in the themes showcase, Arbutus and Russell aren't displayed with beta tags alongside other FSE themes because they don't have the block-templates theme support enabled.

@mattwiebe
Copy link
Contributor

in the themes showcase, Arbutus and Russell aren't displayed with beta tags alongside other FSE themes

No longer! I added the tags to the theme showcase posts, so they're showing up as expected in the FSE tab:

image

I also pushed an update on this branch to allow listing Arbutus and Russell as Classic themes. This requires simply duplicating each theme with is_fse: false so that they'll list on that side as well (gross, but it works and is much less work than modifying getAvailableDesigns logic).

Copy link
Contributor

@zaguiini zaguiini left a comment

Choose a reason for hiding this comment

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

Works great for logged-in and logged-out users via /themes, they're not showing as duplicates.

Question: why do we store the themes list of the design picker inside of a JSON instead of in a database entry, so we could update this list without necessarily re-deploying Calypso?

@jeyip
Copy link
Contributor Author

jeyip commented Dec 1, 2021

why do we store the themes list of the design picker inside of a JSON instead of in a database entry, so we could update this list without necessarily re-deploying Calypso?

I agree with this sentiment, but I'm unsure about the history behind this decision. Maybe @p-jackson or @ciampo could provide more context?

@ciampo
Copy link
Contributor

ciampo commented Dec 1, 2021

I agree with this sentiment, but I'm unsure about the history behind this decision. Maybe @p-jackson or @ciampo could provide more context?

Unfortunately I don't have any insights into this; when I worked on the DesignPicker the available designs configuration was already stored in a JSON file.

@creativecoder
Copy link
Contributor

Questions: why do we store the themes list of the design picker inside of a JSON instead of in a database entry, so we could update this list without necessarily re-deploying Calypso?

I believe API powered design options simply hasn't been implemented yet, in this newer onboarding flow.

@p-jackson
Copy link
Member

Yup, it just hasn't been implemented yet. The designs in /new were originally a new concept called a "Design", which didn't map 1:1 to themes, so there was no database entry to pull from. However now they're just themes, so should be driven by the API the same way the design picker in the /start flow is.

Question: is there a taxonomy or term that all the FSE beta themes have in common? When the design picker in /new is API driven will we be able to filter them using some sort of tag so we only show the FSE beta ones? Or will we need to have a hardcoded list of theme slugs in the front end to know which are eligible for the beta.

@mattwiebe
Copy link
Contributor

The historical reason for storing the themes in JSON is simply that we've (almost) always done that in signup/onboarding. A good reason is performance (no API endpoint needed) but it's poor from an ongoing maintenance perspective. I am happy to see the Blogger Onboarding initiative use the REST API so as to reduce ongoing work in launching new themes.

Question: is there a taxonomy or term that all the FSE beta themes have in common? When the design picker in /new is API driven will we be able to filter them using some sort of tag so we only show the FSE beta ones? Or will we need to have a hardcoded list of theme slugs in the front end to know which are eligible for the beta.

The FSE themes all currently use block-templates on the theme_feature taxonomy. We will migrate that to the more sensible full-site-editing at some point in the future, once we are able to deprecate the old Dotcom FSE implementation which currently uses that tag.

@creativecoder
Copy link
Contributor

Note this issue, for tracking an enhancement to get FSE designs via API during onboarding: #58667

Also, I think it's likely that the design choices may not all be "themes" in the future, in the traditional sense. They could be a theme + a specific home page template, or a theme with an alternative theme.json (which has been discussed in Core, but I don't think it's implemented yet). So whatever API driven solution we create, it should be flexible to adapt to designs being something other that strictly a theme choice.

@jeyip jeyip changed the title Full Site Editing Design Picker: Enable Arbutus and Russell themes Full Site Editing Design Picker: Enable Arbutus theme Dec 2, 2021
@jeyip
Copy link
Contributor Author

jeyip commented Dec 2, 2021

I created a separate PR to enable the Russell theme in the design picker. The reason we're splitting up the PRs is because Russell currently has block error issues when loading in the site editor.

We're planning to investigate the errors before displaying Russell to users in the onboarding flow.

@jeyip jeyip merged commit 8020aa5 into trunk Dec 2, 2021
@jeyip jeyip deleted the add/arbutus-and-russell-to-fse-design-picker branch December 2, 2021 20:47
@github-actions github-actions bot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Dec 2, 2021
nelsonec87 pushed a commit that referenced this pull request Dec 9, 2021
* Enable arbutus and russell in design picker

Co-authored-by: Matt Wiebe <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Design Picker Picking themes and designs during onboarding. [Goal] Full Site Editing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants