MudTooltip: Activation via Hover, Focus or Click

[mckaragoz]

Description

Fixes #4564.
First of all, we needed to wait popover fix to see true effects on server-side mobile. It's done, so we can discuss it now.

So why we need this?

With #4086 Focus event added to the tooltip, but that may be a problem for some situations, because there is no other option to prevent it.

Here is an example. The "GitHub Repository" tooltip doesn't close when you open a new tab and turn the page, because it still has focus and for current situation no way to prevent this.

20220522_020214.mp4

Examples can be multiplied. So instead of force users to certain scenarios, adding options could make sense. The other argument, we still can not guarantee to certainly working tooltip/popover for all server-side/wasm projects. So this PR raises the probability of creating workarounds if we have side effects or something other with .net 7 in future.

So its not a breaking change. Tooltip still shows on focus and hover, but now we have options that ShowOnHover and ShowOnFocus. For the upper example, users can set ShowOnFocus="false" to disable the tooltip.

Or, users still want the tooltip with hover. This time they can use ClickBehavior="TooltipClickBehavior.ClickToHide". With this, tooltip shows on focus, but when user click on it, it disappears. It will solve the problems like upper example.

This PR doesn't have a touch addition. I think touch is a more general thing and should be discussed to adding whole library. But users can play with parameters to have their own touch reaction.

The result of this PR:

20220522_015551.mp4

20220522_015721.mp4

How Has This Been Tested?

Tested visually with Server-Side and WASM and the touch versions.

Types of changes

• Bug fix (non-breaking change which fixes an issue) (fixes bugs, but actually they are not bugs)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

• The PR is submitted to the correct branch (dev).
• My code follows the code style of this project.
• I've added relevant tests.

[codecov]

Codecov Report

Base: 91.50% // Head: 91.50% // Increases project coverage by +0.00% 🎉

Coverage data is based on head (db6dbfd) compared to base (fe75d61).
Patch coverage: 88.23% of modified lines in pull request are covered.

Additional details and impacted files

@@           Coverage Diff           @@
##              dev    #4647   +/-   ##
=======================================
  Coverage   91.50%   91.50%           
=======================================
  Files         381      381           
  Lines       14959    14976   +17     
=======================================
+ Hits        13688    13704   +16     
- Misses       1271     1272    +1     

Impacted Files	Coverage Δ
src/MudBlazor/Components/Tooltip/MudTooltip.razor	100.00% <ø> (ø)
...c/MudBlazor/Components/Tooltip/MudTooltip.razor.cs	94.44% <88.23%> (-3.00%)	⬇️
src/MudBlazor/Components/Chip/MudChip.razor.cs	94.11% <0.00%> (+0.11%)	⬆️
...lazor/Components/FileUpload/MudFileUpload.razor.cs	96.29% <0.00%> (+0.14%)	⬆️
...r/Components/Autocomplete/MudAutocomplete.razor.cs	92.85% <0.00%> (+0.42%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[henon]

I expected that ClickToHide would hide the tooltip when directly clicking on the tooltip. Also, how can one toggle the tooltip by clicking? Can one use both ClickToShow and ClickToHide at the same time? It is impossible right now, right? Wouldn't a single option ClickToToggle be better than ClickToShow and ClickToHide ?

[henon]

Actually a bool ToggleOnClick would be the better API then

[mckaragoz]

Actually a bool ToggleOnClick would be the better API then

Why not. But first i say why i add this, then you decide better. ClickToShow option directly means the toggle thing, ClickToHide is only for hide tooltip (to sure it will always hide). There is two situation i don't know:

1. If there will be racing condition (or focus to open, click to close - so can the click call focus again? I don't know), is there a change to false open the tooltip again instead of closing?
2. Probably there will be more work about touch events in future, click pattern should not conflict with possible touch implementations.

Actually i like the bool parameter. If you say ok, i can change.

[henon]

There can not be a race condition I think. OK, let's do the bool parameter. Click should later just also work for touch too.

[mckaragoz]

Changed to bool parameter and text of docs, could you look at these again?

[henon]

The doc text is ok. The last thing I want to discuss are the side-effects of ToggleOnClick. It is impossible to have a hover tooltip that can be clicked away now?

[mckaragoz]

The doc text is ok. The last thing I want to discuss are the side-effects of ToggleOnClick. It is impossible to have a hover tooltip that can be clicked away now?

As the latest behavior, ToggleOnClick overrides all focus and hover. But i don't understand clicked away part.

[henon]

I know it overrides the others. I just want to know if this is side-effect is absolutely necessary or if it would also work if it would not override the others.

For instance, setting ShowOnFocus = true and ToggleOnClick = true, what if ToggleOnClick has no side-effect? Would it still work? Right now, you can not have both.

[mckaragoz]

I know it overrides the others. I just want to know if this is side-effect is absolutely necessary or if it would also work if it would not override the others.

For instance, setting ShowOnFocus = true and ToggleOnClick = true, what if ToggleOnClick has no side-effect? Would it still work? Right now, you can not have both.

I tried and not see a side effect, at least on standard usage. Then i can offer to simplify this most, remove the "ShowOn" texts and add three parameters Hover Focus Click and each three is independent. If hover is false, the tooltip also doesn't hide when mouse leave etc.

[mckaragoz]

@henon thanks, i like the new API now, its much clear and much fixable if there will be some bugs. I changed the names and behavior. Also changed GitHub and example "Run" button behavior to show tooltip properly.

[henon]

@mckaragoz: So I guess the enum can be removed? And in the docs the Note: about the side effect can be removed.

I am not sure about the naming Click, Hover and Focus, it is not clear from the names what they do. ShowOnClick, ShowOnHover and ShowOnFocus is much more concise. You agree @mikes-gh ?

[mikes-gh]

@mckaragoz: So I guess the enum can be removed? And in the docs the Note: about the side effect can be removed.

I am not sure about the naming Click, Hover and Focus, it is not clear from the names what they do. ShowOnClick, ShowOnHover and ShowOnFocus is much more concise. You agree @mikes-gh ?

Yes you are right Click Hover Focus are actions not behaviours.

[mikes-gh]

Also probably needs tests. Can I just be clear these parameters can all coexist? ShowOnFocus in combination with ShowOnHover is good for accessibility for example with keyboard users without fine motor skills.

[mckaragoz]

Also probably needs tests. Can I just be clear these parameters can all coexist? ShowOnFocus in combination with ShowOnHover is good for accessibility for example with keyboard users without fine motor skills.

In current situation, tooltip already works on hover and on focus, but there is no click option to show/hide. The new one doesn't have breaking changes, it has hover and focus to true, and click to false by default, same behavior.

And yes, all three are possible at the same time, or possible all combinations.

This is important for me because for now there is no option to disable focus option without disabling all tooltip. With this everything is possible :)

[mikes-gh]

In current situation, tooltip already works on hover and on focus, but there is no click option to show/hide. The new one doesn't have breaking changes, it has hover and focus to true, and click to false by default, same behavior.

You've added logic thats not covered by the tests.

Also I would change the Example file name to be consistent with terminology.
TooltipReactExample -> TooltipActivationExample

[mckaragoz]

In current situation, tooltip already works on hover and on focus, but there is no click option to show/hide. The new one doesn't have breaking changes, it has hover and focus to true, and click to false by default, same behavior.

You've added logic thats not covered by the tests.

Also I would change the Example file name to be consistent with terminology.
TooltipReactExample -> TooltipActivationExample

Yeah I will handle all, but henon asks you for the new API naming, should we have "ShowOnHover" or simply "Hover"?

[henon]

@mckaragoz all with ShowOn prefix please

[mckaragoz]

@henon sorry, didn't see the mikes's message on upper, changed names and docs, removed enum, add a click test. And other hover and focus events tests work.

[henon]

Many thanks!