ppy / osu

rhythm is just a *click* away!
https://osu.ppy.sh
MIT License
15.36k stars 2.29k forks source link

"Editor section button" hitbox is too small #27474

Closed YesLifer closed 8 months ago

YesLifer commented 8 months ago

Type

Game behaviour

Bug description

The title says it all. Feels pretty uncomfortable since hovering upwards and clicking does nothing. Though hitboxes of buttons in the upper left corner are fine.

Screenshots or videos

https://github.com/ppy/osu/assets/152286401/ee8ebfb0-4209-4ec8-96ba-8ebfcd4f478d

Version

2024.302.1

Logs

compressed-logs.zip

frenzibyte commented 8 months ago

Can look more like a button, similar to the left-side menu bar. I think that also improves visibility of the selected screen.

https://github.com/ppy/osu/assets/22781491/9feddf7d-e8db-47b3-b464-59424f21f27b

diff --git a/osu.Game/Graphics/UserInterface/OsuTabControl.cs b/osu.Game/Graphics/UserInterface/OsuTabControl.cs
index c260c92b43..f24977927f 100644
--- a/osu.Game/Graphics/UserInterface/OsuTabControl.cs
+++ b/osu.Game/Graphics/UserInterface/OsuTabControl.cs
@@ -116,18 +116,18 @@ public Color4 AccentColour
                 }
             }

-            private const float transition_length = 500;
+            protected const float TRANSITION_LENGTH = 500;

-            protected void FadeHovered()
+            protected virtual void FadeHovered()
             {
-                Bar.FadeIn(transition_length, Easing.OutQuint);
-                Text.FadeColour(Color4.White, transition_length, Easing.OutQuint);
+                Bar.FadeIn(TRANSITION_LENGTH, Easing.OutQuint);
+                Text.FadeColour(Color4.White, TRANSITION_LENGTH, Easing.OutQuint);
             }

-            protected void FadeUnhovered()
+            protected virtual void FadeUnhovered()
             {
-                Bar.FadeTo(IsHovered ? 1 : 0, transition_length, Easing.OutQuint);
-                Text.FadeColour(IsHovered ? Color4.White : AccentColour, transition_length, Easing.OutQuint);
+                Bar.FadeTo(IsHovered ? 1 : 0, TRANSITION_LENGTH, Easing.OutQuint);
+                Text.FadeColour(IsHovered ? Color4.White : AccentColour, TRANSITION_LENGTH, Easing.OutQuint);
             }

             protected override bool OnHover(HoverEvent e)
diff --git a/osu.Game/Screens/Edit/Components/Menus/EditorScreenSwitcherControl.cs b/osu.Game/Screens/Edit/Components/Menus/EditorScreenSwitcherControl.cs
index 1f6d61d0ad..0f5c2173b8 100644
--- a/osu.Game/Screens/Edit/Components/Menus/EditorScreenSwitcherControl.cs
+++ b/osu.Game/Screens/Edit/Components/Menus/EditorScreenSwitcherControl.cs
@@ -9,6 +9,7 @@
 using osu.Game.Graphics.UserInterface;
 using osu.Game.Overlays;
 using osuTK;
+using osuTK.Graphics;

 namespace osu.Game.Screens.Edit.Components.Menus
 {
@@ -21,7 +22,7 @@ public EditorScreenSwitcherControl()

             TabContainer.RelativeSizeAxes &= ~Axes.X;
             TabContainer.AutoSizeAxes = Axes.X;
-            TabContainer.Padding = new MarginPadding(10);
+            TabContainer.Spacing = Vector2.Zero;
         }

         [BackgroundDependencyLoader]
@@ -42,30 +43,45 @@ private void load(OverlayColourProvider colourProvider)

         private partial class TabItem : OsuTabItem
         {
-            private const float transition_length = 250;
+            private readonly Box background;
+            private Color4 backgroundIdleColour;
+            private Color4 backgroundHoverColour;

             public TabItem(EditorScreenMode value)
                 : base(value)
             {
-                Text.Margin = new MarginPadding();
+                Text.Margin = new MarginPadding(10);
                 Text.Anchor = Anchor.CentreLeft;
                 Text.Origin = Anchor.CentreLeft;

                 Text.Font = OsuFont.TorusAlternate;

+                Add(background = new Box
+                {
+                    RelativeSizeAxes = Axes.Both,
+                    Depth = float.MaxValue,
+                });
+
                 Bar.Expire();
             }

-            protected override void OnActivated()
+            [BackgroundDependencyLoader]
+            private void load(OverlayColourProvider colourProvider)
+            {
+                backgroundIdleColour = colourProvider.Background2;
+                backgroundHoverColour = colourProvider.Background1;
+            }
+
+            protected override void FadeHovered()
             {
-                base.OnActivated();
-                Bar.ScaleTo(new Vector2(1, 5), transition_length, Easing.OutQuint);
+                base.FadeHovered();
+                background.FadeColour(backgroundHoverColour, TRANSITION_LENGTH, Easing.OutQuint);
             }

-            protected override void OnDeactivated()
+            protected override void FadeUnhovered()
             {
-                base.OnDeactivated();
-                Bar.ScaleTo(Vector2.One, transition_length, Easing.OutQuint);
+                base.FadeUnhovered();
+                background.FadeColour(backgroundIdleColour, TRANSITION_LENGTH, Easing.OutQuint);
             }
         }
     }
peppy commented 8 months ago

Sure, it can be changed visually, but did you fix the top-edge of screen not allowing clicking as pointed out in the original post? For the menu too?

frenzibyte commented 8 months ago

Menu already allows doing that, the selector only suffers from that issue, and the visual change serves to add some sense to the proposed behaviour (making the button clickable from the top edge).