Skip to content

Fix screenshare button text to be black in dark mode #5654

Merged
JamesBurnside merged 3 commits intomainfrom
jaburnsi/fix-screenshare-dark-mode-color
Feb 18, 2025
Merged

Fix screenshare button text to be black in dark mode #5654
JamesBurnside merged 3 commits intomainfrom
jaburnsi/fix-screenshare-dark-mode-color

Conversation

@JamesBurnside
Copy link
Copy Markdown
Member

What

Update screenshare button stylings to match a regular primary button

Why

Screenshare button icon was black but text was white. Both should be black.

How Tested

before after
image image

No impact to high contrast modes:

aquatic desert dusk night sky
image image image image

@JamesBurnside JamesBurnside added the update_snapshots Set this label to request automated update of UI snapshots label Feb 18, 2025
@JamesBurnside JamesBurnside requested review from a team as code owners February 18, 2025 15:39
@github-actions github-actions Bot removed the update_snapshots Set this label to request automated update of UI snapshots label Feb 18, 2025
@JamesBurnside JamesBurnside added the update_snapshots Set this label to request automated update of UI snapshots label Feb 18, 2025
@github-actions github-actions Bot removed the update_snapshots Set this label to request automated update of UI snapshots label Feb 18, 2025
@github-actions
Copy link
Copy Markdown
Contributor

Calling bundle size is not changed.

  • Current size: 12401100
  • Base size: 12401100
  • Diff size: 0

@github-actions
Copy link
Copy Markdown
Contributor

CallWithChat bundle size is not changed.

  • Current size: 12401112
  • Base size: 12401112
  • Diff size: 0

@github-actions
Copy link
Copy Markdown
Contributor

Chat bundle size is not changed.

  • Current size: 1777281
  • Base size: 1777281
  • Diff size: 0

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions
Copy link
Copy Markdown
Contributor

@azure/communication-react jest test coverage for beta.

Lines Statements Functions Branches
Base 58230 / 94191
61.82%
58230 / 94191
61.82%
1177 / 2693
43.7%
3542 / 5828
60.77%
Current 58222 / 94195
61.81%
58222 / 94195
61.81%
1177 / 2693
43.7%
3504 / 5807
60.34%
Diff -8 / 4
-0.01%
-8 / 4
-0.01%
0 / 0
0%
-38 / -21
-0.43%

@github-actions
Copy link
Copy Markdown
Contributor

@azure/communication-react jest test coverage for stable.

Lines Statements Functions Branches
Base 27986 / 44659
62.66%
27986 / 44659
62.66%
786 / 1436
54.73%
2340 / 3712
63.03%
Current 28000 / 44663
62.69%
28000 / 44663
62.69%
786 / 1436
54.73%
2349 / 3718
63.17%
Diff 14 / 4
0.03%
14 / 4
0.03%
0 / 0
0%
9 / 6
0.14%

color: DefaultPalette.white,
background: theme.semanticColors.primaryButtonBackground,
color: theme.semanticColors.primaryButtonText,
':focus::after': { outlineColor: `${DefaultPalette.white} !important` }, // added !important to avoid override by FluentUI button styles
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we also update the :focus::after outline color here and on hover below?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

They're actually intentional - the black outline is really hard to see, I'd only want to update if we get A11y complaining about it

background: theme.palette.themePrimary,
color: DefaultPalette.white,
background: theme.semanticColors.primaryButtonBackground,
color: theme.semanticColors.primaryButtonText,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cool. Good use of semantic colors.

@JamesBurnside JamesBurnside merged commit 0c21167 into main Feb 18, 2025
@JamesBurnside JamesBurnside deleted the jaburnsi/fix-screenshare-dark-mode-color branch February 18, 2025 19:08
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.

4 participants