Gutenboarding: Consolidate templates metadata #40587
Conversation
…op sending the cache-buster vertical param to mshots
johnHackworth
added
[Status] Needs Review
|
Here is how your PR affects size of JS and CSS bundles shipped to the user's browser: App Entrypoints (~144 bytes added 📈 [gzipped]) Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used. 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. Generated by performance advisor bot at iscalypsofastyet.com. |
| "theme": "alves", | ||
| "src": "https://public-api.wordpress.com/rest/v1/template/demo/alves/alves2/", | ||
| "fonts": [ "Playfair Display", "Fira Sans" ], | ||
| "categories": [ "featured", "business" ] | ||
| } | ||
| ], | ||
| "themes": [ |
There was a problem hiding this comment.
While we're here, is it safe to remove the themes array? In D41177-code, I changed the template demo endpoint to infer a given theme's demo site through wpcom_get_theme_demo_site(). If that was the only consumer of the themes array, it should be safe to remove now. (Or was there another?)
There was a problem hiding this comment.
Oh, sorry, I haven't seen that. Yeah, that array was used only to match with the demo site, so I guess it's safe to remove it now!
| "vertical-whitelist": [] | ||
| "title": "Vesta", | ||
| "slug": "vesta", | ||
| "template": "mayland2", |
There was a problem hiding this comment.
Is there any chance we can get rid either of the slug, or the template field? (I hope not to misrepresent, but IIRC, Jon got those mixed up earlier today, too.)
There are a lot of entities here. IIUC, we call Vesta a... design? But I haven't fully figured out why we need to differentiate between that, and a template. Can we not equate designs with templates -- at least here, for engineering purposes? Is the differentiator the font pairing? (If it is, I'd still suggest going with one entity -- template -- and thinking of that font pairing simply as the default option.)
(I think we can still refer to a combination of theme, template, font pairing/global style as a design -- probably using the template name.)
There was a problem hiding this comment.
I think we can still refer to a combination of theme, template, font pairing/global style as a design -- probably using the template name.
Is the plan to be able to use one template for multiple "designs"? If so, equating the template name with design could be problematic later down the road.
It seems like any combination of template+theme+global styles/fonts (i.e. "design") should have a unique identifier.
There was a problem hiding this comment.
I think we can still refer to a combination of theme, template, font pairing/global style as a design -- probably using the template name.
Is the plan to be able to use one template for multiple "designs"? If so, equating the template name with design could be problematic later down the road.
I think we need to clarify which of these are always coupled, and which can be freely exchanged. E.g. I think the plan is for the user to always select any Global Style/Font Pairing with any template/theme (you could say Global Styles/Font Pairing is one degree of freedom).
It seems like any combination of template+theme+global styles/fonts (i.e. "design") should have a unique identifier.
But I don't think we'd want individual names for each of 'Rivington+Rockfield+Arvo/Montserrat', 'Rivington+Rockfield+Chivo/Open Sans', etc. That would get unwieldy pretty soon (what's a good name for 'Rivington+Rockfield+Arvo/Montserrat' -- something kinda descriptive such as 'RivRockArMon'? 'RRAM'? Or another random name such as 'Gaudi'?). I think it's fine to refer to each design by naming which element is chosen for each degree of freedom (so, back to 'Rivington+Rockfield+Arvo/Montserrat').
tl;dr -- I wouldn't equate design with template names. We can refer to designs by the theme/template/global style they're made up of.
There was a problem hiding this comment.
think of a "design" as a pair of "theme + homepage template + presets for global styles". I agree we shouldn't equate design and template names for .... well, the reasons you mention. It doesn't make sense because a template could potentially be reused in several designs. But at the moment, that's what we have, so we'll iterate to a place where those names are not tied
There was a problem hiding this comment.
This PR removes the "available-designs.json" file on gutenboarding to rely on the "page-templates.json", higher-level data. Those two files where defining the same content, so there isn't any reason to keep both of them.
I don't think this PR actually removes available-designs.json, I think we still need to add a commit to do that.
|
FYI: #40680 changes available designs somewhat. |
| "slug": "vesta", | ||
| "template": "mayland2", | ||
| "theme": "mayland", | ||
| "src": "https://public-api.wordpress.com/rest/v1/template/demo/mayland/mayland2/", |
There was a problem hiding this comment.
I don't think we actually need these URLs, as they can be built the theme and template fields. We're doing that already here:
(Can be handled in a follow-up tho.)
There was a problem hiding this comment.
you are totally right there. We can build the entire thing from the rest of the fields !
|
@ramonjd I'm assuming this is ok to close? Feel free to re-open if need. |
matticbot
removed
the
[Status] Needs Review
Yes please. Thank you! Things are going to look a lot different in the coming weeks 🤞 |
This PR removes the "available-designs.json" file on gutenboarding to rely on the "page-templates.json", higher-level data. Those two files where defining the same content, so there isn't any reason to keep both of them.
It also removes (temporarily) the vertical param on the mshots call on the designs page. Since the call for testing is not going to include vertical content, at this point the only thing it's doing is being a cache buster for shots, so we can remove it by now until it actually has some use.
How to test?