-
-
Notifications
You must be signed in to change notification settings - Fork 5
refactor: popup handling and animation stability #53
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
base: main
Are you sure you want to change the base?
Conversation
TwrblxDevs
commented
Nov 3, 2025
- Added nil-check before accessing currentTheme to prevent potential runtime errors.
- Fixed lingering reference issue by explicitly clearing currentPopup/currentCustomPopup before new assignment.
- Improved InputBegan safety by adding UserInputType check to ignore non-keyboard events.
- Adjusted background transition to use linear damping for smoother fade effect.
- Ensured consistent ZIndex usage across nested elements for better layering stability.
- Added nil-check before accessing currentTheme to prevent potential runtime errors. - Fixed lingering reference issue by explicitly clearing currentPopup/currentCustomPopup before new assignment. - Improved InputBegan safety by adding UserInputType check to ignore non-keyboard events. - Adjusted background transition to use linear damping for smoother fade effect. - Ensured consistent ZIndex usage across nested elements for better layering stability.
|
Forgot to add: General readability and structure improvements (no logic redesign, no variable renaming). |
|
Just some QOL Stuff basically just improving this script and making it easier to read |
parker02311
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the pull request! I have done a quick review before I must head to work. Please take a look over and feel free to ask about any questions and I will get back to you ASAP!
|
Additionally please update your commit messages to follow https://www.conventionalcommits.org/en/v1.0.0/ This can be done using git on your end and force-pushing the commits with new messages tbf I should have added a contributing guide but I also didn't expect anyone to actual contribute |
| } | ||
| local popup = {} :: popup | ||
|
|
||
| local function getColors(valueType: string, theme): {BackgroundColor: Color3, TextColor: Color3, ShadowColor: Color3, ButtonGlowColor: Color3} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
theme should be typed, using RedonUI.theme.theme as the type
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops I'll fix that
| local mt = getmetatable(scope :: any) | ||
| if typeof(mt) == "table" and mt._FUSION_POISONED then | ||
| return | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did this get deleted?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Refer to the other comment.
| local mt = getmetatable(scope :: any) | ||
| if typeof(mt) == "table" and mt._FUSION_POISONED then | ||
| return | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did this get deleted?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoops probably overlooked this on accident as well for the other one too
|
also the commit would be |
My bad github apparently made this title |