Add support for vuetify theming #262

ntma · 2021-10-18T13:32:23Z

This PR allows overriding the default Vuetify theme configuration.

Tasks:

add app context config to override the default theme;
- property baseColor will be replaced by colorTheme;
create toolbar tool to switch between light/dark modes;
write set of default colors (app-config);
- fallback to default if no colors are provided;
remove deprecated color configurations (related to baseColor);
add log message to warn on the deprecated baseColor;
update unit tests;
- baseColor;
- colorTheme;
- darkLayout;
update docs;
- colorTheme property;
- tips on how to pick theme colors;
update locales;

Extras:

expose color classes as CSS variables (docs);
~~allow~~ Vuetify theme presets (material studies).
- requires webpack version >= 4

To improve in the future

solo fields (discussion here);
hover over cards (discussion here);

Material design guidelines

Closes #202

ntma · 2021-10-23T11:28:08Z

Hi guys,

I made the first attempt to colorize both light and dark themes of Wegue. The general idea is that we support both themes without micromanaging the colors throughout the components code.

This is still a work in progress, but I believe it's important that I start sharing the progress so that we start discussing a final set of colors. To whomever check's into this PR, there will be a toolbar control (next to the locales) to switch the themes.

Disclaimer - I am not experienced on styling UI's but this is a skill that I require for my daily work. I don't want to disappoint anyone if the PR doesn't meet the expectations!

Insights on the material colors

Based on material design, Wegue will start using the following colors types to style the UI:

primary - usually the brand color.
on primary - color for text over the primary.
secondary - accent interactive part's of the UI.
on secondary - typically text/icons over secondary.
accent - used mostly in dark mode. Typically, the primary color switches to a dark tone of grey and so we accent details with this color. In my opinion, if the color scheme is simple, this will be just an alias for the secondary color.
surface - color for components such as cards, sheets, menus;
on surface - text/icons over surface.
background - color for scrollable content. Usually we set the html body to this color and build components over it.
on background - text/icons over background.

First set of colors

And so, this is the first set of colors that I came with:

"light": {
  "primary": "#af2622",
  "onprimary": "#ffffff",
  "secondary": "#ec483b",
  "onsecondary": "#ffffff",
  "onsurface": "#bf302d",
  "accent": "#ffffff"
},
"dark": {
  "primary": "#272727",
  "onprimary": "#ffffff",
  "secondary": "#ea9b9b",
  "onsecondary": "#272727",
  "onsurface": "#1e1e1e",
  "accent": "#ea9b9b"
}

Since Wegue was using a tone of red to style the UI, my idea was to use a similar tone (with some variants) for the light theme and then use a lighter shade to accent the dark theme.

Here is a screenshot for each theme:

Pointers

ntma · 2021-10-23T11:36:59Z

Oh, and by the way, the current background layers seem too bright for the dark theme. Is there any dark themed layer that anyone can point me to? It would look cool to switch layers when switching themes 😄

JakobMiksch · 2021-10-25T07:03:54Z

@ntma thanks for your work so far - the screenshot look fancy!
Switching to dark layer on theme change sounds innovative, but should be IMHO more a project specific feature. At least the project I am working on, there would not be a need for this. However, if you need a dark basemap, I can recommend Carto Dark Matter. You can include it as XYZ layer.

ntma · 2021-10-25T08:32:25Z

Switching to dark layer on theme change sounds innovative, but should be IMHO more a project specific feature. At least the project I am working on, there would not be a need for this. However, if you need a dark basemap, I can recommend Carto Dark Matter. You can include it as XYZ layer.

I see. Well, in that case would it be appropriate to add an attribute that identifies which layers to use in dark mode? Material design advises the use of light colors to contrast with the dark background/surface color. I think in some cases, the floating buttons over the map canvas will blend with the bright colors of the background layers.

JakobMiksch

Only a small comment so far

src/locales/de.json

ntma · 2021-10-28T14:35:20Z

Hi guys,

Just made some changes to the wgu-maprecorder-win component so that the record button style fits both light/dark mode. This is easily revertible if anyone disagrees.

Here are some screens:

JakobMiksch · 2021-10-28T14:53:47Z

@ntma the change of the recorder looks fine to me

ntma · 2021-10-29T10:03:25Z

Current setup tested with the following material studies:

Material Study	Config	Validation
Basil	config	✔️ passes
Crane	config	✔️ passes
Fortnightly	config	✔️ passes
Owl	config	❌ fails
Rally	config	❌ fails
Shrine	config	❌ fails

Basil

"theme": {
    "dark": false,
    "themes": {
      "light": {
        "primary": "#356859",
        "onprimary": "#ffffff",
        "secondary": "#FD5523",
        "onsecondary": "#ffffff",
        "onsurface": "#3b7867",
        "accent": "#ffffff"
      },
      "dark": {
        "primary": "#272727",
        "onprimary": "#ffffff",
        "secondary": "#8bcbb2",
        "onsecondary": "#272727",
        "onsurface": "#1e1e1e",
        "accent": "#8bcbb2"
      }
    },
    "options": {
      "customProperties": true
    }
  },

Crane

"theme": {
    "dark": false,
    "themes": {
      "light": {
        "primary": "#5D1049",
        "onprimary": "#ffffff",
        "secondary": "#E30425",
        "onsecondary": "#ffffff",
        "onsurface": "#720D5D",
        "accent": "#ffffff"
      },
      "dark": {
        "primary": "#272727",
        "onprimary": "#ffffff",
        "secondary": "#ce82ab",
        "onsecondary": "#272727",
        "onsurface": "#1e1e1e",
        "accent": "#ce82ab"
      }
    },
    "options": {
      "customProperties": true
    }
  },

Fortnightly

"theme": {
    "dark": false,
    "themes": {
      "light": {
        "primary": "#6c38fb",
        "onprimary": "#ffffff",
        "secondary": "#6c38fb",
        "onsecondary": "#ffffff",
        "onsurface": "#8759fd",
        "accent": "#ffffff"
      },
      "dark": {
        "primary": "#272727",
        "onprimary": "#ffffff",
        "secondary": "#bda2fe",
        "onsecondary": "#272727",
        "onsurface": "#1e1e1e",
        "accent": "#bda2fe"
      }
    },
    "options": {
      "customProperties": true
    }
  },

Owl

"theme": {
    "dark": false,
    "themes": {
      "light": {
        "primary": "#FFDE03",
        "onprimary": "#000000",
        "secondary": "#0336FF",
        "onsecondary": "#ffffff",
        "onsurface": "#f6e900",
        "accent": "#000000"
      },
      "dark": {
        "primary": "#272727",
        "onprimary": "#ffffff",
        "secondary": "#aa9cfe",
        "onsecondary": "#272727",
        "onsurface": "#1e1e1e",
        "accent": "#aa9cfe"
      }
    },
    "options": {
      "customProperties": true
    }
  },

Rally

"theme": {
    "dark": true,
    "themes": {
      "light": {
        "primary": "#1EB980",
        "onprimary": "#ffffff",
        "secondary": "#045D56",
        "onsecondary": "#ffffff",
        "onsurface": "#58c596",
        "accent": "#ffffff"
      },
      "dark": {
        "primary": "#1EB980",
        "onprimary": "#ffffff",
        "secondary": "#045D56",
        "onsecondary": "#ffffff",
        "onsurface": "#58c596",
        "accent": "#ffffff"
      }
    },
    "options": {
      "customProperties": true
    }
  },

Shrine

"theme": {
    "dark": false,
    "themes": {
      "light": {
        "primary": "#FEDBD0",
        "onprimary": "#000000",
        "secondary": "#FEEAE6",
        "onsecondary": "#000000",
        "onsurface": "#ffd0ba",
        "accent": "#ffffff"
      },
      "dark": {
        "primary": "#272727",
        "onprimary": "#ffffff",
        "secondary": "#ffc2ae",
        "onsecondary": "#272727",
        "onsurface": "#1e1e1e",
        "accent": "#ffc2ae"
      }
    },
    "options": {
      "customProperties": true
    }
  },

ntma · 2021-10-29T12:54:58Z

Update for themes that didn't pass the validation before:

Material Study	Config	Validation
Owl	config	✔️ passes
Rally	config	✔️ passes
Shrine	config	➕ ➖ almost

Owl

"theme": {
    "dark": false,
    "themes": {
      "light": {
        "primary": "#FFDE03",
        "onprimary": "#000000",
        "secondary": "#0336FF",
        "onsecondary": "#ffffff",
        "onsurface": "#f6e900",
        "accent": "#000000"
      },
      "dark": {
        "primary": "#272727",
        "onprimary": "#ffffff",
        "secondary": "#faf493",
        "onsecondary": "#272727",
        "onsurface": "#1e1e1e",
        "accent": "#faf493"
      }
    },
    "options": {
      "customProperties": true
    }
  },

Rally

"theme": {
    "dark": true,
    "themes": {
      "light": {
        "primary": "#045D56",
        "onprimary": "#ffffff",
        "secondary": "#1EB980",
        "onsecondary": "#000000",
        "onsurface": "#026d65",
        "accent": "#ffffff"
      },
      "dark": {
        "primary": "#272727",
        "onprimary": "#ffffff",
        "secondary": "#1EB980",
        "onsecondary": "#272727",
        "onsurface": "#1e1e1e",
        "accent": "#1EB980"
      }
    },
    "options": {
      "customProperties": true
    }
  },

Shrine

"theme": {
    "dark": false,
    "themes": {
      "light": {
        "primary": "#FEDBD0",
        "onprimary": "#442C2E",
        "secondary": "#FEEAE6",
        "onsecondary": "#442C2E",
        "onsurface": "#ffd0ba",
        "accent": "#442C2E"
      },
      "dark": {
        "primary": "#272727",
        "onprimary": "#ffffff",
        "secondary": "#ffc2ae",
        "onsecondary": "#272727",
        "onsurface": "#1e1e1e",
        "accent": "#ffc2ae"
      }
    },
    "options": {
      "customProperties": true
    }
  },

ntma · 2021-10-29T14:54:32Z

Hi @JakobMiksch,

I added a few more fixes to account color schemes that use a light color as primary color. The default color for text/icons is white but in this case we want it to be a dark tone for the primary components.

Also added a section to the documentation to explain how to use the "theme" config in the app.config.json. Should I also write a brief tutorial on how to pick colors for the colors classes that Wegue has?

ps: whenever you have some free time, would you please review these two translations? The context of "start" and "stop" is start recording / stop recording.

JakobMiksch

Thanks @ntma
I tried it and it works well. The appearance of the application really benefits from this new scheming approach. See some (mostly cosmetic) remarks inline.

src/locales/de.json

src/components/themeswitcher/ThemeSwitcher.vue

docs/wegue-configuration.md

app-starter/WguAppTemplate.vue

app-starter/static/app-conf.json

app-starter/static/app-conf-minimal.json

JakobMiksch · 2021-11-03T13:55:15Z

Hi @JakobMiksch,

I added a few more fixes to account color schemes that use a light color as primary color. The default color for text/icons is white but in this case we want it to be a dark tone for the primary components.

Also added a section to the documentation to explain how to use the "theme" config in the app.config.json. Should I also write a brief tutorial on how to pick colors for the colors classes that Wegue has?

ps: whenever you have some free time, would you please review these two translations? The context of "start" and "stop" is start recording / stop recording.

It would be great to have a small explanation in the docs how to create color schemes. Or where to generate them.

fschmenger · 2021-11-04T15:41:35Z

Hi @ntma.

First of all this is some awesome work and I'm happy to see you got going!
I finally found some time to look over the code briefly and do a little testing. Here are my findings so far:

I think we should streamline the theming / color related attributes in the component implementations. (@chrismayer This is also a question to the PM) Since we now have multiple colors to manage, 1 configurable color for components is no longer sufficient. My proposal would be to hardwire all the mappings between colors and the theme in the component implementations itself and remove color related component attributes entirely. This will also clean up the app template, which currently is cluttered with piping through color definitions.

This also applies to some specific attributes, e.g.'selColor' in BgLayerList, which are still present but no longer in use. Maybe you could re-check, so there are no unused leftovers.

I'm a little bit confused: Should the toolbarbuttons on the app-bar now appear in the secondary color? I currently don't see this happening. But maybe this is also not desired?!
I realized you changed some of the button shapes, e.g. LocaleSwitcher. The idea behind using toggle groups was to work towards all buttons having the same look and feel (e.g. pressed shape).
What is the status quo about removing the 'darkLayout' module configuration property (corresponds to the 'dark' attribute in the components)? Can this now be handled generically by your new theme switcher? In this context I realized that the panel of the hamburger no longer uses the primary color. Is this something the material design specification asks us to do? The idea behind changing it to the color of the app-bar was, that no dark layout wise destinction has to be made, whether a button is located on the app-bar or hamburger menu.
Maybe you could introduce a backward compability warning that the 'baseColor' configuration property is no longer supported (see migrateAppConfig in main.js)
For merging the configured theme with the default theme you could also use util/Object.js/mergeDeep - however I'm not sure if this make sense here. Maybe it's a better approach to default 'secondary' and 'accent' colors to the 'primary' if the latter are not configured?!
I propose you could add your new settings, e.g. theme switcher to the other application contexts (app-conf-projected and app-conf-projected - but not to app-conf-minimal)

Thats it from me, sorry for the lengthy post.

ntma · 2021-11-04T17:02:42Z

Hi @fschmenger ,

I'll try to make it short 👍

I think we should streamline the theming / color related attributes in the component implementations. (@chrismayer This is also a question to the PM) Since we now have multiple colors to manage, 1 configurable color for components is no longer sufficient. My proposal would be to hardwire all the mappings between colors and the theme in the component implementations itself and remove color related component attributes entirely. This will also clean up the app template, which currently is cluttered with piping through color definitions.

Yes we can do this. I wasn't sure at which level Wegue should control it's colors so I tried to set the control to the top level: appHeader, appFooter and the map.

2.` I'm a little bit confused: Should the toolbarbuttons on the app-bar now appear in the secondary color? I currently don't see this happening. But maybe this is also not desired?!

I am not sure if you are referring to the panel of the hamburger menu in the app-bar. If so, the answer to 4. may also answer this question.

I realized you changed some of the button shapes, e.g. LocaleSwitcher. The idea behind using toggle groups was to work towards all buttons having the same look and feel (e.g. pressed shape).

Hmm, but the LocaleSwitcher functions as a drop-down menu while the others (e.g: Recorder) activate a feature. That's the logic that I was following. I was just trying to give an extra and of course we can revert this change at any time 👍

What is the status quo about removing the 'darkLayout' module configuration property (corresponds to the 'dark' attribute in the components)? Can this now be handled generically by your new theme switcher? In this context I realized that the panel of the hamburger no longer uses the primary color. Is this something the material design specification asks us to do? The idea behind changing it to the color of the app-bar was, that no dark layout wise destinction has to be made, whether a button is located on the app-bar or hamburger menu.

The darkLayout is now controlled generically by Vuetify. In my perspepctive, the baseColor and darkLayout acted like twins in the component configuration. If we streamline one, the other should go with it too. If this isn't desired, we can revert the change.

Regarding the hamburger menu and knowing that:

primary color should be used in the main components (header, footer, brand logo);
secondary color should be used to accent controls;
surface color should be used as the background of panels;

then the panel should have a surface color as background with a secondary color for the buttons.

There is a short insight about this topic in the Primary Colors, Secondary Colors and Surface Colors sections of this material design doc.

Maybe you could introduce a backward compability warning that the 'baseColor' configuration property is no longer supported (see migrateAppConfig in main.js)

Sure thing, I will add this.

For merging the configured theme with the default theme you could also use util/Object.js/mergeDeep - however I'm not sure if this make sense here. Maybe it's a better approach to default 'secondary' and 'accent' colors to the 'primary' if the latter are not configured?!

Interesting, I didn't know about the mergeDeep. Either way, seems a diplomatic solution 👍 My concern is that a user only sets colors for the light theme. In which case the dark theme will fallback to the default blue of Vuetify...

I propose you could add your new settings, e.g. theme switcher to the other application contexts (app-conf-projected and app-conf-projected - but not to app-conf-minimal)

Sure, I will do this.

chrismayer · 2021-11-05T09:43:38Z

Hi @ntma, big thanks for your ongoing work on this. Looks very very promising 👍

I agree that if we introduce theming by vuetify everything should be handled by it. Concurrent approach by having some props set in the app-config outside of the theming config block would cause confusion.

Also configuring should remain easy. So the already mentioned default mechanism is quite important. Users should be able to configure only a subset of theming props or even nothing. Otherwise users/admins and up in config hell, if the need to configure everything.

fschmenger · 2021-11-08T11:35:43Z

Hi @ntma, thanks for the clarifications and links to the notable sources.

Since we have a response from the PM, I think we're good to finalize the topics mentioned in 1) and 4) and control colors at a theme level only. As a summary

Please remove the darkLayout property from app-configs. For the color wise distinction between a button being located on the app-bar or hamburger menu (I will ask the PM for cofirmation further below), you could possible do something like this in AppHeader.vue::getModuleButtons

     getModuleButtons (target) {
      const appConfig = Vue.prototype.$appConfig || {};
      const modulesConfs = appConfig.modules || {};
      let buttons = [];
      for (const key of Object.keys(modulesConfs)) {
        const moduleOpts = appConfig.modules[key];
        if (moduleOpts.target === target) {
          buttons.push({
            type: moduleOpts.win ? 'wgu-toggle-btn' : key + '-btn',
            moduleName: key,
            // Relay the parent control to the button for coloring.
            // Or alternatively compute a 'color' property here based on target==='toolbar',
            // this will avoid redundancies in the component implementations until we have an abstraction for all toolbarbuttons - see #260
            target: target, 
            ...moduleOpts
          });
        }
      }
      return buttons;
    }

Otherwise remove color (this also involves similar color related attributes and default color values ) and dark props from the components.
Remove all other references/initializations to the properties described above from the app-templates.
Update documentation and unit tests accordingly.

Regarding 2) I misread the code ;). I can see you are using the primary color for all the buttons on the app-bar now, which is what I expected.

Otherwise I can see you're pretty much there!

@chrismayer The remaining questions to the PM:
Regarding 4) Are we good to change the background color of the hamburger menu back to 'surface color' ? This is probably a bit opionated, as the hamburger menu is a special dropdown which can also be seen as an extension to the app-bar (and also requires some coloring switches in the code). However I can see the point of Nuno's recommendation.
Regarding 3) You should make a clear ruling when buttons should have a toggle/pressed state. See Nuno's comment above.

ntma · 2021-11-08T15:55:59Z

Hello all,

@chrismayer Glad that the PR is meeting the expectations! 👍 I will try my best to finish it as soon as possible, since I will be a bit busy for the next days.

@fschmenger

Hi @ntma, thanks for the clarifications and links to the notable sources.

Cheers. Feel free to ask if any future change raises some suspicion!

Since we have a response from the PM, I think we're good to finalize the topics mentioned in 1) and 4) and control colors at a theme level only.

I will advance to this change then 👍

Regarding 2) I misread the code ;). I can see you are using the primary color for all the buttons on the app-bar now, which is what I expected.

No problem! I was all over the code for this PR so some changes may not be that clear.

fschmenger · 2021-11-10T13:02:08Z

@ntma I just scrolled over your changes. Could you elaborate on, why the primary color is mandatory for the light theme, and the secondary color for the dark theme? This asymetric approach is a little hard to understand, don't you think :)

Otherwise good job implementing all the fallbacks. Will give it a test run soon.

ntma · 2021-11-10T14:01:43Z

@fschmenger Indeed, thanks for noticing that!

I had it in my notes but eventually forgot to commit that explanation.

…u-maprecorder-win

… for v-select/v-combobox

JakobMiksch · 2021-12-02T14:50:11Z

thanks for @ntma and @fschmenger for the ongoing work. I did not follow all the recent details anymore. Let me know if you need an additional review. Otherwise I trust the approval of the others.

ntma · 2021-12-09T16:51:46Z

Hi guys, I am back!

So, to reply some of the pending questions/reviews from @fschmenger:

The dark theme color is already too dark for the box-shadow to take any effect. I think this will hit us in many situations. So I agree with you, that a specific fix at this place is just not worth it. We could either think about making the dark theme default colors just a bit lighter or just ignore the problem at all and see if a future vuetify version has a generic fix for that. Probably just roll it back for now ?!

I think the problem will persist even if we change the dark theme tones, as the hovering shadow will conflict with the dark shades. I could ignore the issue for now but I feel that Vuetify (like many other UI frameworks) are in a lot of pressure to release their stable version for Vue3. Details like this are probably not their priority right now 😞 Should I still revert the changes?

Regarding the fix introduced for ToggleButton in b1e7684.
Doesn't it work much simpler, if you just assign the color property to the v-btn in favor of the css rule?

Indeed, nice catch! One more caveat for Vuetify 👍 I will change it to your suggestion!

EDIT: Since the specific UI design of the 2 controls is subject to discussion / change in the future (see #243 and #253) I would rather opt for a simplistic and fairly straightforward - rather than perfect - solution here.

Regarding the solo fields, since we are not opting for the near perfect solution (and I totally agree with this decision), should this field take the same color as the header color?

I wonder if you like it, it's a little more simplistic. If so I can either provide this as a follow up PR or suggest as a code change and we get it incorporated in this run.

I agree with the infoclick change. Having custom components converted to Vuetify components will simplify the material theming 👍

fschmenger · 2021-12-13T09:24:56Z

Hi @ntma,
thanks for the investigation and the feedback.

I think the problem will persist even if we change the dark theme tones, as the hovering shadow will conflict with the dark shades. I could ignore the issue for now but I feel that Vuetify (like many other UI frameworks) are in a lot of pressure to release their stable version for Vue3. Details like this are probably not their priority right now 😞 Should I still revert the changes?

I'd say yes. Sorry for bringing up the issue at all. But lately I had time to look into it and noticed that this is a general vuetify issue with dark themes. I think we have to live with hovering effects being lost for dark tones for now. IMO having a straightforward code should be prefered over a specific fix, that won't remove the problem at global scope.

Regarding the solo fields, since we are not opting for the near perfect solution (and I totally agree with this decision), should this field take the same color as the header color?

Did you check the snippet I proposed. I couldn't see the drawback while it looked to me like we're retaining the accent colors. Or am I missing something?

fschmenger · 2021-12-13T09:38:19Z

One last thing from my part, that I cannot see being finally adressed in the discussion above.

Regarding 4) Are we good to change the background color of the hamburger menu back to 'surface color' ? This is probably a bit opionated, as the hamburger menu is a special dropdown which can also be seen as an extension to the app-bar (and also requires some coloring switches in the code). However I can see the point of Nuno's recommendation.

Again i can only point out, that for my personal taste I would opt for the simplest code-wise solution, and use the primary color for the hamburger menu (especially as it seems to me as this is not explicitly discouraged by the MD spec). I propose @chrismayer should make a clear ruling on this and then we go with whatever he has to say.

ntma · 2021-12-13T17:22:51Z

Hi @fschmenger ,

I'd say yes. Sorry for bringing up the issue at all. But lately I had time to look into it and noticed that this is a general vuetify issue with dark themes. I think we have to live with hovering effects being lost for dark tones for now. IMO having a straightforward code should be prefered over a specific fix, that won't remove the problem at global scope.

It's totally fine. I understand that reaching a future proof solution isn't always easy 🙂 I'll revert the changes in the next commit.

Did you check the snippet I proposed. I couldn't see the drawback while it looked to me like we're retaining the accent colors. Or am I missing something?

I did. The main difference is the 3rd elevation added by the 3rd dark shade (docs about material elevation here). Here are two visuals (first with solo and second without), where the red numbers represent the three elevations.

With solo:

Without solo:

Elevation is essential to differentiate components (specially in the dark mode). Lighter shades will simulate positive elevation while darker shades will simulate negative.

In my point of view, solo fields should that a negative elevation (in respect to their parent components). Although they are components that should stand out, they are at the same time recipients for user input. And thus should look like an embedded field into it's parent component. Hence my effort on trying to make it look like that 😁

fschmenger · 2021-12-14T09:14:51Z

Hi @ntma, thanks for the screenshots regarding the solo fields - I can spot the difference now :)

I'm a bit undecided whether this is a good feature for the following reasons:

Wegue didn't have any concept on accecting those fields over a primary surface prior to this PR (I think this has been mentioned somewhere above)
The implementation is currently limited to 2 controls, that may be subject to changes in the future.
Last but not least the solution is not very maintainable. If we we're to add future controls over a primary surface, then these would require the same code switches and most importantly the same css rules that you introduced in wegue.css.

Overall to me the feature looks too minor to justify a bunch of specific code.
I leave the final decision to the meggsimum guys - @chrismayer, @JakobMiksch what's your opinion on this?

IMO, if we decide to keep this in, then for the sake of maintainance you should probably wrap up the wegue.css rules by a class and assign it to the respective controls.

chrismayer · 2021-12-15T09:37:10Z

Hi @ntma,

thanks for your ongoing work and your gigantic patience with the huge amount of remarks, discussions and suggestions by the reviewers. I really appreciate that you're still on this 👍 But to not overstretch you patience 😉 and to get this killer feature into the Wegue code base we should shorten this a bit up. Therefore I propose the following:

We should go the simplest approach for open remarks/request to get them quickly done. For the hamburger menu we should use using primary color (simplest approach? Correct me if I am wrong).
For solo fields I have no strong opinion, please also go a simple, fast and acceptable way here.
All other (hopefully minor) things, which can be improved in the future we should live with for now. These should be formulated in issues, so they are not forgotten. Another advantage of the small splittet issues would be having focused and isolated discussions. To follow the discussion thread in this PR is nearly impossible.

So @ntma would you please do a last round of providing final adaptions, if necessary? Would be very appreciated. Afterwards give us a ping that this is finalized.
@fschmenger Would you give afterwards a last check regarding heavy bugs or blockers? If none I'd happily merge this and we proceed from there.

Again thank you all (especially @ntma and @fschmenger) for your huge work on this!

ntma · 2021-12-17T11:10:45Z

Hi @chrismayer ,

Glad that you appreciate the effort done in this PR! 😁

So @ntma would you please do a last round of providing final adaptions, if necessary? Would be very appreciated. Afterwards give us a ping that this is finalized.

I did the last round and:

the hamburger menu is now primary;
solo fields are now filled fields. The visual difference can be seen in this comment. In the end, having a 3rd elevation may not look that bad as long as this property is uniform across all fields over primary colors. Unfortunately this nasty override is still required for the cases where we override the "onprimary" color (ie: app-conf-projected.json).

As a last remark, this PR adds the ability of using the Vuetify color classes as CSS variables (docs here). I didn't test it but I believe these variables can be used to style the map features if required.

And with that done, I conclude this work. Thank you all for the patience on reviewing this PR. If there is still something that I missed or requires changes, please let me know 👍

fschmenger · 2021-12-20T09:06:34Z

Hi @ntma, thanks for your ongoing effort.

I can see 2 finalization tasks:

In conjunction with e8b4760: Could you please remove the following line https://github.com/meggsimum/wegue/blob/43eb1375c9251aeca9f33cc280863c32b5e87302/app-starter/components/AppHeader.vue#L112 and the associated color props from the components/ buttons, so we are consistent with hardwiring of colors on component level.
Are the specific css rules still required since 43eb137? Please either remove them or see my comment above for attaching them to a css class (e.g. 'wgu-select'). https://github.com/meggsimum/wegue/blob/43eb1375c9251aeca9f33cc280863c32b5e87302/src/assets/css/wegue.css#L42-L62

Otherwise this looks good to me!

ntma · 2021-12-20T10:52:05Z

Hi @fschmenger,

In conjunction with e8b4760: Could you please remove the following line and the associated color props from the components/ buttons, so we are consistent with hardwiring of colors on component level.

Makes sense. Done!

Are the specific css rules still required since 43eb137? Please either remove them or see my comment above for attaching them to a css class (e.g. 'wgu-select').

Unfortunately yes. As far as I know, there is no simple way of overriding the text/border/input color in case the field is over a primary color. The CSS overrides are now wrapped into .wgu-solo-field.

Cheers 👍

fschmenger · 2021-12-20T14:43:44Z

Hi @ntma, thank you for the prompt response and fixes.

Unfortunately yes. As far as I know, there is no simple way of overriding the text/border/input color in case the field is over a primary color. The CSS overrides are now wrapped into .wgu-solo-field.

I'm a little bit confused again, as I can't see the rule taking effect now - why the name of the css class as the associated components are now no longer solo? I asked my DOM inspector to point me to the difference but never saw the theme--light assigned (the wgu-solo-fields always have theme--dark in the class list, even when I toggle the dark mode).
Maybe you can point me to the difference again?

EDIT: After reading over your comment again, I can see the effect for the app-conf-projected. Regard it as resolved :)

fschmenger · 2021-12-20T15:35:06Z

@chrismayer I reviewed the code and ran some additional integration tests on my app with it's specific theme. I think this PR is as good as it can get, so feel free to merge.

@ntma Thanks a lot for going through all the proposals and all the time and energy you invested into this. I'm looking forward to work with you on some topic again in the future! 👍

ntma · 2021-12-21T13:24:56Z

@ntma Thanks a lot for going through all the proposals and all the time and energy you invested into this. I'm looking forward to work with you on some topic again in the future! 👍

Anytime! If some issue arises from this PR in the future, I don't mind taking a look into it 👍

JakobMiksch

Fantastic work.
I tried it again and I love it!
I also tried the built version produced by npm run build and it seems to work well.
Ready to merge from my side!

chrismayer · 2021-12-23T07:18:36Z

THIS IS GREAT! 🚀

Thanks for your fantastic and tireless work @ntma. Very much appreciated. I hope you stay part of the Wegue community. Would be a great benefit for us all.

Also big thanks to @fschmenger for managing this and for the endless reviews. And of course thanks to @JakobMiksch for your input.

I am going to merge now. 🎉

ntma · 2021-12-28T09:51:51Z

Thanks for your fantastic and tireless work @ntma. Very much appreciated. I hope you stay part of the Wegue community. Would be a great benefit for us all.

Anytime! I'll try to contribute more whenever I find myself with some spare time 👍

JakobMiksch reviewed Oct 25, 2021

View reviewed changes

src/locales/de.json Outdated Show resolved Hide resolved

JakobMiksch reviewed Nov 3, 2021

View reviewed changes

ntma added 11 commits November 11, 2021 11:15

Added app context config for vuetify theming

427829a

Added toolbar button to control light/dark modes

717f4b5

Initial attempt to colour both light and dark themes

9ad9732

Lingual adjustments (locales/de.json)

915ce9b

Light/dark theme for wgu-attributetable-win | wgu-geocoder-input | wg…

586fc6c

…u-maprecorder-win

Applying orange tone for errors

4853d88

Using onprimary to colorize typography/icons over primary panels

0cc6e87

Documentation for theme configuration | revert dark property as input…

c53336a

… for v-select/v-combobox

Missing locales (de)

d6b6d75

changed 'theme' property to 'colorTheme'

a4ffa8d

Linguistic adjustments

287d47c

Nuno Amorim added 3 commits December 9, 2021 17:14

Better fix for ToggleButton hover effect (suggested by @fschmenger)

5e6cb3f

Minor linguistic correction

0bbc9b1

Updated unit tests to the latest changes

d80e88a

Nuno Amorim added 2 commits December 14, 2021 10:02

Revert changes for hovering BgLayerList

d215fe7

Removed unused unit test for css colors on ol buttons

1ba71d1

fschmenger mentioned this pull request Dec 16, 2021

Generalized feature hover #269

Merged

Nuno Amorim added 2 commits December 17, 2021 09:55

Set primary to the hamburguer menu

e8b4760

Simplify solo fields (using immutable filled property)

43eb137

ntma changed the title ~~WIP: Add support for vuetify theming~~ Add support for vuetify theming Dec 17, 2021

Nuno Amorim added 3 commits December 20, 2021 10:18

Moved color property to be fully controlled by each component

5ffd994

Wrapped solo fields CSS overrides into .wgu-solo-field

110d1e4

Updated unit tests for ThemeSwitcher

d2a2931

JakobMiksch approved these changes Dec 21, 2021

View reviewed changes

chrismayer merged commit faca708 into wegue-oss:master Dec 23, 2021

fschmenger mentioned this pull request Jan 18, 2022

Infoclick vuetify tables #278

Merged

Add support for vuetify theming #262

Add support for vuetify theming #262

Conversation

ntma commented Oct 18, 2021 • edited Loading

Tasks:

Extras:

To improve in the future

Material design guidelines

ntma commented Oct 23, 2021 • edited Loading

Insights on the material colors

First set of colors

Pointers

ntma commented Oct 23, 2021

JakobMiksch commented Oct 25, 2021

ntma commented Oct 25, 2021

JakobMiksch left a comment

Choose a reason for hiding this comment

ntma commented Oct 28, 2021

JakobMiksch commented Oct 28, 2021

ntma commented Oct 29, 2021 • edited Loading

Basil

Crane

Fortnightly

Owl

Rally

Shrine

ntma commented Oct 29, 2021

Owl

Rally

Shrine

ntma commented Oct 29, 2021 • edited Loading

JakobMiksch left a comment

Choose a reason for hiding this comment

JakobMiksch commented Nov 3, 2021 • edited Loading

fschmenger commented Nov 4, 2021 • edited Loading

ntma commented Nov 4, 2021 • edited Loading

chrismayer commented Nov 5, 2021 • edited Loading

fschmenger commented Nov 8, 2021

ntma commented Nov 8, 2021 • edited Loading

fschmenger commented Nov 10, 2021

ntma commented Nov 10, 2021

JakobMiksch commented Dec 2, 2021

ntma commented Dec 9, 2021

fschmenger commented Dec 13, 2021

fschmenger commented Dec 13, 2021

ntma commented Dec 13, 2021

With solo:

Without solo:

fschmenger commented Dec 14, 2021

chrismayer commented Dec 15, 2021 • edited Loading

ntma commented Dec 17, 2021

fschmenger commented Dec 20, 2021

ntma commented Dec 20, 2021

fschmenger commented Dec 20, 2021 • edited Loading

fschmenger commented Dec 20, 2021

ntma commented Dec 21, 2021

JakobMiksch left a comment

Choose a reason for hiding this comment

chrismayer commented Dec 23, 2021

ntma commented Dec 28, 2021

ntma commented Oct 18, 2021 •

edited

Loading

ntma commented Oct 23, 2021 •

edited

Loading

ntma commented Oct 29, 2021 •

edited

Loading

ntma commented Oct 29, 2021 •

edited

Loading

JakobMiksch commented Nov 3, 2021 •

edited

Loading

fschmenger commented Nov 4, 2021 •

edited

Loading

ntma commented Nov 4, 2021 •

edited

Loading

chrismayer commented Nov 5, 2021 •

edited

Loading

ntma commented Nov 8, 2021 •

edited

Loading

chrismayer commented Dec 15, 2021 •

edited

Loading

fschmenger commented Dec 20, 2021 •

edited

Loading