Skip to content
New issue

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

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

Already on GitHub? # to your account

fix(spinner): override wrong tokens mapping #1500

Merged
merged 11 commits into from
Aug 2, 2024

Conversation

mivaylo
Copy link
Contributor

@mivaylo mivaylo commented Jul 31, 2024

PR Checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • If applicable, have a visual design approval

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

What is the current behavior?

image

Issue Number: CDE-1513

What is the new behavior?

image

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

Spinner inverse tokens were correct but their mapping to the global ones is wrong. This overrides the wrong mapping and these tokens are not used in other places so we are safe with the fix.

Update: after discussion with @valentin-mladenov and design team we've found out that two more tokens have incorrect mapping, so we are adding them too in this PR.

Another improvement that the PR includes is providing a more meaningful demo for the inverse spinner (with proper background) in the demo app and the storybook

Copy link
Contributor

github-actions bot commented Jul 31, 2024

👋 @mivaylo,

  • 🙏 The Clarity team thanks you for opening a pull request
  • 🎉 The build for this PR has succeeded
  • 🔍 The PR is now ready for review
  • 🍿 In the meantime, view a preview of this PR
  • 🖐 You can always follow up here. If you're a VMware employee, you can also reach us on our internal Clarity Support space

Thank you,

🤖 Clarity Release Bot

@valentin-mladenov
Copy link
Contributor

Follow up of #1488

Copy link
Contributor

This PR introduces visual changes: 407570f
If these changes are intended and correct, please cherry-pick the above commit to this PR.

git checkout cde-1513-override-wrong-tokens-mapping
git fetch https://github.com/vmware-clarity/ng-clarity.git 407570fe0c0a7d43d6c8fbdd9c4526a28c1a6381
git cherry-pick 407570fe0c0a7d43d6c8fbdd9c4526a28c1a6381
git push

Copy link
Contributor

github-actions bot commented Aug 1, 2024

This PR introduces visual changes: 37a273e
If these changes are intended and correct, please cherry-pick the above commit to this PR.

git checkout cde-1513-override-wrong-tokens-mapping
git fetch https://github.com/vmware-clarity/ng-clarity.git 37a273ea3b0d2076ffe61ca037d1593ce54b38c0
git cherry-pick 37a273ea3b0d2076ffe61ca037d1593ce54b38c0
git push

Copy link
Contributor

@valentin-mladenov valentin-mladenov left a comment

Choose a reason for hiding this comment

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

Check my comments

@mivaylo mivaylo force-pushed the cde-1513-override-wrong-tokens-mapping branch from 8bb90e3 to 3184036 Compare August 2, 2024 13:27
Copy link
Contributor

github-actions bot commented Aug 2, 2024

This PR introduces visual changes: efd9764
If these changes are intended and correct, please cherry-pick the above commit to this PR.

git checkout cde-1513-override-wrong-tokens-mapping
git fetch https://github.com/vmware-clarity/ng-clarity.git efd9764c146591ff05d78d90176dff738e020cf5
git cherry-pick efd9764c146591ff05d78d90176dff738e020cf5
git push

Copy link
Contributor

@valentin-mladenov valentin-mladenov left a comment

Choose a reason for hiding this comment

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

LGTM. Check my comment.

@mivaylo mivaylo merged commit c5883fd into main Aug 2, 2024
8 checks passed
@mivaylo mivaylo deleted the cde-1513-override-wrong-tokens-mapping branch August 2, 2024 15:09
Copy link
Contributor

Hi there 👋, this is an automated message. To help Clarity keep track of discussions, we automatically lock closed PRs after 14 days. Please look for another open issue or open a new issue with updated details and reference this one as necessary.

@github-actions github-actions bot locked and limited conversation to collaborators Aug 17, 2024
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants