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 Request]: Range Slider - Handle Tooltip Hovers #14549

Closed
1 task done
Tracked by #6337
KevinCamelo opened this issue Aug 30, 2023 · 15 comments · Fixed by #15129
Closed
1 task done
Tracked by #6337

[Feature Request]: Range Slider - Handle Tooltip Hovers #14549

KevinCamelo opened this issue Aug 30, 2023 · 15 comments · Fixed by #15129
Labels
component: slider needs: community contribution Due to roadmap and resource availability, we are looking for outside contributions on this issue. proposal: accepted This request has gone through triaging and we are accepting PR's against it. status: waiting for maintainer response 💬 type: enhancement 💡
Milestone

Comments

@KevinCamelo
Copy link

KevinCamelo commented Aug 30, 2023

The problem

The range slider component currently relies on the user to review the left min or right max value to determine what number is being currently selected. Based on a conversation with theDesign Systems Adoption Guild and the Carbon team, tooltip indicators would be a welcome addition to the pattern.

The solution

Upon hover on the minimum or maximum value handle, show a tooltip that displays the number that corresponds to the active handle.

rangehover

Examples

Example Source
Material https://m3.material.io/components/sliders/specs
Flutter https://api.flutter.dev/flutter/material/RangeSlider-class.html

Audit link: https://app.mural.co/t/cloudappservicesdesign3998/m/cloudappservicesdesign3998/1692204645335/c54284851ac0a172eef2d7b2e6bd918416cd738e?sender=ua50b53708bb0fd6ce5c12121

Application/PAL

Carbon for Cloud

Business priority

Low Priority = release date is not dependent on fix or not upcoming

Available extra resources

None, but work has been done on the base version of the Range Slider component via #6337. What needs to be added is the tooltip interaction.

Code of Conduct

@sstrubberg
Copy link
Member

@m4olivei any chance you can add this spec to #14297 ?

Let me know!

@sstrubberg sstrubberg added component: slider proposal: accepted This request has gone through triaging and we are accepting PR's against it. needs: community contribution Due to roadmap and resource availability, we are looking for outside contributions on this issue. and removed status: needs triage 🕵️‍♀️ labels Sep 18, 2023
@github-actions
Copy link
Contributor

The Carbon team has accepted this proposal! Our team doesn’t have the cycles to work on this now, so we are requesting community contributors. Please see the labels for roles that are needed. If you are willing to help out, comment below and we will get in touch!

@m4olivei
Copy link
Contributor

@m4olivei any chance you can add this spec to #14297 ?

Hey @sstrubberg 👋 . @KevinCamelo and I agreed that this could happen as a follow up to #14297 . Is there something that's changed such that this is required?

@sstrubberg
Copy link
Member

We were chatting about it in our backlog grooming call this morning and thought it would be worthwhile to see if you could bring it in to the current work. No worries if not. We can always tackle it later. Do you foresee a lot of changes to incorporate it?

@m4olivei
Copy link
Contributor

@sstrubberg I don't think it would be a lot of changes. Mainly I'm hoping to get what we have committed to avoid continual maintenance of merge conflicts. Already there is a non-trivial merge conflict that I need to resolve on account of upstream changes to the Slider component that were merged recently. It's also true that the team I work on doesn't have a pressing need for this component, so I get allocated to other projects ahead of this. Finding time to work on this is a challenge.

@sstrubberg
Copy link
Member

Totally get it. Get your stuff merged. Based on your availability we'll tackle this next.

@sstrubberg sstrubberg added the planning: umbrella Umbrella issues, surfaced in Projects views label Sep 19, 2023
@tay1orjones tay1orjones added this to the 2023 Q4 milestone Oct 2, 2023
@m4olivei
Copy link
Contributor

m4olivei commented Nov 2, 2023

Hey all 👋 . I have time to pick this up in our current Sprint cycle. I've been experimenting locally. A couple of questions for you all:

  • Would we also like to have tooltips for the single handle slider as well?
  • If yes, I'd propose that it would be opt-in for backward compatibility, adding a tooltips prop to turn on the functionality. Although maybe the argument for tooltips is strong enough here to have it be the default for single and double handle?
  • I presume we want to use the Carbon tooltip component here? The Carbon tooltip component comes with more padding than is depicted here. Are we OK with the Carbon tooltips as is?

image

@KevinCamelo
Copy link
Author

@m4olivei Oh great Matt, excited to see! Chiming in on the one design related question, although I'm a fan of the making this a default option as that was the original intent.

I presume we want to use the Carbon tooltip component here? The Carbon tooltip component comes with more padding than is depicted here. Are we OK with the Carbon tooltips as is?

I wonder if we can use the definition tooltip here for the slightly tighter padding? Is that against policy Carbon design squad? @laurenmrice

@m4olivei
Copy link
Contributor

m4olivei commented Nov 8, 2023

I'm a fan of the making this a default option as that was the original intent.

On second thought, I'm up for that as well. If we can agree on default to tooltips for single and double handle slider, that would be great. I think my main concern is I'd hope for consistency between single and double handle. Since both scenarios are handled in the same component, giving only double handle the tooltips and not single handle, would be hard to maintain.

Maybe we still have a prop to allow consumers to disable tooltips in case it conflicts in some way with their existing application. Would love if Carbon maintainers would chime in on that.

@m4olivei
Copy link
Contributor

m4olivei commented Nov 8, 2023

I opened a PR with progress so far:

https://deploy-preview-15129--v11-carbon-react.netlify.app/?path=/story/components-slider--two-handle-slider

I've assumed here that we turn on tooltips by default for both slider varients. There is also no way to opt out at present, I can add that in as we refine the open questions here. Look forward to any feedback.

@mbgower
Copy link

mbgower commented Nov 9, 2023

What is the rationale of having the hover? It seems redundant to the input field.

If there were no inputs, I see a case for a tip that appeared on focus or hover, but AFAIK, Carbon does not have such a variant.

@mjabbink
Copy link

mjabbink commented Nov 9, 2023

@KevinCamelo What you describe as the “problem” is not a problem. It’s also not an accessibility issue, but rather an information overload, heavy-handed, and what I call “belt and suspenders”—all things we try to avoid.

The animated example visualizes more of a problem—too much information for no “real” benefit. It’s what I call forced benefit. Please closely examine the amount of information/numbers displayed in the tiny area below. I see a lot of numbers and repeating on top of that.

Screenshot 2023-11-09 at 10 06 37 AM

The examples you show do not have input fields with value, so in that case, it makes sense. We will make that an option and NOT the option to do toggle tip with value and input with value.

We will add to our guidance not to use both simultaneously but to create an alternate approach, which we do think.

@KevinCamelo
Copy link
Author

KevinCamelo commented Nov 9, 2023

Hi all. Thanks for the notes here — the range slider squad met today to discuss our actions moving forward .

TLDR:

  • The range slider component will now use tooltips in an either/or scenario. If text-inputs are present tooltips will not be displayed. If no text-inputs are present, tooltips will be displayed on hover, focus, and selection.
  • This logic will extend to the single slider.
  • A future enhancement is planned for changes to the way the line segment in a range slider is presented.

Tooltips:

Instead of adding tooltips alongside the text inputs, the decision for product teams will be either/or.

  • If a team decides to use the text-input, they will not get the tooltip hover.
  • If a team disables the text-inputs, the tooltips will appear.

The tooltip style used will be the style provided by theicon variant of the tooltip.

@m4olivei here is a diagram of the requested design changes. @laurenmrice could you please sign off on this implementation?

Screenshot 2023-11-09 at 2 04 35 PM

Line segment updates

Open issues mentioned during the call was a lack of clarity of when the line starts or ends and the difficulty in seeing the line-segment (that represents the range) behind the selected line.

  • An example was presented with using the gray 30 token to make it easier to understand where the line segment starts and ends.
  • Michael Gower also shared an example of how Carbon Charts starts and ends its line segments.
  • This will be acted on in a future enhancement to this component.

@laurenmrice
Copy link
Member

@KevinCamelo Yes, I sign off on this. Thank you for documenting what we talked about in our meeting.

@m4olivei
Copy link
Contributor

Thanks for the detailed notes @KevinCamelo . I think I've covered off all the requirements in the PR. There are detailed testing instructions in the PR, but in short, I've added a couple stories to highlight the tooltips use case:

Be sure to review other stories for regressions in both Slider and Tooltip components as there were changes necessary in both places to fulfill the requirements.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: slider needs: community contribution Due to roadmap and resource availability, we are looking for outside contributions on this issue. proposal: accepted This request has gone through triaging and we are accepting PR's against it. status: waiting for maintainer response 💬 type: enhancement 💡
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

7 participants