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

Slider: fix modus slider selection #3026

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

SuruthiShriP
Copy link
Contributor

@SuruthiShriP SuruthiShriP commented Nov 25, 2024

Description

There is no difference in the visibility of selected and non selected part of the slider as
the entire part is shown in blue color.

https://deploy-preview-3026--moduswebcomponents.netlify.app/?path=/story/components-slider--default

References #2717

@SuruthiShriP SuruthiShriP requested review from coliff and a team as code owners November 25, 2024 15:05

This comment was marked as duplicate.

@coliff

This comment was marked as resolved.

@coliff coliff changed the title fix modus slider selection Slider: fix modus slider selection Nov 25, 2024
@coliff coliff marked this pull request as draft November 25, 2024 15:19
@SuruthiShriP SuruthiShriP marked this pull request as ready for review November 25, 2024 15:22
@coliff

This comment was marked as resolved.

@SuruthiShriP SuruthiShriP force-pushed the issue-2717/fix-slider-selection branch from 5f3dfaf to d23f7ba Compare November 25, 2024 16:28
@SuruthiShriP

This comment was marked as resolved.

@coliff coliff force-pushed the issue-2717/fix-slider-selection branch from d23f7ba to a345d9b Compare November 26, 2024 06:10
Copy link

@jewel-shajan jewel-shajan left a comment

Choose a reason for hiding this comment

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

For disabled state UI change is incorrect.
image In the Changed PR
image Current Version

@coliff
Copy link
Member

coliff commented Nov 26, 2024

Yes, the disabled state should be gray. The slider design in this PR is good, an improvement, but it's actually different to what appears on the Figma file (which doesn't have a different color for the selected range).

We need input from: @enowak1031 and @mitch-trimble if we can proceed with this.

image

@SuruthiShriP
Copy link
Contributor Author

image
does this makes sense?

@coliff coliff marked this pull request as draft November 29, 2024 08:31
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.

3 participants