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

fix(react-message-bar): force links to be underlined #32850

Merged
merged 13 commits into from
Oct 1, 2024

Conversation

mainframev
Copy link
Contributor

@mainframev mainframev commented Sep 17, 2024

According to design spec links inside MessageBar should be underlined

Previous Behavior

image

New Behavior

image

Related Issue(s)

@fabricteam
Copy link
Collaborator

fabricteam commented Sep 17, 2024

📊 Bundle size report

Package & Exports Baseline (minified/GZIP) PR Change
react-components
react-components: entire library
1.1 MB
272.007 kB
1.1 MB
272.081 kB
222 B
74 B
react-link
Link
17.192 kB
6.98 kB
17.326 kB
7.032 kB
134 B
52 B
react-message-bar
MessageBar (all components)
24.722 kB
9.205 kB
24.85 kB
9.256 kB
128 B
51 B
Unchanged fixtures
Package & Exports Size (minified/GZIP)
react-breadcrumb
@fluentui/react-breadcrumb - package
113.699 kB
31.498 kB
react-components
react-components: Button, FluentProvider & webLightTheme
69.14 kB
20.137 kB
react-components
react-components: Accordion, Button, FluentProvider, Image, Menu, Popover
218.317 kB
63.258 kB
react-components
react-components: FluentProvider & webLightTheme
44.447 kB
14.59 kB
react-portal-compat
PortalCompatProvider
8.39 kB
2.64 kB
react-timepicker-compat
TimePicker
107.387 kB
35.758 kB
🤖 This report was generated against 213771402438cb4e594bdd7b37b6dcbe77136447

@mainframev mainframev requested a review from a team September 17, 2024 10:31
@mainframev mainframev changed the title fix(react-message-bar): update examples to have underlined links fix(react-message-bar): force links to be underlined Sep 18, 2024
@fabricteam
Copy link
Collaborator

fabricteam commented Sep 18, 2024

Perf Analysis (@fluentui/react-components)

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 637 628 5000
Button mount 308 304 5000
Field mount 1147 1130 5000
FluentProvider mount 716 705 5000
FluentProviderWithTheme mount 82 89 10
FluentProviderWithTheme virtual-rerender 34 34 10
FluentProviderWithTheme virtual-rerender-with-unmount 88 73 10
MakeStyles mount 872 866 50000
Persona mount 1738 1724 5000
SpinButton mount 1393 1380 5000
SwatchPicker mount 1661 1658 5000

@marcosmoura
Copy link
Contributor

This might be unrelated to this PR. But I am wondering if all links should be underlined in the context of the Design System. If that's the case, maybe it would be worth exploring if we can make this change to the @fluentui/react-link package itself?

Copy link
Member

@ling1726 ling1726 left a comment

Choose a reason for hiding this comment

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

These kinds of selectors are costy and also fragile - they depend on the styles of the component. Let's introduce a context for this if it only applies to MessageBar. Similar to https://github.com/microsoft/fluentui/blob/master/packages/react-components/react-avatar/library/src/contexts/AvatarContext.ts

mainframev and others added 4 commits September 27, 2024 22:48
…ents/MessageBarBody/useMessageBarBodyContextValues.ts

Co-authored-by: Oleksandr Fediashov <[email protected]>
…ents/MessageBarBody/MessageBarBody.types.ts

Co-authored-by: Oleksandr Fediashov <[email protected]>
@mainframev mainframev dismissed ling1726’s stale review October 1, 2024 11:00

Shift made a review for the same suggestion. Thanks Ling.

@mainframev mainframev merged commit 4fb4983 into microsoft:master Oct 1, 2024
19 checks passed
mltejera pushed a commit to mltejera/office-ui-fabric-react that referenced this pull request Oct 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Links should be underlined in MessageBar
5 participants