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

Update success color #731

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Update success color #731

wants to merge 4 commits into from

Conversation

homing1
Copy link
Contributor

@homing1 homing1 commented Nov 27, 2024

Link: preview

This PR updates the success semantic color to match Fluent 2.

Testing

  1. Visit the following pages to review the affected colors, should still work on dark and high contrast theme. Compare with https://react.fluentui.dev/ . Also compare with https://design.learn.microsoft.com/ for the current Atlas colors.

Color | Atlas Design | Microsoft
Badge | Atlas Design | Microsoft
Button | Atlas Design | Microsoft
Help | Atlas Design | Microsoft
Input | Atlas Design | Microsoft
Notification | Atlas Design | Microsoft
Progress Bar | Atlas Design | Microsoft

Additional information

[Optional]

Contributor checklist

  • Did you update the description of this pull request with a review link and test steps?
  • Did you submit a changeset? Changesets are required for all code changes.
  • Does your pull request implement complex UI logic (js/ts)? Consider adding an integration test to test your user flow.
  • Does your pull request add a new page to the documentation site? Add your new page for automated accessibility testing in /integration/tests/accessibility.
  • Does your pull request change any transforms? Did you test they work on right to left?

@homing1 homing1 requested a review from a team as a code owner November 27, 2024 01:11
Copy link

changeset-bot bot commented Nov 27, 2024

🦋 Changeset detected

Latest commit: f30ce9e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@microsoft/atlas-css Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR


&.notification-#{$name} {
border-color: $dark;
background-color: $background;
color: $dark;

@if $name == 'success' {
Copy link
Contributor Author

@homing1 homing1 Nov 27, 2024

Choose a reason for hiding this comment

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

This is a temporary rule to prevent the success changes from breaking other colored notifications. Fluent's message bar border color is a shade of green that isn't used for any other component and doesn't fit any of the existing success theme variables. Danger and warning have the same issue.

The naming here is confusing, but success-active is one of the unused variables so it's available for a color without affecting existing components. It would make the most sense to add a new variable success-border to the theme map, but it would mean expanding the theme map even more.

https://react.fluentui.dev/?path=/docs/components-messagebar--docs

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.

1 participant