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

Remove SASS warnings by using color.scale and color.channel functions #5370

Merged
merged 3 commits into from
Oct 9, 2024

Conversation

mcslayer
Copy link
Contributor

@mcslayer mcslayer commented Oct 7, 2024

Done

Change SASS function, which is tagged as deprecated

  • lightness to color.channel
  • darken, lighten, transparentize to color.scale

Use SASS documentation

QA

  • Unfortunately, I don't know how better to show a demo for checking the absence of warnings. But everything should work and not show SASS warnings in the console.

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 release (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.

@webteam-app
Copy link

mcslayer is not a collaborator of the repo

@jmuzina
Copy link
Member

jmuzina commented Oct 7, 2024

Hi @mcslayer , thanks for your interest and work!

It looks like there are some small visual changes to .p-strip--suru.

Before and after:
image

Diff view:
image

I believe your changes to $color-suru-start and $color-suru-end have changed their evaluated results slightly. I'd suggest taking a look there to make sure the appearance of the component is not changed.

@mcslayer
Copy link
Contributor Author

mcslayer commented Oct 7, 2024

Thank you for your review. I apologize for any mistakes in the previous submission. Upon further investigation, I discovered that the issue was related to the color.scale function, which did not accurately convert colors. As a result, I opted to use the color.adjust function instead. I tested various options in the Sass playground, and I can now confirm that the implementation is correct.
Screenshot 2024-10-07 at 21 33 02

Copy link
Member

@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 your help @mcslayer, these warnings we quite annoying and we were just about to do something about it :)

@bartaz bartaz merged commit 2ee4aec into canonical:main Oct 9, 2024
13 checks passed
@jmuzina jmuzina mentioned this pull request Oct 10, 2024
3 tasks
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.

4 participants