Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add visual component for assigning user tags to beatmaps #32221

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

bdach
Copy link
Collaborator

@bdach bdach commented Mar 4, 2025

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.

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.
@peppy peppy self-requested a review March 8, 2025 11:57
@peppy
Copy link
Member

peppy commented Mar 8, 2025

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?

@bdach
Copy link
Collaborator Author

bdach commented Mar 8, 2025

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.

Comment on lines +377 to +378
voteCount.Value -= 1;
voted.Value = false;
Copy link
Member

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.

@peppy
Copy link
Member

peppy commented Mar 9, 2025

I'd honestly rather than do it here probably. It shouldn't be a huge undertaking.

Sounds fine. Diff so far is palatable so feel free to go ahead.

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants