chore(css): moves all our sass usage to CSS props via XTheme#4623
chore(css): moves all our sass usage to CSS props via XTheme#4623johncowen wants to merge 8 commits intokumahq:masterfrom
Conversation
✅ Deploy Preview for kuma-gui ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
schogges
left a comment
schogges
left a comment
There was a problem hiding this comment.
LGTM
There are a few left-over CSS rules that are not tokenized yet and could potentially be changed to use --x-<token> now, i.e.
Could be resolved in a follow-up or here.
|
Oh top spot 👏 , let me sort that here |
Signed-off-by: John Cowen <john.cowen@konghq.com> Assisted-by: Claude Code (Opus 4.6)
Signed-off-by: John Cowen <john.cowen@konghq.com>
Signed-off-by: John Cowen <john.cowen@konghq.com>
Signed-off-by: John Cowen <john.cowen@konghq.com>
Signed-off-by: John Cowen <john.cowen@konghq.com>
Signed-off-by: John Cowen <john.cowen@konghq.com>
Signed-off-by: John Cowen <john.cowen@konghq.com>
Signed-off-by: John Cowen <john.cowen@konghq.com>
johncowen
force-pushed
the
chore/x-theme-vars
branch
from
6ff09b4 to
88e3ea2
Compare
|
Ok I made a couple of extra commits, one to sort the last non-tokenized colors:
Apart from this, when looking at moving the Again just some observations from my AI usage here.
|
There was a problem hiding this comment.
Did you check x-components for variables and tokenization? I think there is at least this left
https://github.com/kumahq/kuma-gui/blob/master/packages/x/src/components/x-icon/XIcon.vue#L162-L166
| <style lang="scss" scoped> | ||
| :deep(> *) { | ||
| & { | ||
| --x-anchor-text-color: #{$kui-color-text-primary}; |
There was a problem hiding this comment.
I think we should use --x-color-text-primary here
| --icon-space-before: #{$kui-space-30}; | ||
| --icon-space-after: #{$kui-space-30}; |
There was a problem hiding this comment.
I think we should use --x-space-30 here
2 participants
We want to be in a position where we can easily theme our application (e.g. light/dark).
This PR is a step towards that by moving the majority of our build-time/static SASS vars to use runtime/dynamic CSS props instead.
As CSS changes can be hard to spot via automated testing etc, please give the preview site a good click around on, to my eyes it looks fairly identical to before this PR.
This PR is also a part experiment/bounce-around of AI.
The PR has 2 commits, one entirely generated by Claude/Opus4.6 and one with some tiny amends by me. I've included the prompts and plan that Claude created based off that and out of interest I asked it to record the time/tokens used in the frontmatter. Unfortunately this is only the time/tokens to generate the plan, I ran out of my session allowance just as Claude finished up its first pass (the first commit), and I was asking it to record the time/tokens for the implem. also. I guess I could have recorded this myself but I quit Claude and forgot to take a note of the time/tokens 🙈 .
As of right now I plan on deleting the plan before merge, I've left it in here seeing as this is the "modern source code" and we are an Open Source project. But as we haven't defined what we want to do here (if anything) I guess I'd probably delete it once we've reviewed this experiment.
Lastly, this PR still requires some review as I've already spotted one place where I think the colors have altered slightly, but all in all I think it looks pretty promising!
Oh I should add, Claude was running in a docker based sandbox with the project mounted inside along with an iptables based firewall. Be careful out there folks! 😅
Edit: I added another commit with a mostly AI generated README for
XTheme(I asked it to take a look at the plan again), I made some fairly extensive edits to that myself and added a short README forx-icon-startalso. 12% of my current session usage for writing the README 😱 .Lastly I'm planning on deleting the plan from here, in fact I'll do it now, I'm guessing it will still be visible in the commits if folks are interested. edit: plan deleted taking out of Draft