GFX: Theming tweaks #1590

AllienWorks · 2019-05-02T17:07:56Z

No much has changed visually, but this PR brings updates & tweaks to how our themes are handled (now properly, Material-way).

This will also help with dynamic themes in the future, which I wanted to keep away as a surprise, but you'd find out anyway, so.. :)

zaSmilingIdiot · 2019-05-03T07:53:01Z

@AllienWorks Looks like travis ran into an issue with the tests... testing locally results in the same error.

While we can increase the amount of memory node uses to run with during the tests, which should help solving the problem, I'm not particularly keen on doing that unnecessarily. To that end, I've traced the issue to the inclusion of both light and dark themes, specifically, adding both the following imports into _config.scss:

// Import themes
@import 'theme-light';
@import 'theme-dark';

Notably, just including one or the other starts the tests runner at least.
Testing this a little more specifically, removing the line:
@include angular-material-theme($dark-theme);
for example from either of the theme files seems to work as well (while importing both theme files in _config.scss).

Seems there is a problem with including angular-material-theme more than once? Anyways, looking at the make-up of its inclusion, the same data/inputs/construct are used in both theme files. Would it not be simpler for the sake of theme creation (and to possibly solve this issue), to take the whole of:

// instead of creating a theme with mat-light-theme like a sane person, we will create our own theme-creating function that lets us apply our own foreground and background palettes.
@function create-part-theme($primary, $accent, $warn) {
  @return (
    primary: $primary,
    accent: $accent,
    warn: $warn,
    is-dark: true,
    foreground: $theme-foreground,
    background: $theme-background,
  );
}

// The rest of this is like a normal theme.scss file. The only difference being that we use create-part-dark-theme instead of mat-light-theme
$theme-primary: mat-palette($part-green, 500, 100, 900);
$theme-accent: mat-palette($part-blue, 500, 100, 900);
$theme-warn: mat-palette($part-red);

$dark-theme: create-part-theme($theme-primary, $theme-accent, $theme-warn);


// Include theme styles for core and each component used in app
.part-dark-theme {
  @include angular-material-theme($dark-theme);
}

out of each theme file and import it once (possibly via a separate scss file??) in _config.scss after the theme files have been imported? Replacing the variables $dark-theme and $light-theme with just something like $theme. I'm not sure if its possible, so just checking...

AllienWorks · 2019-05-03T12:07:14Z

To that end, I've traced the issue to the inclusion of both light and dark themes, specifically, adding both the following imports into _config.scss

Seems there is a problem with including angular-material-theme more than once?

This is a correct way of adding multiple themes which can be switched dynamically. Honestly I'm not quite sure what might be the problem there.

Basically you define the primary, accent and warn colors of your theme. You can use the Material color pallets for this or define your custom ones (what I did).

This will however only swap the main colors. As I was experimenting with dark theme, it become apparent that we'll need to do more than that, ie. redefine even the foreground and background colors for the components - that's what _theme-dark.scss and _theme-light.scss do. Than you take these presets and pair them with some class for when they should be used:

// Include theme styles for core and each component used in app
.part-dark-theme {
  @include angular-material-theme($dark-theme);
}

It's way more complicated than I anticipated, as we'd need to redefine all colors in all components based on theme used. That would mean a lot of new code for each new theme used. In my naivity, I thought one small file with defined colors for each theme would be enough, but it's not the case.

The dark theme isn't even super nice at the moment - even half-finished.. You can check it with replacing the class in index.html file:

<body class="part-light-theme">
- to -
<body class="part-dark-theme">

So yes, I'm having second thoughts about this. All new code will need to be styled two times (color-wise) for 2 themes. If more themes are created, it will get even worse.

At this point I'm thinking "let's stick to one default theme only". If you guys agree, I will remove the dark theme code from this PR. However this PR has even more changes which I'd like to see merged.

zaSmilingIdiot · 2019-05-03T13:58:39Z

Fair enough... you're the expert on these things which is why I was asking. :D

I got the whole material theme setup (actually did some looking into how the themes were generated, so I felt a little more educated :b). From I could determine, most places were
@include angular-material-theme(...) only once, which resulted in the previous observations when I tried removing from just one of the themes. Which led to the whole question of whether it was possible to define the themes, and then include angular-material-theme once somewhere, and pass all the theme definitions to it. Rather than including it in every theme file and importing all theme files directly to it.

The way I would have seen this working is (which is subject to my own experiences so may be a little shortsighted or "not quite right", so take it for what it is):

the themes defined would have been pre-built (using gulp or grunt for example). This might be included as part of a pre-build or pre-compile step of the application.
on application startup, check whether a saved theme exists. If not, use a predefined default theme file.
using JS and referencing the index.html page, inject/replace the css src for the selected theme into the index.html page.

Granted, I haven't tried that with Material themes before, so oh well :D

Anyways...

If you're happy to keep a single theme for now then I am as well. And if there are other changes that you want included here, then perhaps removing one of themes for now is better, just to get those changes in.

AllienWorks · 2019-05-06T10:54:49Z

OK, dark theme removed + some small tweaks added. Ready to merge @zaSmilingIdiot

AllienWorks added 9 commits May 2, 2019 18:06

Theme: palette update

e450011

Theme: custom tweaks to default theme

67d1703

Theme: custom PART light theme

9df2ca2

Theme: custom PART dark basics

002a3c3

Theme: split into multiple theme files + color palette

ea2c68b

Theming: updated color palettes

672f5f9

Theme: switch back to default

c7a2044

Fix color variables

ae8265f

Remove depreciated theme.scss from styles

c982fae

AllienWorks added 2 commits May 6, 2019 12:44

Theme: remove dark theme

4569186

GFX: small updates

ff4ff20

zaSmilingIdiot merged commit 20ade6d into particl:market-beta May 7, 2019

AllienWorks deleted the gfx/theming branch May 7, 2019 09:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GFX: Theming tweaks #1590

GFX: Theming tweaks #1590

AllienWorks commented May 2, 2019

zaSmilingIdiot commented May 3, 2019 •

edited

AllienWorks commented May 3, 2019

zaSmilingIdiot commented May 3, 2019

AllienWorks commented May 6, 2019

GFX: Theming tweaks #1590

GFX: Theming tweaks #1590

Conversation

AllienWorks commented May 2, 2019

zaSmilingIdiot commented May 3, 2019 • edited

AllienWorks commented May 3, 2019

zaSmilingIdiot commented May 3, 2019

AllienWorks commented May 6, 2019

zaSmilingIdiot commented May 3, 2019 •

edited