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

bug(mat-slider): Max value not updating correctly when new value is 0 #23913

Closed
1 task
YSFKBDY opened this issue Nov 6, 2021 · 8 comments · Fixed by #24218
Closed
1 task

bug(mat-slider): Max value not updating correctly when new value is 0 #23913

YSFKBDY opened this issue Nov 6, 2021 · 8 comments · Fixed by #24218
Labels
area: material/slider good first issue This issue is a good place to start for first time contributors to the project help wanted The team would appreciate a PR from the community to address this issue P4 A relatively minor issue that is not relevant to core functions

Comments

@YSFKBDY
Copy link

YSFKBDY commented Nov 6, 2021

Is this a regression?

  • Yes, this behavior used to work in the previous version

The previous version in which this bug was not present was

No response

Description

I am changing slider's max value in my component while/after doing some works. And it seems it has a problem updating visually when new max value is 0.

Reproduction

Steps to reproduce:

  1. Define slider like this: <mat-slider min="0" [displayWith]="displayValue" [max]="maxValue" step="1" [thumbLabel]="true" [tickInterval]="1" [(ngModel)]="sliderValue" (ngModelChange)="changeSliderValue()"></mat-slider>
  2. Default sliderValue is 0, maxValue is 0.
  3. Click a button and change maxValue - let's say to 6. Updates slider and ticks correctly.
  4. Click another button and change maxValue to 0. Updates slider and ticks incorrectly, shows previous ticks.
  5. You can see the same bug here: https://material.angular.io/components/slider/examples

Expected Behavior

Updating slider and ticks correctly.

Actual Behavior

Updated slider and ticks incorrectly.

Environment

  • Angular: 10.2.0
  • CDK/Material: 11.0.2
  • Browser(s): Chome
  • Operating System (e.g. Windows, macOS, Ubuntu): Windows 10 x64
@YSFKBDY YSFKBDY added the needs triage This issue needs to be triaged by the team label Nov 6, 2021
@jelbourn jelbourn added area: material/slider good first issue This issue is a good place to start for first time contributors to the project help wanted The team would appreciate a PR from the community to address this issue P4 A relatively minor issue that is not relevant to core functions and removed needs triage This issue needs to be triaged by the team labels Nov 18, 2021
@jelbourn
Copy link
Member

Confirmed that the ticks do not update after setting the max to zero. This actually happens both in the current and MDC-based slider (which is impressive because they're completely different implementations).

@DMezhenskyi
Copy link
Contributor

Hi! Is anybody working on it? Otherwise I could try to fix it if you don’t mind

@jelbourn
Copy link
Member

jelbourn commented Dec 9, 2021

@DMezhenskyi Feel free to send a PR

@lukonik
Copy link

lukonik commented Dec 27, 2021

@DMezhenskyi PLZ Let me know, if you are still going to work on this issue and I'll close my PR :)

@lukonik
Copy link

lukonik commented Dec 27, 2021

Apologize for so many closed PRs had some troubles with source control.

@DMezhenskyi
Copy link
Contributor

DMezhenskyi commented Dec 27, 2021

@DMezhenskyi PLZ Let me know, if you are still going to work on this issue and I'll close my PR :)

Hi @lukonik! If this fix is urgent then I do not mind if your solution eventually will be merged. My fix is ready, however, I can deliver my PR in about 3 weeks when I get back from vacation.

P.s from what I remember in my solution, besides the case you covered in your PR, there is also a method _updateTickIntervalPercent where happens division by zero if min and max values are the same. If it happens, the result of the calculation will be “Infinity” which is not a valid CSS value for the tick’s background-size property, so it is also a cause of wrongly repainted ticks.

@lukonik
Copy link

lukonik commented Dec 28, 2021

@DMezhenskyi PLZ Let me know, if you are still going to work on this issue and I'll close my PR :)

Hi @lukonik! If this fix is urgent then I do not mind if your solution eventually will be merged. My fix is ready, however, I can deliver my PR in about 3 weeks when I get back from vacation.

P.s from what I remember in my solution, besides the case you covered in your PR, there is also a method _updateTickIntervalPercent where happens division by zero if min and max values are the same. If it happens, the result of the calculation will be “Infinity” which is not a valid CSS value for the tick’s background-size property, so it is also a cause of wrongly repainted ticks.

Hey, thx for your reply.
it's not urgent at all, for me at least :). I'll close my PR then and feel free to push your one .

amysorto pushed a commit that referenced this issue Jan 28, 2022
…4218)

* fix(material/slider): Ticks updated wrongly if the max property changed to 0

When we get equal min & max value for the slider then we can get division by zero which leads to an improper style value for the background-size (Infinity%). This fix introduces a tiny function that checks if we deal with finite numbers during calculations where potentially division by zero may occur.

Fixes #23913

* fix(material/slider): Check if interval persentage a valid number

Fixes #23913

(cherry picked from commit c1f25bc)
amysorto pushed a commit that referenced this issue Jan 28, 2022
…4218)

* fix(material/slider): Ticks updated wrongly if the max property changed to 0

When we get equal min & max value for the slider then we can get division by zero which leads to an improper style value for the background-size (Infinity%). This fix introduces a tiny function that checks if we deal with finite numbers during calculations where potentially division by zero may occur.

Fixes #23913

* fix(material/slider): Check if interval persentage a valid number

Fixes #23913
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Feb 28, 2022
forsti0506 pushed a commit to forsti0506/components that referenced this issue Apr 3, 2022
…gular#24218)

* fix(material/slider): Ticks updated wrongly if the max property changed to 0

When we get equal min & max value for the slider then we can get division by zero which leads to an improper style value for the background-size (Infinity%). This fix introduces a tiny function that checks if we deal with finite numbers during calculations where potentially division by zero may occur.

Fixes angular#23913

* fix(material/slider): Check if interval persentage a valid number

Fixes angular#23913
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: material/slider good first issue This issue is a good place to start for first time contributors to the project help wanted The team would appreciate a PR from the community to address this issue P4 A relatively minor issue that is not relevant to core functions
Projects
None yet
4 participants