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

Enable fine-grained styling in code #11280

Merged
merged 2 commits into from
Jun 16, 2022
Merged

Enable fine-grained styling in code #11280

merged 2 commits into from
Jun 16, 2022

Conversation

msujew
Copy link
Member

@msujew msujew commented Jun 9, 2022

What it does

Closes #11063
Closes #6743

Adds a new StylingService (and an associated StylingParticipant) which allows us to dynamically change styles using code. This has the advantage that we can set styles based on the existence of a color in a color theme.

As a simple example, we can take the notification hover color, which isn't set for some themes (hc-black). Hovering over a notification when the color isn't set will make the background transparent, while it should usually just keep the existing background color.

I used this feature to also implement the missing capabilities for high contrast theming and clean up some more (unused) css files (such as the variables-dark.useable.css/ variables-bright.useable.css files).

How to test

  1. Open VSCode and Theia side by side
  2. Select the dark high contrast theme for both of them
  3. Assert that they behave (close to) the same with the high contrast theme
  4. Assert that no visual regressions appear on other themes

Review checklist

Reminder for reviewers

@colin-grant-work colin-grant-work self-requested a review June 9, 2022 15:30
@colin-grant-work colin-grant-work added accessibility issues related to accessbilitiy theming issues related to theming labels Jun 9, 2022
Copy link
Contributor

@colin-grant-work colin-grant-work left a comment

Choose a reason for hiding this comment

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

Looks super good 👍, and you've made it look easy! I thought it would take a lot more code than this to implement the feature. I haven't noticed any regressions in a quick look, but I'll leave an HC theme in place for a bit to see if anything comes up. A few comments on the code.

packages/core/src/browser/style/index.css Outdated Show resolved Hide resolved
packages/core/src/browser/style/index.css Outdated Show resolved Hide resolved
packages/core/src/browser/common-frontend-contribution.ts Outdated Show resolved Hide resolved
packages/core/src/browser/styling-service.ts Outdated Show resolved Hide resolved
packages/core/src/browser/common-frontend-contribution.ts Outdated Show resolved Hide resolved
@msujew
Copy link
Member Author

msujew commented Jun 10, 2022

@colin-grant-work Thanks, I split the styling participants related to a certain area of the app into one class each 👍

@msujew msujew force-pushed the msujew/theming branch 2 times, most recently from ed09ad1 to ffbc3c0 Compare June 13, 2022 13:44
@colin-grant-work colin-grant-work self-requested a review June 13, 2022 14:01
Copy link
Contributor

@colin-grant-work colin-grant-work left a comment

Choose a reason for hiding this comment

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

With the modularization, it should be easier for downstream applications to customize or add to particular parts. 👍

@msujew msujew merged commit 1a46fb0 into master Jun 16, 2022
@msujew msujew deleted the msujew/theming branch June 16, 2022 14:49
@github-actions github-actions bot added this to the 1.27.0 milestone Jun 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accessibility issues related to accessbilitiy theming issues related to theming
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow fine-grained CSS control for theming high contrast theming
2 participants