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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add theme support for code snippet #5004

Merged
merged 6 commits into from Mar 1, 2024
Merged

Conversation

britneywwc
Copy link
Contributor

@britneywwc britneywwc commented Feb 23, 2024

Done

  • Added theme support for icons used in code snippets (get-link, linux-prompt)
  • Added theme support for code snippets
  • Added dark theme examples for code snippets: /dropdown-multiple-dark, /default-dark, /icon-dark

Not done

  • Code snippet with syntax highlighting is currently not supported on dark theme
  • Cannot use the current theme colours because the dark code background is grey and not black, doesn't have the correct contrast in colour

Fixes WD-8977

QA

  • Open demo
  • Go to docs/examples and check if code snippets' themes update accordingly
  • Check examples:
    • Code Snippet / Dark
    • Code Snippet / Multiple dropdowns Dark
    • Code Snippet / Icon Dark

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:

  • PR should have one of the following labels to automatically categorise it in release notes:
    • Feature 馃巵, Breaking Change 馃挘, Bug 馃悰, Documentation 馃摑, Maintenance 馃敤.
  • Vanilla version in package.json should be updated relative to the most recent release, following semver convention:
    • if CSS class names are not changed it can be bugfix relesase (x.x.X)
    • if CSS class names are changed/added/removed it should be minor version (x.X.0)
    • see the wiki for more details
  • Any changes to component class names (new patterns, variants, removed or added features) should be listed on the what's new page.

Screenshots

image

@webteam-app
Copy link

Demo starting at https://vanilla-framework-5004.demos.haus

@@ -109,6 +109,8 @@ $colors--light-theme--text-inactive: rgba($color-x-dark, $inactive-text-opacity-
$colors--light-theme--link-default: $color-link !default;
$colors--light-theme--link-visited: $color-link-visited !default;

$colors--light-theme--code-background-default: $color-code-background !default;
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we will need different code backgrounds, so I would treat it as a variant of background colour.

So following out naming conventions it would be $colors--light-theme--background-code instead.

Please update all the names to follow this convention.

Copy link
Contributor

Choose a reason for hiding this comment

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

where did this code background come from?

Copy link
Contributor

Choose a reason for hiding this comment

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

We have them defined already, this is just moving them into CSS variables in theme:

$color-code-background: rgba($color-x-dark, 0.03);
$color-code-background-dark: rgba($color-x-light, 0.3);

@bartaz
Copy link
Contributor

bartaz commented Feb 26, 2024

@lyubomir-popov Is this ok from design point of view? Uses dark background for code (that we have already in Vanilla).

@lyubomir-popov
Copy link
Contributor

lyubomir-popov commented Feb 26, 2024

@lyubomir-popov Is this ok from design point of view? Uses dark background for code (that we have already in Vanilla).

The background of the code snippet has a lot more contrast with the base background than in the light theme, how was that derived? I think we need to make it similar to the contrast we have in the light theme.

@bartaz
Copy link
Contributor

bartaz commented Feb 26, 2024

@lyubomir-popov Is this ok from design point of view? Uses dark background for code (that we have already in Vanilla).

The background of the code snippet has a lot more contrast with the base background than in the light theme, how was that derived? I think we need to make it similar to the contrast we have in the light theme.

@lyubomir-popov This is what we have in Vanilla for a while: https://vanillaframework.io/docs/examples/base/code-inline-dark

If we need to adjust this, it's a good moment to do this. Can you suggest other values?

What should the code background be (on white, paper, dark)?
What should the code snippet header background be (in all 3 versions)?

@lyubomir-popov
Copy link
Contributor

Hi I don't have capacity to do this right now, please proceed with this, and we can improve later.

@@ -13,11 +13,11 @@
&.prolog,
&.doctype,
&.cdata {
color: $colors--light-theme--text-muted;
color: $colors--theme--text-muted;
Copy link
Contributor

Choose a reason for hiding this comment

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

We probably need to look into theming rest of the syntax highlighting colours, but this can wait.

Copy link
Contributor

@bartaz bartaz left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for that!

@bartaz bartaz merged commit 035954e into canonical:main Mar 1, 2024
5 checks passed
@britneywwc britneywwc deleted the theme-code branch March 8, 2024 10:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants