Skip to content

[A11y][chat]Remove theme8 color palette for <Link>#4691

Merged
AmyL219 merged 5 commits intomainfrom
amyl/a11y/link
Jun 5, 2024
Merged

[A11y][chat]Remove theme8 color palette for <Link>#4691
AmyL219 merged 5 commits intomainfrom
amyl/a11y/link

Conversation

@AmyL219
Copy link
Copy Markdown
Contributor

@AmyL219 AmyL219 commented Jun 4, 2024

What

Fix A11y issue for the ticket

The v9 theme pattern has the correct schema based on Alex P's input. So remove the v8 color theme overwrite in the token.

Why

How Tested

Process & policy checklist

  • I have updated the project documentation to reflect my changes if necessary.
  • I have read the CONTRIBUTING documentation.

Is this a breaking change?

  • This change causes current functionality to break.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 4, 2024

Calling bundle size is not changed.

  • Current size: 4890051
  • Base size: 4890051
  • Diff size: 0

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 4, 2024

CallWithChat bundle size is decreased✅.

  • Current size: 6217652
  • Base size: 6217700
  • Diff size: -48

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 4, 2024

Chat bundle size is decreased✅.

  • Current size: 2064835
  • Base size: 2064883
  • Diff size: -48

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 4, 2024

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 4, 2024

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

Lines Statements Functions Branches
Base 26200 / 40952
63.97%
26200 / 40952
63.97%
715 / 1277
55.99%
2085 / 3313
62.93%
Current 26252 / 40951
64.1%
26252 / 40951
64.1%
715 / 1277
55.99%
2065 / 3307
62.44%
Diff 52 / -1
0.13%
52 / -1
0.13%
0 / 0
0%
-20 / -6
-0.49%

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 4, 2024

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

Lines Statements Functions Branches
Base 50355 / 81042
62.13%
50355 / 81042
62.13%
1030 / 2294
44.89%
2949 / 4853
60.76%
Current 50354 / 81041
62.13%
50354 / 81041
62.13%
1030 / 2294
44.89%
2933 / 4845
60.53%
Diff -1 / -1
0%
-1 / -1
0%
0 / 0
0%
-16 / -8
-0.23%

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 4, 2024

errorText: themeV8.semanticColors.errorText,
colorNeutralStroke1Selected: themeV8.palette.neutralQuaternary,
colorNeutralForeground2: themeV8.palette.neutralSecondary,
colorBrandForegroundLink: themeV8.palette.themePrimary,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Are we good with the default value instead then? Do we need to update colorBrandForegroundLinkHover too?
image

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The default color is set as colorBrandForegroundLink: palette.themeDarkAlt,, themeDarkAlt is what Alex P want. And for Hover one, AlexP doesn't mention a update on it

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you check with Alex on the hover state(so we won't forget about it)? Otherwise, looks good to me!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I have confirmed with AlexP

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 5, 2024

@AmyL219 AmyL219 enabled auto-merge (squash) June 5, 2024 16:42
@AmyL219 AmyL219 merged commit 5ffd9bd into main Jun 5, 2024
@AmyL219 AmyL219 deleted the amyl/a11y/link branch June 5, 2024 16:45
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.

3 participants