Migrate SCSS variables to CSS3 to enable auto dark and more features #429

DRSDavidSoft · 2024-10-28T02:25:39Z

Description

This is a preliminary PR to address #428. Changes so far include:

Converted all SCSS variables to CSS variables instead where required (no scss variable remained after this change)
Drop IE support (eliminating the need for base64 encoding of SVG images, reducing both file clutter and size)
General cleanup to improve consistency across themes

Note: This PR currently doesn't introduce visual changes. If there are any differences from the previous styles, please flag them. I’ve reviewed this but would appreciate a thorough check.

Additional Requests:

I used d($_SERVER); to verify theme consistency. But it might not output use of all the Kint features, so could you please prepare a script for me that would make use of all the Kint features for a detailed comparison between the previous and old styles?
For theme files, we currently have separate files for solarized-light and solarized-dark. Ideally, we should consolidate to a single solarized.css file with color variables adaptable to both themes. Users might prefer explicitly setting light or dark themes, though, and we should avoid disrupting existing user settings. I propose a “theme alias” feature in Kint, mapping old themes to a unified auto theme while applying kint-dark or kint-light classes as appropriate. This would allow us to ship just one file instead of three. Let me know if you agree, and I’ll work on a PR.
Once decisions are finalized, another PR could be opened to refresh theme colors using professionally curated palettes in preparation for the Kint 6 release.

Theme Variable Enhancements

Added CSS variables for spacing, colors, and design elements for easier customization and theme management.

Simplification and Consistency

Replaced hardcoded values with CSS variables in order to enable declarative overriding (e.g. using @media (prefers-color-scheme: dark) query)
Simplified complex selectors and properties to improve readability and maintainability.

The changes will help in maintaining a consistent design and make future modifications more straightforward.

Tests

Verified the produced css in the compiled directory against the older versions by setting the following:

RichRenderer::$theme = 'aante.css';

To ensure new styles exactly match previous themes.

jnvsor · 2024-10-28T07:32:39Z

Good work

I propose adding a "theme alias" feature in Kint which is very simple, it would map the old theme files to the new auto theme, while also applying a kint-dark or kint-light class to the renderer.

Hmm I'd rather apply it to the body since there's only supposed to be "1 true stylesheet" for kint in a page. Are there any fancy frontend systems you know of that could break that? (ie. react)

I'll review this today

jnvsor

Additionally a weird rendering artifact only occurs on firefox not chrome:

I'll try to debug this myself

resources/sass/aante.scss

resources/sass/_base.scss

resources/sass/aante-dark.scss

resources/sass/aante.scss

jnvsor · 2024-10-28T08:03:02Z

resources/sass/original.scss

@@ -1,27 +1,30 @@
 @import "base";

+:root {


Since these are specific to original I deliberately didn't bother putting them in variables. Maybe 1 and 2 should be since they're used more than once...

In that case what do you think of naming them --gradient-start/--gradient-end?

Exactly, I just named them color1 and color2 as a temporary placeholder.

I prefer to use gradient-start and gradient-end, just that since there are two gradients used here, maybe they can be named primary-gradient-[start|end] and secondary-gradient-[start|end], I just didn't know which was which when I did it.

Alternatively, would be to make the entire gradient a variable instead. Then, instead of using CSS to apply them to the elements in question, the newly created --background-image variable can be used instead.

There are 2 gradients but color3 color4 and highlight-color are only used once so you might as well make them literals

The main purpose for using vars instead of literals is being able to override them in the TBA dark theme variant of this theme.

resources/sass/_base.scss

DRSDavidSoft · 2024-10-28T11:27:40Z

Hmm I'd rather apply it to the body since there's only supposed to be "1 true stylesheet" for kint in a page.

Good call, so setting the theme to an alias aante-dark will actually load the aante theme, and apply kint-dark class to the body itself

Are there any fancy frontend systems you know of that could break that? (ie. react)
I don't think there would be a problem, although I don't think React would be used this way with PHP.

If a request from React is made to a PHP backend (e.g. Using Axiom), then it's most likely a "fetch" type request (à la ajax) and won't be directly rendered in the page.

There is one problem that comes to mind, which is that we will need to declare our styles as .kint-dark .kint-rich { ... } instead of .kint-rich.kint-dark { ... }. Actually, not a real problem, just something to have in mind.

jnvsor · 2024-10-28T11:35:10Z

Good call, so setting the theme to an alias aante-dark will actually load the aante theme, and apply kint-dark class to the body itself

If a request from React is made to a PHP backend (e.g. Using Axiom), then it's most likely a "fetch" type request (à la ajax) and won't be directly rendered in the page.

Yeah but you can't set a class on the body from kint in PHP, would have to be in JS. I can imagine different dumps with the light and dark version toggling the class on the body which is not what we want I presume.

Maybe it should be a class like .kint-rich.kint-dark but that wouldn't work for the folder... I guess we could just have JS store the dark flag of first dump moved to the folder and use that for the folder class

DRSDavidSoft · 2024-10-28T11:50:41Z

Maybe it should be a class like .kint-rich.kint-dark but that wouldn't work for the folder... I guess we could just have JS store the dark flag of first dump moved to the folder and use that for the folder class

If I am not mistaken, the folder functionality already makes use of JS, right? If so, it makes sense for PHP to set the kint-dark class on the kint-rich element, then when the folder feature is initialized, using JS we will check whether this class if set, and if so, copy it to the folder element.

I can imagine different dumps with the light and dark version toggling the class on the body which is not what we want I presume.

I wouldn't say that it would make sense, but I believe if the user can set the theme to be anything for each individual dump, it would make sense that they could also turn the dark mode on/off by specifying another "theme alias".

In conclusion, this makes more sense to me when I think about it, rather than setting kint-dark on the body itself.

you can't set a class on the body from kint in PHP, would have to be in JS.

Quite right, the closest thing I can imagine is having a parent .kint-dark element, but none of this makes sense as opposed to just applying the kint-dark class directly on kint-rich element.

I guess functionality speaking, it doesn't make sense to have per-dump dark control, so a "global" dark mode control approach would've been nice. But technically speaking, setting it on the whole page doesn't make sense, and setting it on each individual dump makes more sense.

jnvsor · 2024-10-28T11:54:13Z

If I am not mistaken, the folder functionality already makes use of JS, right? If so, it makes sense for PHP to set the kint-dark class on the kint-rich element, then when the folder feature is initialized, using JS we will check whether this class if set, and if so, copy it to the folder element.

Yeah that's exactly what I meant. Let's go with that (I can work on the php/js side) once this migration is done

jnvsor

Sorry I didn't press the big green button

resources/sass/_base.scss

jnvsor · 2024-11-08T17:10:26Z

@DRSDavidSoft any movement on this? 8.4 is now scheduled to release on the 21st so we have a little under 2 weeks.

I'd like to push an RC by the end of this weekend, but I still have more changes to make before then.

If you can't make it by then just take your time and we'll aim for 6.1

DRSDavidSoft · 2024-11-09T01:03:51Z

@jnvsor I was just a bit busy at work, if possible I would like to focus the PR only at migrating the variables for now to keep it simpler and allow us the time for it to be merged in 6.0. Then, I will work on another PR to address the auto dark mode and the new icons. I hope that's also fine with you.

If so, can you please make a list of the changes that I need to apply for this PR to be merged? That would help me prioritize the changes as well as allowing #428 to be implemented in a later PR that will be based on this one.

jnvsor · 2024-11-09T07:48:30Z

Yeah that works for me

I'll just resolve everything and make a new review then

jnvsor · 2024-11-09T07:53:47Z

resources/sass/aante-dark.scss


-$variable-type-color: invert(#06f);
-$variable-type-color-hover: invert(#f00);
+:root {


Put vars in .kint-rich (In all scss files)

In most children extending _base they already have a .kint-rich block so merge them rather than duplicating them

I applied this change, although I believe there could be some benefits for using the :root space, particularly if we use a short and concise prefix such as --kt-.

Would you prefer the following declarations? Otherwise, I'll put the variables in .kint-rich as requested.

:root { --kt-variable-type-color: ... --kt-variable-type-color-hover: ... }

This would allow the users to easily override Kint themes using custom values later on.

Nah just leave it in .kint-rich - it's just as hard for a user to write

.kint-rich { --variable-type-color: #F00; }

As

:root { --kt--variable-type-color: #F00; }

Actually what I mean is the ability to override the variable, but sure, I submitted the PR using the .kint-rich instead of :root

Note: using the .kint-rich approach, the user have to supply a more qualifying css selector in order to override their variables. e.g. add kint-custom class to the body, then insert overriding vars as .kint-custom .kint-rich, hopefully I was able to demonstrate my point.

You can also do this with :root .kint-rich without touching the HTML, but even with that it's not a downgrade.

Before you had to style individual elements since you didn't have access to variables at all. Now you can still do it without more specific selectors if you make sure your css comes after kints (Which is admittedly pretty hard to do but easy enough for actual kint themes)

resources/sass/aante-light.scss

resources/sass/original.scss

resources/sass/_base.scss

jnvsor · 2024-11-09T08:12:00Z

resources/sass/_base.scss

    background: {
      color: #ccc;
      image: url('data:image/svg+xml;utf8,<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 2 2"><path fill="%23FFF" d="M0 0h1v2h1V1H0z"/></svg>');
-      size: if($color_size >= 30, 30px, 100%);
+      size: 100%;


I'm not sure these changes won't break some layouts but I'll merge them first and test them myself later

I don't see why the new approach would break anything, but we could also go this route as well:
size: min(30px, 100%)
which has the same effect.

Alternatively, if you found a breaking change with either of these CSS-only implementations, we can revert back to the SCSS solution.

jnvsor · 2024-11-11T06:59:53Z

I'm off to work now, I'll review tweak and merge later today

jnvsor · 2024-11-11T17:55:08Z

There's a few small things but I'll just take care of them here. For now could you hold off on making any more changes until I've pushed? Otherwise we're just going to cause conflicts

jnvsor · 2024-11-11T20:17:42Z

Merged in 63fcf33 with some tweaks after that.

The CSS could really use some love. I noticed the .kint-parent doesn't have a fixed height it's just flowing with the content. That could potentially be causing client-side perf issues.

Anyway I'll leave CSS here and push rc1 when I'm done with my changes

DRSDavidSoft marked this pull request as draft October 28, 2024 02:25

DRSDavidSoft force-pushed the css-updates branch 4 times, most recently from 5b08590 to c8d9c63 Compare October 28, 2024 02:35

jnvsor requested changes Oct 28, 2024

View reviewed changes

DRSDavidSoft mentioned this pull request Oct 28, 2024

Remove outdated CSS vendor prefix #430

Merged

DRSDavidSoft force-pushed the css-updates branch from c8d9c63 to 8c1a9dd Compare October 28, 2024 11:30

jnvsor requested changes Oct 28, 2024

View reviewed changes

resources/sass/_base.scss Show resolved Hide resolved

resources/sass/_base.scss Show resolved Hide resolved

resources/sass/_base.scss Outdated Show resolved Hide resolved

resources/sass/_base.scss Outdated Show resolved Hide resolved

jnvsor requested changes Nov 9, 2024

View reviewed changes

DRSDavidSoft force-pushed the css-updates branch from 8c1a9dd to 8595632 Compare November 11, 2024 06:53

[WIP] migrate from scss variables to css vars instead

7b0b042

DRSDavidSoft force-pushed the css-updates branch from 8595632 to 7b0b042 Compare November 11, 2024 07:36

DRSDavidSoft marked this pull request as ready for review November 11, 2024 07:36

jnvsor approved these changes Nov 11, 2024

View reviewed changes

jnvsor closed this Nov 11, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate SCSS variables to CSS3 to enable auto dark and more features #429

Migrate SCSS variables to CSS3 to enable auto dark and more features #429

DRSDavidSoft commented Oct 28, 2024 •

edited

Loading

jnvsor commented Oct 28, 2024

jnvsor left a comment

jnvsor Oct 28, 2024

DRSDavidSoft Oct 28, 2024

jnvsor Oct 28, 2024

DRSDavidSoft Nov 11, 2024

DRSDavidSoft commented Oct 28, 2024

jnvsor commented Oct 28, 2024

DRSDavidSoft commented Oct 28, 2024

jnvsor commented Oct 28, 2024

jnvsor left a comment

jnvsor commented Nov 8, 2024

DRSDavidSoft commented Nov 9, 2024 •

edited

Loading

jnvsor commented Nov 9, 2024

jnvsor Nov 9, 2024

DRSDavidSoft Nov 11, 2024

jnvsor Nov 11, 2024

DRSDavidSoft Nov 11, 2024 •

edited

Loading

jnvsor Nov 11, 2024

jnvsor Nov 9, 2024

DRSDavidSoft Nov 11, 2024

jnvsor commented Nov 11, 2024

jnvsor commented Nov 11, 2024

jnvsor commented Nov 11, 2024 •

edited

Loading

Migrate SCSS variables to CSS3 to enable auto dark and more features #429

Migrate SCSS variables to CSS3 to enable auto dark and more features #429

Conversation

DRSDavidSoft commented Oct 28, 2024 • edited Loading

Description

Theme Variable Enhancements

Simplification and Consistency

Tests

jnvsor commented Oct 28, 2024

jnvsor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DRSDavidSoft commented Oct 28, 2024

jnvsor commented Oct 28, 2024

DRSDavidSoft commented Oct 28, 2024

jnvsor commented Oct 28, 2024

jnvsor left a comment

Choose a reason for hiding this comment

jnvsor commented Nov 8, 2024

DRSDavidSoft commented Nov 9, 2024 • edited Loading

jnvsor commented Nov 9, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DRSDavidSoft Nov 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jnvsor commented Nov 11, 2024

jnvsor commented Nov 11, 2024

jnvsor commented Nov 11, 2024 • edited Loading

DRSDavidSoft commented Oct 28, 2024 •

edited

Loading

DRSDavidSoft commented Nov 9, 2024 •

edited

Loading

DRSDavidSoft Nov 11, 2024 •

edited

Loading

jnvsor commented Nov 11, 2024 •

edited

Loading