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

Tooltip : updated tooltip styles #28264

Merged
merged 4 commits into from
Jun 20, 2023
Merged

Conversation

kkakroo
Copy link
Contributor

@kkakroo kkakroo commented Jun 20, 2023

Previous Behavior

Tooltip content exceeds its container when long text is added

New Behavior

Tooltip content gets trimmed with ellipsis when long text is added

Related Issue(s)

@fabricteam
Copy link
Collaborator

fabricteam commented Jun 20, 2023

Perf Analysis (@fluentui/react-components)

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 591 623 5000
Button mount 289 294 5000
Field mount 1060 1081 5000
FluentProvider mount 663 669 5000
FluentProviderWithTheme mount 75 90 10
FluentProviderWithTheme virtual-rerender 81 71 10
FluentProviderWithTheme virtual-rerender-with-unmount 79 78 10
InfoButton mount 13 16 5000
MakeStyles mount 855 857 50000
Persona mount 1670 1625 5000
SpinButton mount 1351 1380 5000

@size-auditor
Copy link

size-auditor bot commented Jun 20, 2023

Asset size changes

Size Auditor did not detect a change in bundle size for any component!

Baseline commit: 9b5b3f33ad05e2dce6457c72b9e5ffa352033e47 (build)

@codesandbox-ci
Copy link

codesandbox-ci bot commented Jun 20, 2023

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit c6dddb8:

Sandbox Source
@fluentui/react 8 starter Configuration
@fluentui/react-components 9 starter Configuration
Tooltip content exceeds its container when using url such as https://.... Issue #28228

@fabricteam
Copy link
Collaborator

fabricteam commented Jun 20, 2023

📊 Bundle size report

Package & Exports Baseline (minified/GZIP) PR Change
react-tooltip
Tooltip
47.263 kB
16.585 kB
47.319 kB
16.618 kB
56 B
33 B
Unchanged fixtures
Package & Exports Size (minified/GZIP)
react-alert
Alert
82.552 kB
21.771 kB
react-avatar
Avatar
47.7 kB
14.504 kB
react-avatar
AvatarGroup
15.682 kB
6.306 kB
react-avatar
AvatarGroupItem
63.876 kB
18.984 kB
react-components
react-components: Button, FluentProvider & webLightTheme
65.162 kB
17.952 kB
react-components
react-components: Accordion, Button, FluentProvider, Image, Menu, Popover
210.365 kB
58.697 kB
react-components
react-components: FluentProvider & webLightTheme
36.395 kB
11.996 kB
react-persona
Persona
54.621 kB
16.435 kB
react-portal-compat
PortalCompatProvider
6.473 kB
2.196 kB
react-table
DataGrid
156.306 kB
42.785 kB
react-table
Table (Primitives only)
44.652 kB
12.468 kB
react-table
Table as DataGrid
131.96 kB
33.791 kB
react-table
Table (Selection only)
77.561 kB
19.178 kB
react-table
Table (Sort only)
76.891 kB
18.989 kB
react-tags
Tag
23.153 kB
7.922 kB
🤖 This report was generated against 9b5b3f33ad05e2dce6457c72b9e5ffa352033e47

Copy link
Contributor

@behowell behowell left a comment

Choose a reason for hiding this comment

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

Hi @kkakroo, thanks for the PR. Unfortunately it looks like this approach breaks the withArrow styles, probably because the overflow: hidden style clips the arrow. Additionally, we'd prefer to avoid clipping/ellipsifying the tooltip content if possible, since the point of the tooltip is to show additional information.

It looks like overflowWrap: 'break-word' would be a reasonable fix for the linked issue. If you'd like you can update this PR with that style, or I can make a PR with that change.

Also, please put the style on the 'root' class; the 'visible' class is just about controlling visibility.

@kkakroo
Copy link
Contributor Author

kkakroo commented Jun 20, 2023

Hi @behowell , thanks for the comment.
Had not taken the withArrow styles into consideration, have updated the styles accordingly.

Copy link
Contributor

@behowell behowell left a comment

Choose a reason for hiding this comment

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

Thanks for the update. It'd be good to add visual regression test for this case as well. Could you add a test case to apps\vr-tests-react-components\src\stories\Tooltip.stories.tsx? Thanks.

@kkakroo kkakroo requested a review from a team as a code owner June 20, 2023 22:43
@kkakroo
Copy link
Contributor Author

kkakroo commented Jun 20, 2023

@behowell , added VR tests as requested.

Copy link
Contributor

@behowell behowell left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for making the updates!

@behowell behowell merged commit 64fb6d6 into microsoft:master Jun 20, 2023
marcosmoura added a commit to marcosmoura/fluentui that referenced this pull request Jun 21, 2023
* master:
  Fixed bugs and added more stories to the Breadcrumb (microsoft#28267)
  refactor: Keep vanillajs code only where needed (microsoft#28278)
  fix: correcting focus behavior of react-search (microsoft#28241)
  Tooltip : updated tooltip styles (microsoft#28264)
marcosmoura added a commit to marcosmoura/fluentui that referenced this pull request Jun 22, 2023
* master: (95 commits)
  docs(react-drawer): improve drawer stories examples (microsoft#28283)
  bugfix: adds grid-template-columns to DialogBody styles to ensure grid template layout (microsoft#28272)
  Doc: Tree Infinite Scrolling (microsoft#28197)
  fix(react-card): infer a11y id from immediate header element (microsoft#28266)
  Fixed bugs and added more stories to the Breadcrumb (microsoft#28267)
  refactor: Keep vanillajs code only where needed (microsoft#28278)
  fix: correcting focus behavior of react-search (microsoft#28241)
  Tooltip : updated tooltip styles (microsoft#28264)
  applying package updates
  feat(react-tags): add styles for size (microsoft#28229)
  docs(react-dialog): update DialogTriggerOutsideDialog to include focus behavior (microsoft#28176)
  bugfix: Ensures dialog actions stretches on breakpoints (microsoft#28258)
  applying package updates
  fix: TableHeaderCell should not render button when not sortable (microsoft#28097)
  fix(react-file-type-icons): Map mhtml extension to html icon (microsoft#28112)
  Fix overlapping axis labels on smaller viewports (microsoft#28239)
  useArrowNavigationGroup grid-linear axis (microsoft#28253)
  applying package updates
  fix: Alert example missing aria-label for icon (microsoft#28234)
  Overflow divider fix (microsoft#28011)
  ...
marcosmoura added a commit to marcosmoura/fluentui that referenced this pull request Jun 22, 2023
* master:
  docs(react-drawer): best practices (microsoft#28040)
  docs(react-drawer): add missing documentation for Drawer stories (microsoft#28284)
  docs(react-drawer): improve type descriptions and fix TS circular references (microsoft#28282)
  docs(react-drawer): improve drawer stories examples (microsoft#28283)
  bugfix: adds grid-template-columns to DialogBody styles to ensure grid template layout (microsoft#28272)
  Doc: Tree Infinite Scrolling (microsoft#28197)
  fix(react-card): infer a11y id from immediate header element (microsoft#28266)
  Fixed bugs and added more stories to the Breadcrumb (microsoft#28267)
  refactor: Keep vanillajs code only where needed (microsoft#28278)
  fix: correcting focus behavior of react-search (microsoft#28241)
  Tooltip : updated tooltip styles (microsoft#28264)
@kkakroo kkakroo deleted the tooltip-fix branch June 25, 2023 15:17
@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-tooltip@v9.2.18 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-components@v9.22.1 has been released which incorporates this pull request.:tada:

Handy links:

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]: Tooltip content exceeds its container when using url such as https://....
6 participants