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

Fix hot reload min/max change breaking sliders #4

Merged
merged 7 commits into from
Feb 1, 2024

Conversation

Boegie19
Copy link
Contributor

@Boegie19 Boegie19 commented Jan 8, 2024

fix hot reload min/max change breaking slider

Problem

if you change the min or max values with a hot reload the slider beaks
The slider will still think the initial min and max variables are the current once even after changing them.
This causes the following problems:

  1. Making it smaller makes you not able to use the full slider.
  2. Making it bigger makes the dot go off screen.

My solution

Change it so that useInstance only sets a percentage with out knowing the min and max value.
This fixes both problems put above.
This means that I changed the value state to a percentage state and calculate the value every frame.

Changed behaver

This fix changes the behaver of the slider once the values are changed.
Before value stayed the same even if the min and max changed
After now the percentage will stay the same meaning the value changes if min and max are changed

@LastTalon LastTalon added this to the v0.4.3 milestone Jan 9, 2024
jackTabsCode
jackTabsCode previously approved these changes Jan 31, 2024
@jackTabsCode
Copy link
Contributor

Please fix formatting then we can merge @Boegie19

@LastTalon LastTalon changed the title fix hot reload min/max change breaking slider Fix hot reload min/max change breaking sliders Jan 31, 2024
Copy link
Contributor

@jackTabsCode jackTabsCode left a comment

Choose a reason for hiding this comment

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

You misspelled percentageValue and setPercentageValue, and I'd advise initialPercent instead of initpercent

@LastTalon
Copy link
Member

I can just go ahead and take care of that myself. It won't change any functionality in the code.

@LastTalon LastTalon merged commit f59e163 into matter-ecs:main Feb 1, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants