-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Add visual component for assigning user tags to beatmaps #32221
Conversation
Visual part for ppy#31913. Opening separately for appropriate visual UI adjustments. Also mostly ready to be hooked up to the results screen, pending merge of ppy/osu-web#11951.
Hmm, I think we're going to need a search box for tags (probably autofocused) selection, I guess that will be a follow-up effort? |
I'd honestly rather than do it here probably. It shouldn't be a huge undertaking. Unless you're uncomfortable with the diff size here I suppose. |
voteCount.Value -= 1; | ||
voted.Value = false; |
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.
I guess the only weird thing I've seen is that a tag returning to 0 count doesn't disappear and instead stays visible with a 0
. Maybe fine, not sure.
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.
Applied in 3d4dd85
Sounds fine. Diff so far is palatable so feel free to go ahead. |
Search box added in 00127b3 |
I've made some changes. Not hugely keen on the number of subclasses, but adjusting the naming helped things become acceptable to my eye. |
That's mostly because I see a lot of these as single-use but point taken 👍 No objections to the changes. |
2025-03-04.13-10-41.mp4
Visual part for #31913. Opening separately for appropriate visual UI adjustments.
Also mostly ready to be hooked up to the results screen, pending merge of ppy/osu-web#11951 - but not hooked up quite yet.