canonical / vanilla-framework

From community websites to web applications, this CSS framework will help you achieve a consistent look and feel.
https://vanillaframework.io
GNU Lesser General Public License v3.0
798 stars 163 forks source link

Update application layout to new theming #5182

Closed pastelcyborg closed 23 hours ago

pastelcyborg commented 1 week ago

Done

Fixes WD-11869

QA

Check if PR is ready for release

If this PR contains Vanilla SCSS code changes, it should contain the following changes to make sure it's ready for the release:

Screenshots

In lieu of screenshots, would recommend pulling the branch and testing manually; would be numerous screenshots to upload here.

webteam-app commented 1 week ago

Demo

Jenkins

demos.haus

pastelcyborg commented 1 week ago

@jmuzina @bartaz Marked this as ready for review, but did want to call out a few open questions I have:

bartaz commented 1 week ago

Looks like this has blown in scope from "update the colours to be themed" to "refactor application layout structure", which makes is a bit harder to review.

Would it be possible to make minimal colour changes separately (just to make sure that any colours used by application layout are themed), and leave the rest of it to be done separately? Any structure changes need to be backwards compatible, so we have to do them with care.

jmuzina commented 1 week ago

Borders on the structure example seem to have a bit too much contrast, unless this is intended: image

pastelcyborg commented 1 week ago

Borders on the structure example seem to have a bit too much contrast, unless this is intended: image

So these borders are part of the demo CSS file, rather than using any particular part of Vanilla, so theming doesn't apply. I'm open to ideas (or otherwise we could just remove them entirely).

bartaz commented 1 week ago

Overall, the structure example is just a structure, it's not about styling at all. So theming doesn't matter to it. We could (and probably should) disable the theming on it completely.

The important bit is if we got rid of any direct colour variables from application/panels code.

pastelcyborg commented 1 week ago

After discussing, we've decided to keep some of the important elements on the structure example (for instance, panel), but get rid of any unnecessary ones. We concluded it's important to represent a holistic version of application layout, since the structure without any panels isn't particularly useful to end users. Will make these changes.

pastelcyborg commented 6 days ago

This is ready for another review since I've stripped down the structure example even further.

bartaz commented 1 day ago

The color of the scrollbar for the sidebar (I've been testing Split) seems to change depending on light/dark theme, despite the sidebar itself always being dark-theme. This makes it particularly hard to see the scrollbar on light-theme.

We haven't been theming (or otherwise altering) native browser scrollbars, so I wouldn't consider it a part of this PR.

If there are any particular issues I'd raise them separately, we would need to have a separate proper look on how theming affects scrollbars across variety of browsers and OSes (and if we can control it in any way).