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

[Feature]: Addition to Tooltip for managing overflow #33073

Closed
1 task done
gouttierre opened this issue Oct 17, 2024 · 7 comments · Fixed by #33278
Closed
1 task done

[Feature]: Addition to Tooltip for managing overflow #33073

gouttierre opened this issue Oct 17, 2024 · 7 comments · Fixed by #33278

Comments

@gouttierre
Copy link
Contributor

gouttierre commented Oct 17, 2024

Area

React Components (@fluentui/react-components)

Describe the feature that you would like added

In the 8th version, there's a utility feature for managing overflow and tooltips, showcased in the Tooltip example titled "Tooltip only on overflow."

However, in version 9, this particular example is absent from the ToolTip code samples, leading to some confusion among partners who are updating their products to v9.

To bridge this transition smoothly for migration, it would be beneficial to include a "Tooltip only on overflow" example in version 9's React Storybook.

Additional context

Reported by Azure Portal [Nicole Thuna]

Have you discussed this feature with our team

Gouttierre Gomes, Paul Gildea

Validations

  • Check that there isn't already an issue that requests the same feature to avoid creating a duplicate.

Priority

High

@tudorpopams
Copy link
Contributor

@mainframev can you please check if this can be achieved with the current implementation of Tooltip / Overflow?

@mainframev
Copy link
Contributor

mainframev commented Nov 1, 2024

@tudorpopams @gouttierre

Hello folks 👋🏻

I've checked this, v8 Tooltip had a specific property and overflow logic. It's selecting for overflow check either self or parent based on overflowMode property passed to TooltipHost. The current v9 implementation does not have this by design. The visibility of Tooltip based on overflow content can be added additionally by the user.

I quickly made a a couple examples demonstrating this.

I suggest, that maybe instead of creating a story example of this scenario, provide this issue and our migration doc, that says:

"overflowMode=> Not supported. If this behavior is needed, the tooltip's visibility can be controlled using thevisibleprop andonVisibleChange` event."

It's already covered with controlled example and adding a simple check for overflowing content via ref should not be a problem for user.

@dmytrokirpa
Copy link
Contributor

I suggest we could add the useIsOverflowing hook to the core library to streamline both examples. This approach could enhance simplicity and efficiency. Here's the link with your examples slightly updated:

https://stackblitz.com/edit/fu8vhs-iz3vyv?file=src%2FuseIsOverflowing.ts,src%2FSelfOverflow.tsx

wdyt?

@mainframev
Copy link
Contributor

mainframev commented Nov 6, 2024

@dmytrokirpa

Good question. I feel like it might not be used often and such things can be abstracted to utility hook on the user side without a problem. We can add it to react-utilities, but currently it holds hooks and other utilities that a re being first used in our development, where we sure that they are useful and used, this might end up as a kind of a "dead code".

@nikolaiessel
Copy link

We always use such a feature in our products, especially in v8 DetailsList: Our customers always want to display more columns/informations, than it is usefull for a fluent/responsive design. At same time no wrapping is wanted and a fixed column width is required to build a result view with grouped entries.

So we use the overflow feature to only show the tooltip if the data of the user does not fit the cell.

Isn´t it desired to "hide" the HTML/DOM specifics by the library? In this case the inclusion of overflow-check via a OoB hook would be some way.

Btw:
The initial TooltipOverflowSelf example contained an error in the cleanup function: the listener needs to be removed, not added. One of many mistakes that can happen, if the "user" does this logic by himself ;)

@mainframev
Copy link
Contributor

mainframev commented Nov 15, 2024

@nikolaiessel

The initial TooltipOverflowSelf example contained an error in the cleanup function: the listener needs to be removed, not added. One of many mistakes that can happen, if the "user" does this logic by himself ;)

Thank you for the feedback! I appreciate that you understand this was just a quickly made example for demonstration purposes. 😊

We always use such a feature in our products, especially in v8 DetailsList: Our customers always want to display more columns/informations, than it is usefull for a fluent/responsive design. At same time no wrapping is wanted and a fixed column width is required to build a result view with grouped entries.

I understand your perspective—thank you for providing the context. Component APIs can change significantly from version to version, and when a feature is not supported, it’s often an intentional decision. In my opinion, the Tooltip component should not handle overflow logic by default. However, it does provide the necessary props to control its visibility, leaving overflow handling as the user’s responsibility.

To support your migration, we could create documentation examples featuring a custom hook that checks for overflowing content. If, after discussion, if it will be decided to make this hook reusable, we can include it in react-utilities and suite. This should help ease the transition from v8 to v9.

UPD: here is the context on last change to this topic

UPD2: onlyIfTruncated story example existed in preview version

This issue has been automatically marked as stale because it has marked as requiring author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment. Thank you for your contributions to Fluent UI!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment