Added new type of scrollable menu #5803
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Booplicate
added
awaiting code review
Before, passing in an area with width less than 560 px, cut off buttons tips Also we forgot to add text_align to the style
multimokia
requested changes
May 20, 2020
maybe not Co-Authored-By: multimokia <[email protected]>
multimokia
previously approved these changes
May 21, 2020
ThePotatoGuy
requested changes
May 22, 2020
ThePotatoGuy
requested changes
May 22, 2020
Co-authored-by: ThePotatoGuy <[email protected]>
ThePotatoGuy
requested changes
May 30, 2020
There was a problem hiding this comment.
I think a better color scheme for selected/unselected is to use the same one that the selectors (and piano config) use - standard pink for not selected, peachy for selected. Especially since we use gray everywhere else to signify that an option is disabled/uninteractable.
But I still think the disabled style looks better here
ThePotatoGuy
approved these changes
Jun 6, 2020
ThePotatoGuy
removed
the
awaiting code review
multimokia
approved these changes
Jun 6, 2020
Booplicate
deleted the
WeAddedCheckboxesToScrollableMenuSoYouCanCheckWhileScrolling
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds a scrollable menu with checkboxes. In theory this should make code more clear by removing label loops that were used for
mas_gen_scrollable_menu. Instead of returning a value, buttons switch it betweenTrue/False. To close the menu the user needs to click the return button. It'll also return a dict with buttons keys corresponding to their values.I used the existing styles for unchecked buttons. If you think they look confusing, we can add a new style for those buttons.
Also while we're in styles, I fixed that old bug which didn't allow to use narrow scrollable menus (<560 px).
Testing
Truevalues, no crashes, buttons checks/unchecks)mas_gen_scrollable_menustill look good, and bothmas_gen_scrollable_menuandmas_check_scrollable_menusupport menus of any width.