-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Rework user beatmap tag picker to match web #36989
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: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,16 +10,16 @@ | |
| using osu.Framework.Graphics; | ||
| using osu.Framework.Graphics.Containers; | ||
| using osu.Framework.Graphics.Shapes; | ||
| using osu.Framework.Graphics.Sprites; | ||
| using osu.Framework.Input.Events; | ||
| using osu.Framework.Localisation; | ||
| using osu.Framework.Testing; | ||
| using osu.Game.Graphics; | ||
| using osu.Game.Graphics.Containers; | ||
| using osu.Game.Graphics.Sprites; | ||
| using osu.Game.Graphics.UserInterface; | ||
| using osu.Game.Graphics.UserInterfaceV2; | ||
| using osu.Game.Input.Bindings; | ||
| using osu.Game.Screens.Ranking.Statistics; | ||
| using osu.Game.Overlays; | ||
| using osuTK; | ||
|
|
||
| namespace osu.Game.Screens.Ranking | ||
|
|
@@ -37,6 +37,11 @@ private partial class AddTagsPopover : OsuPopover | |
|
|
||
| private CancellationTokenSource? loadCancellationTokenSource; | ||
|
|
||
| public AddTagsPopover() | ||
| : base(withPadding: false) | ||
| { | ||
| } | ||
|
|
||
| [BackgroundDependencyLoader] | ||
| private void load() | ||
| { | ||
|
|
@@ -46,32 +51,38 @@ private void load() | |
| Anchor.BottomCentre, | ||
| }; | ||
|
|
||
| Content.Padding = new MarginPadding { Vertical = 20 }; | ||
|
|
||
| Children = new Drawable[] | ||
| { | ||
| new Container | ||
| { | ||
| Size = new Vector2(400, 300), | ||
| Children = new Drawable[] | ||
| { | ||
| searchBox = new SearchTextBox | ||
| new Container | ||
| { | ||
| HoldFocus = true, | ||
| RelativeSizeAxes = Axes.X, | ||
| Depth = float.MinValue, | ||
| Padding = new MarginPadding { Horizontal = 20 }, | ||
| Child = searchBox = new SearchTextBox | ||
| { | ||
| HoldFocus = true, | ||
| RelativeSizeAxes = Axes.X, | ||
| Depth = float.MinValue, | ||
| }, | ||
| }, | ||
| new OsuScrollContainer | ||
| { | ||
| RelativeSizeAxes = Axes.X, | ||
| Y = 40, | ||
| Height = 260, | ||
| ScrollbarOverlapsContent = false, | ||
| Child = searchContainer = new SearchContainer | ||
| { | ||
| RelativeSizeAxes = Axes.X, | ||
| AutoSizeAxes = Axes.Y, | ||
| Padding = new MarginPadding { Right = 5, Bottom = 10 }, | ||
| Padding = new MarginPadding { Horizontal = 20 }, | ||
| Direction = FillDirection.Vertical, | ||
| Spacing = new Vector2(10), | ||
| Spacing = new Vector2(2.5f), | ||
| } | ||
| } | ||
| }, | ||
|
|
@@ -135,6 +146,8 @@ private partial class GroupFlow : FillFlowContainer, IFilterable | |
| { | ||
| public IEnumerable<LocalisableString> FilterTerms { get; } | ||
|
|
||
| private readonly Circle circle; | ||
|
|
||
| public bool MatchingFilter | ||
| { | ||
| set => Alpha = value ? 1 : 0; | ||
|
|
@@ -150,20 +163,49 @@ public GroupFlow(string? name) | |
| RelativeSizeAxes = Axes.X; | ||
| AutoSizeAxes = Axes.Y; | ||
| Direction = FillDirection.Vertical; | ||
| Spacing = new Vector2(5); | ||
| Spacing = new Vector2(2.5f); | ||
|
|
||
| Add(new StatisticItemHeader { Text = name ?? "uncategorised" }); | ||
| Add(new FillFlowContainer | ||
| { | ||
| RelativeSizeAxes = Axes.X, | ||
| AutoSizeAxes = Axes.Y, | ||
| Direction = FillDirection.Horizontal, | ||
| Spacing = new Vector2(5, 0), | ||
| Margin = new MarginPadding { Vertical = 2.5f }, | ||
| Children = new Drawable[] | ||
| { | ||
| circle = new Circle | ||
| { | ||
| Anchor = Anchor.CentreLeft, | ||
| Origin = Anchor.CentreLeft, | ||
| Height = 12, | ||
| Width = 6, | ||
| }, | ||
| new OsuSpriteText | ||
| { | ||
| Anchor = Anchor.CentreLeft, | ||
| Origin = Anchor.CentreLeft, | ||
| Text = name ?? "uncategorized", | ||
| Font = OsuFont.Style.Heading2, | ||
| } | ||
| }, | ||
| }); | ||
|
Comment on lines
+168
to
+192
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've detached this from Maybe it'll worth it to eventually do that, given that I've seen this type of header used on the new results screen as well.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I much prefer ^C^V to weird twiddles on things that should never exist for sure. You're not going to hear a complaint about this from me. |
||
|
|
||
| FilterTerms = name == null ? [] : [name]; | ||
| } | ||
|
|
||
| [BackgroundDependencyLoader] | ||
| private void load(OverlayColourProvider colourProvider) | ||
| { | ||
| circle.Colour = colourProvider.Highlight1; | ||
| } | ||
| } | ||
|
|
||
| private partial class DrawableAddableTag : OsuAnimatedButton, IFilterable | ||
| { | ||
| public readonly UserTag Tag; | ||
|
|
||
| private Box votedBackground = null!; | ||
| private SpriteIcon votedIcon = null!; | ||
| private Box background = null!; | ||
|
|
||
| private readonly Bindable<bool> voted = new Bindable<bool>(); | ||
| private readonly BindableBool updating = new BindableBool(); | ||
|
|
@@ -177,69 +219,65 @@ public DrawableAddableTag(UserTag tag) | |
| RelativeSizeAxes = Axes.X; | ||
| AutoSizeAxes = Axes.Y; | ||
|
|
||
| Content.CornerRadius = 4; | ||
|
|
||
| ScaleOnMouseDown = 0.95f; | ||
|
|
||
| voted.BindTo(Tag.Voted); | ||
| updating.BindTo(Tag.Updating); | ||
| } | ||
|
|
||
| [Resolved] | ||
| private OsuColour colours { get; set; } = null!; | ||
| private OverlayColourProvider colourProvider { get; set; } = null!; | ||
|
|
||
| [BackgroundDependencyLoader] | ||
| private void load() | ||
| { | ||
| Content.AddRange(new Drawable[] | ||
| { | ||
| new Box | ||
| background = new Box | ||
| { | ||
| RelativeSizeAxes = Axes.Both, | ||
| Colour = colours.Gray7, | ||
| Colour = colourProvider.Background3, | ||
| Depth = float.MaxValue, | ||
| }, | ||
| new Container | ||
| { | ||
| RelativeSizeAxes = Axes.Y, | ||
| Width = 30, | ||
| Anchor = Anchor.CentreRight, | ||
| Origin = Anchor.CentreRight, | ||
| Depth = float.MaxValue, | ||
| Children = new Drawable[] | ||
| { | ||
| votedBackground = new Box | ||
| { | ||
| RelativeSizeAxes = Axes.Both, | ||
| }, | ||
| votedIcon = new SpriteIcon | ||
| { | ||
| Size = new Vector2(16), | ||
| Icon = FontAwesome.Solid.ThumbsUp, | ||
| Anchor = Anchor.Centre, | ||
| Origin = Anchor.Centre, | ||
| } | ||
| } | ||
| }, | ||
| new FillFlowContainer | ||
| new GridContainer | ||
| { | ||
| RelativeSizeAxes = Axes.X, | ||
| AutoSizeAxes = Axes.Y, | ||
| Direction = FillDirection.Vertical, | ||
| Spacing = new Vector2(2), | ||
| Padding = new MarginPadding(5) { Right = 35 }, | ||
| Children = new Drawable[] | ||
| Padding = new MarginPadding { Horizontal = 10, Vertical = 5 }, | ||
| ColumnDimensions = new[] | ||
| { | ||
| new OsuTextFlowContainer(t => t.Font = OsuFont.Default.With(weight: FontWeight.SemiBold)) | ||
| new Dimension(GridSizeMode.Absolute, 100), | ||
| new Dimension(GridSizeMode.Absolute, 10), | ||
| new Dimension(), | ||
| }, | ||
| RowDimensions = new[] | ||
| { | ||
| new Dimension(GridSizeMode.AutoSize), | ||
| }, | ||
| Content = new[] | ||
| { | ||
| new Drawable?[] | ||
| { | ||
| RelativeSizeAxes = Axes.X, | ||
| AutoSizeAxes = Axes.Y, | ||
| Text = Tag.DisplayName, | ||
| new OsuTextFlowContainer(t => t.Font = OsuFont.Style.Caption1.With(weight: FontWeight.Bold)) | ||
| { | ||
| Anchor = Anchor.CentreLeft, | ||
| Origin = Anchor.CentreLeft, | ||
| RelativeSizeAxes = Axes.X, | ||
| AutoSizeAxes = Axes.Y, | ||
| Text = Tag.DisplayName, | ||
| }, | ||
| null, | ||
| new OsuTextFlowContainer(t => t.Font = OsuFont.Style.Caption2) | ||
| { | ||
| Anchor = Anchor.CentreLeft, | ||
| Origin = Anchor.CentreLeft, | ||
| RelativeSizeAxes = Axes.X, | ||
| AutoSizeAxes = Axes.Y, | ||
| Text = Tag.Description, | ||
| } | ||
| }, | ||
| new OsuTextFlowContainer(t => t.Font = OsuFont.Default.With(size: 14)) | ||
| { | ||
| RelativeSizeAxes = Axes.X, | ||
| AutoSizeAxes = Axes.Y, | ||
| Text = Tag.Description, | ||
| } | ||
| } | ||
| }, | ||
| loadingLayer = new LoadingLayer(dimBackground: true), | ||
|
|
@@ -264,8 +302,7 @@ protected override void LoadComplete() | |
|
|
||
| voted.BindValueChanged(_ => | ||
| { | ||
| votedBackground.FadeColour(voted.Value ? colours.Lime2 : colours.Gray2, 250, Easing.OutQuint); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't wanna be caught dead defending my dogshit programmer design but I also can't lie that this particular colour change bothers me a little. The intent of using lime was to very unambiguously highlight the voted state. And it was also supposed to match the list of tags too, in a way that visually very unambiguously links the two:
You get two splotches of lime that correspond to each other. Now, with only one of the two places adjusted, this visual association is severed and the result seems worse to me:
The obvious solution would be to apply the same colour to the tags as well, so I tried that and... it looks kinda sauceless..?
It's probably because it starts to encroach on the "add" button's colour and is not distinguished as much anymore. This is where I would turn to a designer for help, but we don't have one, so I'm going to summon @peppy instead to play the role of benevolent UI/UX dictator.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With all that in mind I'm not really sure if it was a good idea to get this design applied here. I guess it works better on the website, since there's no tag buttons to be tied with. This was a quick change intended to be used later when I add the picker on the beatmap listing overlay, so it's fine if it's skipped here for now (or at all, if those two pickers should have different layouts altogether).
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's worthwhile to update this still because the current one in master has dogwater colour choices. Maybe this? Screen.Recording.2026-03-19.at.13.57.36.movdiff --git a/osu.Game/Screens/Ranking/UserTagControl.AddTagsPopover.cs b/osu.Game/Screens/Ranking/UserTagControl.AddTagsPopover.cs
index 2147d473a3..db529bdebe 100644
--- a/osu.Game/Screens/Ranking/UserTagControl.AddTagsPopover.cs
+++ b/osu.Game/Screens/Ranking/UserTagControl.AddTagsPopover.cs
@@ -206,6 +206,7 @@ private partial class DrawableAddableTag : OsuAnimatedButton, IFilterable
public readonly UserTag Tag;
private Box background = null!;
+ private GridContainer grid = null!;
private readonly Bindable<bool> voted = new Bindable<bool>();
private readonly BindableBool updating = new BindableBool();
@@ -230,6 +231,9 @@ public DrawableAddableTag(UserTag tag)
[Resolved]
private OverlayColourProvider colourProvider { get; set; } = null!;
+ [Resolved]
+ private OsuColour colours { get; set; } = null!;
+
[BackgroundDependencyLoader]
private void load()
{
@@ -241,7 +245,7 @@ private void load()
Colour = colourProvider.Background3,
Depth = float.MaxValue,
},
- new GridContainer
+ grid = new GridContainer
{
RelativeSizeAxes = Axes.X,
AutoSizeAxes = Axes.Y,
@@ -302,7 +306,8 @@ protected override void LoadComplete()
voted.BindValueChanged(_ =>
{
- background.FadeColour(voted.Value ? colourProvider.Highlight2 : colourProvider.Background3, 250, Easing.OutQuint);
+ background.FadeColour(voted.Value ? colours.Lime2 : colourProvider.Background3, 250, Easing.OutQuint);
+ grid.FadeColour(voted.Value ? Colour4.Black : Colour4.White, 250, Easing.OutQuint);
}, true);
FinishTransforms(true);
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It could use a different colour for the activated state, the black text looks bad on lime especially with the added drop shadow. Also I feel that lime is standing out too much when so much of it is shown in the list, the highlight colour was much more toned down in comparison. If anything I'd go for the second option, especially since web wants to go in that direction, and maybe try making the add button use a different more standout colour? I've tried flipping it around and made the tag buttons use the highlight colour while the add button is lime:
If going in this direction I guess the tag buttons could also be changed to use
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| votedIcon.FadeColour(voted.Value ? Colour4.Black : Colour4.White, 250, Easing.OutQuint); | ||
| background.FadeColour(voted.Value ? colourProvider.Highlight2 : colourProvider.Background3, 250, Easing.OutQuint); | ||
| }, true); | ||
| FinishTransforms(true); | ||
|
|
||
|
|
||





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.
taken from https://github.com/ppy/osu-web/blob/master/resources/css/colors.less#L79