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

Load a theme via a theme key in volto.config.js or in package.json #4625

Merged
merged 19 commits into from
Apr 11, 2023

Conversation

sneridagh
Copy link
Member

No description provided.

@sneridagh sneridagh requested a review from tiberiuichim March 30, 2023 11:24
@netlify
Copy link

netlify bot commented Mar 30, 2023

Deploy Preview for volto ready!

Name Link
🔨 Latest commit 14de867
🔍 Latest deploy log https://app.netlify.com/sites/volto/deploys/642af25107a45400081b9cc0
😎 Deploy Preview https://deploy-preview-4625--volto.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@cypress
Copy link

cypress bot commented Mar 30, 2023

Passing run #4796 ↗︎

0 489 20 0 Flakiness 0

Details:

A bit more of re-wording
Project: Volto Commit: 14de8670a4
Status: Passed Duration: 12:43 💡
Started: Apr 3, 2023 3:39 PM Ended: Apr 3, 2023 3:51 PM

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

@@ -130,6 +130,10 @@ class AddonConfigurationRegistry {
this.packages = {};
this.customizations = new Map();

// Theme from a package.json key or from volto.config.js
// Programatically via volto.config.js wins
this.theme = packageJson.theme || this.voltoConfigJS.theme;
Copy link
Contributor

Choose a reason for hiding this comment

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

I might want an addon to provide the theme.

The volto.config.js implies that the project is no longer throw-away, or at least there's a standard "distribution-like" volto.config.js that I use in all my projects.

Copy link
Member Author

@sneridagh sneridagh Mar 30, 2023

Choose a reason for hiding this comment

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

I agree that would be desirable, but the current state is that you need to define the enabled addons list in volto.config.js or in package.json anyways, so adding the theme is nothing that we "loose"...

In fact, these (addons and theme) are the only things that you need to "save" in order of a project to be expendable. Same applies for the dockerized approach... as long as you have a volto.config.js that you can forward to your expendable project, you are safe.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would add an option to define a theme name from an environment var. We have it already for addons iirc, so it would be a nice complement, and a very very nice addition for docker users :)

Copy link
Collaborator

@stevepiercy stevepiercy left a comment

Choose a reason for hiding this comment

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

Just a bunch of grammar and spelling fixes.

docs/source/addons/theme.md Outdated Show resolved Hide resolved
docs/source/addons/theme.md Outdated Show resolved Hide resolved
docs/source/addons/theme.md Outdated Show resolved Hide resolved
docs/source/addons/theme.md Outdated Show resolved Hide resolved
docs/source/addons/theme.md Outdated Show resolved Hide resolved
docs/source/addons/theme.md Outdated Show resolved Hide resolved
docs/source/addons/theme.md Outdated Show resolved Hide resolved
docs/source/addons/theme.md Outdated Show resolved Hide resolved
docs/source/addons/theme.md Outdated Show resolved Hide resolved
docs/source/addons/theme.md Outdated Show resolved Hide resolved
@tiberiuichim
Copy link
Contributor

here's my ideal scenario:

  • volto provides a hook that addons can hook into and be a part of the addons registry processing. Right now it could be done from razzle extend, but the addons registry would have to be reinitialized, etc. So we need a way, from razzle.config.js, to trigger additional hooks with a signature like (webpackConfig, addonRegistry) => config. These can come from the addons or, for this theming engine, from Volto itself
  • this would allow getCustomThemeAddons() to be removed from the addons-registry and into its own module, the "theming engine"

Other then that, I love it! Thanks for working on this!

Co-authored-by: Steve Piercy <web@stevepiercy.com>
@sneridagh
Copy link
Member Author

sneridagh commented Mar 31, 2023

@stevepiercy thanks for the review! Thanks for bearing with me! Yesterday I was not specially tuned for documenting, and the subject was not easy either.

@sneridagh
Copy link
Member Author

sneridagh commented Mar 31, 2023

@tiberiuichim I see, yeah, we could iterate over it, by all means! Right now, this implementation was one that made sense to me.

The main goal is for people not having to paste a recipe in their add-ons and then the entry points...

/cc @plone/volto-team let me know if you are fine with this and we all agree with merging it.

@sneridagh sneridagh requested a review from a team March 31, 2023 11:00
@sneridagh sneridagh changed the title Use an entrypoint in volto.config.js or in package.json to load a theme Load a theme via a theme key in volto.config.js or in package.json Mar 31, 2023
Copy link
Collaborator

@stevepiercy stevepiercy left a comment

Choose a reason for hiding this comment

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

One more round, and I think we're good on the docs.

news/4625.feature Outdated Show resolved Hide resolved
docs/source/addons/theme.md Outdated Show resolved Hide resolved
sneridagh and others added 2 commits April 2, 2023 09:49
Co-authored-by: Steve Piercy <web@stevepiercy.com>
I guess discard is ok

Co-authored-by: Steve Piercy <web@stevepiercy.com>
docs/source/addons/theme.md Outdated Show resolved Hide resolved
@@ -130,6 +130,10 @@ class AddonConfigurationRegistry {
this.packages = {};
this.customizations = new Map();

// Theme from a package.json key or from volto.config.js
// Programatically via volto.config.js wins
this.theme = packageJson.theme || this.voltoConfigJS.theme;
Copy link
Contributor

Choose a reason for hiding this comment

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

I would add an option to define a theme name from an environment var. We have it already for addons iirc, so it would be a nice complement, and a very very nice addition for docker users :)

* master:
  developer process for first time contributing (#4617)
  Trigger CI on pull_request event (#4629)
  Pining of `pydata-sphinx-theme` and `sphinx-book-theme`, CI is complaining. (#4626)
  Set sameSite in `18N_LANGUAGE` cookie (#4627)
@sneridagh
Copy link
Member Author

@stevepiercy I made a few last time amendments, do you mind take a final look?

@ksuess
Copy link
Member

ksuess commented Apr 3, 2023

Please integrate theme.md in docs/addons/index.md

Copy link
Collaborator

@stevepiercy stevepiercy left a comment

Choose a reason for hiding this comment

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

See suggestions.

docs/source/addons/theme.md Outdated Show resolved Hide resolved
docs/source/addons/theme.md Outdated Show resolved Hide resolved
docs/source/addons/theme.md Show resolved Hide resolved
docs/source/addons/theme.md Outdated Show resolved Hide resolved
3. Declare the theme as an add-on by adding its name as the value for the `addons` key in either `volto.config.js` or `package.json`.
4. After starting Volto, the theme should be active.
Now you can add overrides to the default theme in `src/theme`, same as you would in a project.
5. Now you can safely delete your project's `theme` folder, since the one in the add-on will take precedence.
Copy link
Member

Choose a reason for hiding this comment

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

Why should I delete the theme folder of the project. I want to use the theme, but use a different font.
Can I customize with src/theme/extras/custom.overrides for example heading sizes and fonts?

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess that we could state better the history behind the add-on themes and why after this, you don't need a project theme anymore (since only one is active at the same time). Effectively you are moving the theme from the project to an add-on, with all its consequences, so now your theme lives there.

Copy link
Member

Choose a reason for hiding this comment

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

I think "only one (theme) is active at the same time" needs explanation.
The naive understanding of a theme is just the collection of some CSS rules, maybe variables. But here you mean what exactly?

Copy link
Member

Choose a reason for hiding this comment

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

I think "only one (theme) is active at the same time".
The naive understanding of a theme is just a collection of CSS rules and maybe variables. How would you explain that thing that is the only one activated?

Now you can add overrides to the default theme in `src/theme`, same as you would in a project.
5. Now you can safely delete your project's `theme` folder, since the one in the add-on will take precedence.

## Using your own escape hatch
Copy link
Member

Choose a reason for hiding this comment

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

Maybe start with a first line about why and when a hatch is needed. From an integrater point of view, without having built multiple themes before.

docs/source/addons/theme.md Show resolved Hide resolved
sneridagh and others added 2 commits April 3, 2023 15:54
Co-authored-by: Steve Piercy <web@stevepiercy.com>
Co-authored-by: Katja Süss <k.suess@rohberg.ch>
docs/source/addons/theme.md Outdated Show resolved Hide resolved
sneridagh and others added 2 commits April 3, 2023 16:35
Co-authored-by: Katja Süss <k.suess@rohberg.ch>
@sneridagh
Copy link
Member Author

@ksuess I amended here and there, please take a look. Also always when we talk about a theme, is as in a Volto Semantic UI theme. It's true that we are overloading the term, here and there. Maybe it does need more rework.
Let me know what do you think.

@ksuess
Copy link
Member

ksuess commented Apr 3, 2023

I'm not able to outsource a simple (very simple) theme. For getting familiar with this topic, I set up a vanilla project and a simple add-on and followed the first section of addons/theme.md. But do not see the theme add-on customizations.
If you are too busy, no problem. But as I see myself as a multiplier, it could maybe help to enhance the docu for newbies.
So may I point you to
https://github.com/rohberg/project-with-theme
https://github.com/rohberg/springtime-theme/blob/main/src/theme/theme.config
and ask what I am missing in this very reduced example setup.

@ksuess
Copy link
Member

ksuess commented Apr 3, 2023

@ksuess I amended here and there, please take a look. Also always when we talk about a theme, is as in a Volto Semantic UI theme. It's true that we are overloading the term, here and there. Maybe it does need more rework. Let me know what do you think.

Ah, OK, so it's the theme.config that is THE theme. The Semantic UI theme.

@sneridagh
Copy link
Member Author

sneridagh commented Apr 3, 2023

I'm not able to outsource a simple (very simple) theme. For getting familiar with this topic, I set up a vanilla project and a simple add-on and followed the first section of addons/theme.md. But do not see the theme add-on customizations.
If you are too busy, no problem. But as I see myself as a multiplier, it could maybe help to enhance the docu for newbies.
So may I point you to
https://github.com/rohberg/project-with-theme
https://github.com/rohberg/springtime-theme/blob/main/src/theme/theme.config
and ask what I am missing in this very reduced example setup.

@ksuess it does work here like a charm! Not with the custom CSS you tried (the breadcrumbs). But:

body {
  background-color: red;
}

Works well.

Also, did you yalc'd the Volto version with the one in the PR, right?

The breadcrumb one should have been :)

.ui.secondary.segment {
  background-color: magenta;
}

image

@sneridagh sneridagh requested a review from davisagli April 3, 2023 15:36
@ksuess
Copy link
Member

ksuess commented Apr 4, 2023

I'm not able to outsource a simple (very simple) theme. For getting familiar with this topic, I set up a vanilla project and a simple add-on and followed the first section of addons/theme.md. But do not see the theme add-on customizations.
If you are too busy, no problem. But as I see myself as a multiplier, it could maybe help to enhance the docu for newbies.
So may I point you to
https://github.com/rohberg/project-with-theme
https://github.com/rohberg/springtime-theme/blob/main/src/theme/theme.config
and ask what I am missing in this very reduced example setup.

@ksuess it does work here like a charm! Not with the custom CSS you tried (the breadcrumbs). But:

body {
  background-color: red;
}

Works well.

Also, did you yalc'd the Volto version with the one in the PR, right?

The breadcrumb one should have been :)

.ui.secondary.segment {
  background-color: magenta;
}
image

[EDIT] With a fresh set up, this is now OK for me. Thank you.


You can see an example of such a theme in: https://github.com/kitconcept/volto-light-theme

## Modify a custom theme from another add-on
Copy link
Member

@ksuess ksuess Apr 4, 2023

Choose a reason for hiding this comment

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

It's still not clear to me, what part of this section are instructions for the reusable theme, and which instructions belong to the add-on which slightly customizes the theme.
I stop bothering now, as I planned to take some days of from yesterday on.
Thanks for the clarifications so far. I'll come back with questions later, like: "How would I use a reusable theme like kitconcept/volto-light-theme, but with another text color (probably $text-color ? But where? /theme/custom/_main.scss ?)"

A first try, wich is not OK so far: https://github.com/rohberg/project-with-theme

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah no worries, thanks for the valuable feedback! We will continue developing this and the story behind, and improving docs. The plan to work on this and the future of theming in Volto during Beethoven Sprint.

@sneridagh sneridagh merged commit ef3f6f8 into master Apr 11, 2023
@sneridagh sneridagh deleted the themeEntrypoints branch April 11, 2023 15:25
sneridagh added a commit that referenced this pull request Apr 11, 2023
…on` (#4625)

Co-authored-by: Steve Piercy <web@stevepiercy.com>
Co-authored-by: Katja Süss <k.suess@rohberg.ch>
@wesleybl
Copy link
Member

@sneridagh do you know if it is possible to put the public folder in an addon, as is done with the theme? See: #4654

sneridagh added a commit that referenced this pull request Apr 17, 2023
* master: (22 commits)
  Release changelog notes for 16.20.1
  Release 17.0.0-alpha.5
  Generate a split sitemap (also fix robots.txt) (#4639)
  Fix search block in edit mode re-queries multiple blocks with an empty search text (#4694)
  Fix Move to top of folder ordering in folder content view (#4691)
  Changelog
  Revert "Add current page parameter to the route in the listing and search block pagination (#4159)" (#4695)
  Release generate-volto 7.0.0-alpha.4
  Force the resolution of the `react-error-overlay` package to `6.0.9` (#4687)
  Fix training links (#4635)
  Release 17.0.0-alpha.4
  Release changelog notes for 16.20.0 (#4684)
  Update to latest backend versions (#4682)
  Support RelationList field with StaticCatalogVocabulary and SelectWidget. (#4614)
  Load a theme via a `theme` key in `volto.config.js` or in `package.json` (#4625)
  docs: improve creating view documentation (#4636)
  fix sitemap.xml.gz is not compressed #4622 (v2) (#4663)
  Make URL a literal string to fix broken link (#4667)
  Move developer guidelines to contributing #4665 (#4666)
  Update Volto contributing to align with and refer to the new Plone co… (#4634)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants