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

Add theming for chips #4168

Merged
merged 7 commits into from
Dec 2, 2021

Conversation

bethcollins92
Copy link
Contributor

Done

  • Added theming for chips with different shades of border/background/text for each chip type
  • Added theme default setting for chips
  • Added example in the docs
  • Added an example for the dismiss chip as this was missing

Fixes #4158

QA

  • Open demo
  • Check the dark chip example
  • Check adding is-dark or is-light class names work as expected
  • Review updated documentation:
    • Chip documentation - new example for the dismiss chip and dark theme

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.
  • Documentation side navigation should be updated with the relevant labels.

@bethcollins92 bethcollins92 added the Feature 🎁 New feature or request label Dec 1, 2021
@webteam-app
Copy link

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

@bartaz
Copy link
Contributor

bartaz commented Dec 1, 2021

@bethcollins92 I think we should have dark chips in the variants example as well:
https://vanilla-framework-4168.demos.haus/docs/examples/patterns/chip/variants

I guess just duplicate whole thing inside the dark strip (in the same example file below current stuff).


// Theming
@if ($theme-default-p-chip == 'dark') {
.p-chip {
Copy link
Contributor

Choose a reason for hiding this comment

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

This part seems extremely repetitive, but I'm not sure if we can do anything about it - we do need separate color treatment for each of those variants…

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had the same feeling when writing it but couldn't figure out a way around it!

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.

LGTM, thanks!

@lyubomir-popov
Copy link
Contributor

lyubomir-popov commented Dec 1, 2021

Why does the close button at the bottom have class .is-light?
image

Copy link
Contributor

@lyubomir-popov lyubomir-popov left a comment

Choose a reason for hiding this comment

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

I think there's an extra class that makes the close grey on red... not very readable. Suggestion to remove it in the code.

templates/docs/examples/patterns/chip/variants.html Outdated Show resolved Hide resolved
@bethcollins92
Copy link
Contributor Author

@lyubomir-popov The .is-light class was in the wrong place. Fixed it now

@bethcollins92 bethcollins92 merged commit 9d2b664 into canonical:main Dec 2, 2021
@bethcollins92 bethcollins92 deleted the 4158-dark-theme-chips branch December 2, 2021 09:53
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.

Dark theme for chips
4 participants