Skip to content

feat: Add warning tone for Badge#826

Merged
nats12 merged 2 commits intomainfrom
natalie/badge-warning-tone
May 29, 2024
Merged

feat: Add warning tone for Badge#826
nats12 merged 2 commits intomainfrom
natalie/badge-warning-tone

Conversation

@nats12
Copy link
Copy Markdown
Contributor

@nats12 nats12 commented May 28, 2024

Short description

Adds a new warning variant for the Badge component's tone option.

Screenshot 2024-05-28 at 11 30 36

PR Checklist

  • Added tests for bugs / new features
  • Updated docs (storybooks, readme)
  • Executed npm run validate and made sure no errors / warnings were shown
  • Described changes in CHANGELOG.md
  • Bumped version in package.json and package-lock.json (npm --no-git-tag-version version <major|minor|patch>) ref
  • Reviewed and approved Chromatic visual regression tests in CI

Versioning

Minor

@nats12 nats12 requested review from a team and engfragui and removed request for a team May 28, 2024 09:31
@nats12 nats12 changed the title chore: Add warning tone for Badge feat: Add warning tone for Badge May 28, 2024
@engfragui
Copy link
Copy Markdown
Contributor

@nats12 The code change looks sane to me 👌 The only thing I noticed is that the colors are the same for both light and dark mode versions, which I find unusual...

Screenshot 2024-05-29 at 10 21 00 Screenshot 2024-05-29 at 10 20 56

I'm assuming this new Badge tone has been requested by Design team? 🤔

Copy link
Copy Markdown
Contributor

@engfragui engfragui left a comment

Choose a reason for hiding this comment

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

Code change looks correct so approving. Raised a small note re: colors in light and dark mode, but otherwise good to go in case it's confirmed that the colors are as intended by the Desing team 👌

@nats12
Copy link
Copy Markdown
Contributor Author

nats12 commented May 29, 2024

I'm assuming this new Badge tone has been requested by Design team? 🤔

@engfragui indeed, see convo here about the latest token style, and adding it to this component.

The only thing I noticed is that the colors are the same for both light and dark mode versions, which I find unusual...

I believe dark mode isn't supported for these components, so those aren't the actual dark mode styles that would be applied. That screenshot illustrates an example from the docs on how to style them differently. It seems most are inverted, with a slightly darker tint than in light mode, I'll do the same for warning:

Screenshot 2024-05-29 at 14 13 13

Thanks for raising! 🙌🏼

@nats12 nats12 merged commit d6c635b into main May 29, 2024
@nats12 nats12 deleted the natalie/badge-warning-tone branch May 29, 2024 13:14
@nats12 nats12 mentioned this pull request May 31, 2024
6 tasks
nats12 added a commit that referenced this pull request May 31, 2024
* feat: Add 'medium' weight to Heading (#814)

* feat: Add 'medium' weight to Heading

* Update changelog

* Bump version

* fix: Adjust modal alignment to middle at breakpoint (#821)

* revert: Modal alignment to middle at breakpoint adjustment (#823)

ref: 3aefefb

* feat: Add warning tone for Badge (#826)

## Short description

Adds a new `warning` variant for the Badge component's `tone` option.


<img width="281" alt="Screenshot 2024-05-28 at 11 30 36" src="https://github.com/Doist/reactist/assets/15801768/b829b743-6356-4932-8b00-ab4a34d44769">


## PR Checklist

<!--
Feel free to leave unchecked or remove the lines that are not applicable.
-->

-   [x] Added tests for bugs / new features
-   [x] Updated docs (storybooks, readme)
-   [x] Executed `npm run validate` and made sure no errors / warnings were shown
-   [x] Described changes in `CHANGELOG.md`
-   [x] Bumped version in `package.json` and `package-lock.json` (`npm --no-git-tag-version version <major|minor|patch>`) [ref](https://docs.npmjs.com/cli/v6/commands/npm-version)
-   [x] Reviewed and approved Chromatic visual regression tests in CI

## Versioning

Minor

* chore: Bump up version to v23.3.0 (#827)

---------

Co-authored-by: Frankie Yan <frankieyan@users.noreply.github.com>
Co-authored-by: Ricardo Amaral <ricardo@doist.com>
Co-authored-by: Ernesto García <ernesto@doist.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants