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

For CSS, create files for (1) base structure, (2) kindergartenprof theme #388

Closed
taoeffect opened this issue Apr 27, 2018 · 6 comments
Closed
Assignees
Labels
App:Frontend Kind:Core Anything that changes or affects the fundamental core data structures & design of the application. Kind:Documentation Note:UI/UX Priority:High

Comments

@taoeffect
Copy link
Member

taoeffect commented Apr 27, 2018

Problem

The current CSS Style Guide does not speak in terms of themes, but it needs to.

Also, our current CSS setup does not respect the existence of necessary theming support.

Solution

Part 1. - Refactor CSS structure

The current CSS structure of the project needs to be refactored to do two things:

  1. Create a file for the default structure related classes of the app (i.e. things that do not change across themes
  2. Create a file for the kindergartenprofessional theme (make sure to not name it "default-theme.scss", but name it something like kindergartenprofessional.scss; the "default" will be set via a JavaScript variable/setting)

Part 2. - Integrate JavaScript with CSS to set default theme to kindergartenprofessional

Basically what the subtitle of this section says.

There needs to be JavaScript code that sets the default theme and is responsible for switching themes.

Part 3. - Update Style Guide documentation to explain how to update and add new themes

The Style Guide currently ignores themes completely, but theming should probably be what everything in the style guide revolves around, because theming is what determines the location of files, their names, and how changes to the CSS is done.

@taoeffect taoeffect added App:Frontend Priority:High Kind:Documentation Note:UI/UX Kind:Core Anything that changes or affects the fundamental core data structures & design of the application. labels Apr 27, 2018
@sandrina-p
Copy link
Contributor

sandrina-p commented Apr 28, 2018

Part 1. - Refactor CSS structure

  1. That "file" in reality is the current /bulma_overrides structure. What's inside is dynamic so it can adapt to every theme without needing to change.
  2. The file was created at theme/kindergarten-professional.scss on #346 Start building Design System - Style Guide #387.

Part 2. - Integrate JavaScript with CSS to set default theme to kindergartenprofessional

So, as we discussed on Slack. We have two options: CSS Preprocessors (SCSS variables) or Native CSS (CSS Custom Properties Variables). If you don't know exactly what's the difference, this articles explain it well

How it would work if we use...

1. CSS Preprocessors (SCSS variables)

  • We need to update Grunt configurations to create a CSS bundle for each theme. kindergarten-professional.css, another-theme.css, etc... Not sure how we would do that. But whe would end up with repeated bundles where only those dynamic values (colors, typography) changed.
  • When the users changes the theme on the site, the CSS file for that theme needs to be requested and replace the previous one. This is an extra network request.

2. Native CSS - CSS Variables

If they do it soon enough it would be awesome, because we would only need to to add a class to the <body /> to toggle new css variables and voilà! Otherwise we need to fork Bulma and change the code to support CSS Variables`. Something similar to this article, so we have a fallback for browsers that don't support CSS Variables (IE and Opera Mini)


I believe we could wait a little longer before forking Bulma. The later we fork it, the stable it will be. Meanwhile we keep improving GI App with theming in mind.

@taoeffect
Copy link
Member Author

taoeffect commented Apr 28, 2018

  1. Native CSS - CSS Variables

Bulma doesn't support yet CSS variables but it's on their roadmap.
If they do it soon enough it would be awesome, because we would only need to to add a class to the to toggle new css variables and voilà! Otherwise we need to fork Bulma and change the code to supportCSS Variables`. Something similar to this article, so we have a fallback for browsers that don't support CSS Variables (IE and Opera Mini)

This looks complicated as hell.

Another problem is: say we have 10 themes, doing it this way would mean clients would have to load and process all of that CSS. Plus, it's not clear to me how CSS variables fit into the theme stuff anyway.

There needs to be one javascript variable that gets toggled.

That should be a variable called "theme", and it should refer to the filename of the css theme that's being used.

So let's go with the multiple theme file approach.

  • We don't have time to wait for Bulma
  • We want multiple theme files, and for one to be injected dynamically into the page when a JS variable is toggled. @hubudibu can help with that
  • Now we don't have to worry about supporting old or non-standard browsers too
  • We must fork Bulma anyway (per Get rid of dependencies + Adopt/fork rest #372)

@sandrina-p
Copy link
Contributor

Another problem is: say we have 10 themes, doing it this way would mean clients would have to load and process all of that CSS. Plus, it's not clear to me how CSS variables fit into the theme stuff anyway.

What don't you understand exactly?

There needs to be one javascript variable that gets toggled. That should be a variable called "theme", and it should refer to the filename of the css theme that's being used.

That's the beauty about CSS variables. The same css file will serve all themes! JS only needs to toggle a parent classname that triggers different CSS variables values. No need for extra CSS network. request.

@taoeffect
Copy link
Member Author

What don't you understand exactly?

Nevermind, I think I see how it would work.

That's the beauty about CSS variables. The same css file will serve all themes! JS only needs to toggle a parent classname that triggers different CSS variables values. No need for extra CSS network. request.

OK, it's not a big deal, if you want to do it this way, let's do it this way. If we encounter performance issues or any other issues with this approach, we can always adjust later as need be. 👍

@mmbotelho
Copy link
Collaborator

@taoeffect @sandrina-p is this issue still relevant?

@taoeffect
Copy link
Member Author

Probably not... I think the #665 can supersede this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
App:Frontend Kind:Core Anything that changes or affects the fundamental core data structures & design of the application. Kind:Documentation Note:UI/UX Priority:High
Projects
None yet
Development

No branches or pull requests

3 participants