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: Slider put in Canvas NaN Exception #2604

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

TranquilAbyss
Copy link
Contributor

PR Details

Handled Slider UIElement measurement when availableSizeWithoutMargins is infinity. This was done by setting it to max value. The UI math after this cannot handle infinity values. Also other UI Elements provide real numbers for desired size.

I have considered multiple values to put in as the default. But when the the UI Editor Slider Layout section is first opened the visual width is set to a high number, maybe max float. Due to canvas not having a limited size the Slider lacks the support to handle what to do in this situation. When the Layout is open the availableSizeWithoutMargins.X value becomes a high number, meaning that value is originally from the parent, this is is why I chose float.Max.

It also worth noting that there is still a bug behind the original bug. The first slider put into a canvas wont resize on width change until it is re-selected. I think most UI elements measure override to smallest desired size, while the slider appears to use the availableSizeWithoutMargins which is infinity for this issue configuration.

I plan on leaving the issue like this. Unless someone has a better default, please test this solution before suggesting.

Related Issue

#2236

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My change requires a change to the documentation.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • I have built and run the editor to try this change out.

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.

1 participant