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

ColorPicker Tooltip doesn't disappear when focused #3237

Closed
michael-hawker opened this issue Sep 3, 2020 · 15 comments
Closed

ColorPicker Tooltip doesn't disappear when focused #3237

michael-hawker opened this issue Sep 3, 2020 · 15 comments

Comments

@michael-hawker
Copy link
Collaborator

Describe the bug
When using the ColorPicker, the tooltip doesn't disappear unless the user clicks somewhere else that takes focus. This causes the tooltip to overlay itself when scrolling. This can be seen pretty easily in the Control Gallery Sample.

image

Steps to reproduce the bug

  1. Go to control gallery sample for ColorPicker
  2. Click and drag on the color picker to cause tooltip to appear
  3. Scroll with mouse wheel
  4. Note: Tooltip is now obscuring other parts of the page.

A similar issue appears if you later also give ColorPicker focus again and then scroll, as it seems whenever ColorPicker has focus it displays the Tooltip.

Expected behavior
The tooltip should disappear after a short time after the user has finished manipulating the selection.

Version Info
Controls Gallery Package

Windows 10 version Saw the problem?
Insider Build (xxxxx)
May 2020 Update (19041) Yes
November 2019 Update (18363)
May 2019 Update (18362)
October 2018 Update (17763)
April 2018 Update (17134)
Fall Creators Update (16299)
Creators Update (15063)
Device form factor Saw the problem?
Desktop Yes
Xbox
Surface Hub
IoT
@msft-github-bot msft-github-bot added the needs-triage Issue needs to be triaged by the area owners label Sep 3, 2020
@StephenLPeters StephenLPeters added area-ColorPicker team-Controls Issue for the Controls team and removed needs-triage Issue needs to be triaged by the area owners labels Sep 3, 2020
@StephenLPeters
Copy link
Contributor

@kikisaints I think we could probably do something with effectiveViewPortChanged to fix this but some thought would need to go into the desired behavior.

@Felix-Dev
Copy link
Contributor

@StephenLPeters What speaks against collapsing the tooltip as soon as the user starts scrolling? The tooltip wouldn't be aligned with the "actual" color picker (the circle users can drag around the color panel to select a color) any longer and it rapidly starts to look visually unpleasing.

@StephenLPeters
Copy link
Contributor

StephenLPeters commented Sep 4, 2020

I think the options are either close the tip or move it with the color spectrum. you are right we wouldn't want it to be misaligned (as it is currently). And if we decide to move there are questions around what to do when it scrolls off screen and stuff like that.

@marcelwgn
Copy link
Contributor

I think closing the tooltip makes more sense, otherwise we would need to check when the colorspectrum is actually visible on screen and didn't get occluded by something else as part of scrolling.

@Felix-Dev
Copy link
Contributor

Felix-Dev commented Sep 4, 2020

My suggestion is that the tooltip should be

  • shown when the user moves the circle (either by dragging it or changing its position on the color spectrum panel via a click)
  • closed after the default WinUI ToolTip show duration time has passed with the color picker circle not being moved (as suggested by @michael-hawker)
  • closed as soon as the user starts scrolling (if it is still visible).

If we only show the ToolTip for a few seconds anyway (if not currently being dragged) then we don't really have to take care of preserving the tooltip when scrolling. If the user scrolls away from the color picker, they are moving their focus elsewhere so we can close the tooltip just fine. Any additional inconveniences caused by it not disappearing "fast enough" when the user scrolls reasonably fast would thus be eliminated.

@StephenLPeters
Copy link
Contributor

I think closing the tooltip makes more sense, otherwise we would need to check when the colorspectrum is actually visible on screen and didn't get occluded by something else as part of scrolling.

EffectiveViewportChanged lets us do just that though

@marcelwgn
Copy link
Contributor

That's true yes. Should we follow the TeachingTip behavior in this case? A persistent ToolTip like in this case seems is quite similar to a TeachingTip in this regard.

@Felix-Dev
Copy link
Contributor

Felix-Dev commented Sep 5, 2020

I implemented my suggestion outlined above in WinUI for better visualization:

colorpicker-example

The tooltip got a default show duration time of 5 seconds (meant to match UWP's default tooltip duration time). Changing the position of the circle either by mouse/touch or keyboard will show the tooltip and the tooltip will be hidden if no movement change has been registered for 5 seconds.

When the user scrolls, the tooltip is hidden. As written above, since we should hide the tooltip anyway if no change happens after some time, there is little reason for us to preserve the tooltip when the user scrolls. A scrolling user is most likely directing their focus away from the color picker (if they interacted with it prior to the scrolling) and thus the overlay nature of the tooltip will cause visual inconveniences if the user scrolls reasonably fast. As the tooltip would disappear after five seconds anyway (since the user is now likely interacting with other UI elements after their scrolling) we prevent the visual inconveniences by hiding it directly.

(The current behavior of showing the tooltip on focus received and hiding it on focus lost remains unchanged.)

Honestly, I think this is looking quite good (minus the visual quality of the color spectrum in the gif).

@michael-hawker @StephenLPeters @chingucoding What do you think?

@Felix-Dev
Copy link
Contributor

@StephenLPeters Your thoughts on the suggestion above (see the GIF)?

@Felix-Dev
Copy link
Contributor

@StephenLPeters Friendly ping.

@michael-hawker
Copy link
Collaborator Author

@Felix-Dev looks good to me as the OP. 🦙❤

@michael-hawker
Copy link
Collaborator Author

@Felix-Dev I thought I remember seeing a PR for something like this? Did it not get linked or am I thinking of something else?

@Poopooracoocoo
Copy link

@StephenLPeters ping

@asklar
Copy link
Member

asklar commented Aug 20, 2021

@Felix-Dev did you send a PR for this yet? if not, can you do so?

@github-actions
Copy link

This issue is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 5 days.

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

No branches or pull requests

7 participants