Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow for dynamic window colour with inline CSS variables #88

Merged
merged 2 commits into from
Mar 11, 2024

Conversation

liam-mills
Copy link
Contributor

@liam-mills liam-mills commented Feb 23, 2024

Hi @khang-nd,

I had a use case where I was wanting to allow for window colours to be dynamically and globally changed by updating the --window-background-color variable. I personally wasn't too keen on having to add a <style> tag and overwrite the window ::before pseudo element and .title-bar manually (as seen here), so I opted for this method instead. I also wanted to be able to change this colour on the fly through JS, and updating the CSS variable rather than updating the <style> element seems more straightforward.

Changes

  • Explicitly set the --window-background-color variable as the background-color for .window and .title-bar, allowing for it to be redefined within the inline style attribute of each element.
  • Changed postcss-css-variables to preserve CSS variables. Ideally I would like to set it to explicitly preserve just the --window-background-color and allow for all other CSS variables to be converted to their static counterpart as per original functionality, but I don't believe that postcss-css-variables allows for this. There is an open issue in the postcss-css-variables repo asking for this functionality. This does now mean that all CSS variables are now non-static. Keen to hear if you have any thoughts on whether there is a better way to approach this.
  • Slightly changed a calc() used in _treeview.scss to work with the above modification to postcss-css-variables. The end result of the calculation remains the same as previous.
  • Updated the docs to replace the previous <style> overwrite to instead set the --window-background-color on both windows demonstrating the colour change functionality.

Would love to hear your thoughts on this and whether you had any feedback. Happy to edit my PR if changes are required. Cheers.

Copy link

vercel bot commented Feb 23, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
7css ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 9, 2024 9:50am

@khang-nd
Copy link
Owner

Thanks for the PR, I'll have a closer look when I get the time.

Copy link
Owner

@khang-nd khang-nd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @liam-mills, thanks for bringing this up and even bother contributing with a PR. What you've done here makes perfect sense from my pov as well. Considering that CSS variables are now widely supported, I don't think it's necessary to have them transformed statically, and even leads to difficulty in customization in this case.

Just added the point that we may remove the plugin completely from the build step as we no longer need it. Appreciated :)

build.js Outdated Show resolved Hide resolved
Copy link
Owner

@khang-nd khang-nd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks sexy, no issues found. Thank you for the great contribution! 🙂

@khang-nd khang-nd merged commit b3e90f1 into khang-nd:main Mar 11, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants