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

Rando V3 - Small UI Cleanup #4367

Merged
merged 2 commits into from
Sep 28, 2024

Conversation

Malkierian
Copy link
Contributor

@Malkierian Malkierian commented Sep 27, 2024

Rando's Combobox and Slider Options were set to decrement when the saved value was bigger than the available options, but if the difference was more than 1, it would still be trying to reference an option out of bounds, thus displaying garbage data in the label, also causing a crash. This changes those two to just set it to the max directly to avoid this.

Also changes the tristate "Off" graphic to be nothing instead of the cross, to avoid confusion thinking that was enabled.

Build Artifacts

Modify Combobox and Slider Options to clamp directly to options.size() - 1 instead of just decrementing if current value is higher than max.
Copy link
Contributor

@Pepe20129 Pepe20129 left a comment

Choose a reason for hiding this comment

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

Looks good.
I've just though that maybe we should eventually have a setting to change the checkbox to have an X when disabled again but that's something for the future when more "UI settings" are implemented (like accent color, configurable padding/text size, etc.).

@Malkierian
Copy link
Contributor Author

Yeah, this was more for consistency, as normal checkboxes being off are blank, but the cross is used for actually disabled options, so there were actually two possible angles for confusion with the way the tristates were.

@Malkierian Malkierian merged commit 668f0fe into HarbourMasters:develop-rando Sep 28, 2024
5 checks passed
@Malkierian Malkierian deleted the ui-cleanup branch September 28, 2024 05:47
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.

2 participants