Refactor buttons again to work with latest design guidelines

osu.Game.Tests/Visual/Beatmaps/TestSceneBeatmapCardDownloadButton.cs

@@ -1,10 +1,9 @@
 // Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
 // See the LICENCE file in the repository root for full licence text.
 
-using System.Linq;
+using System;
 using NUnit.Framework;
 using osu.Framework.Allocation;
-using osu.Framework.Bindables;
 using osu.Framework.Graphics;
 using osu.Game.Beatmaps;
 using osu.Game.Beatmaps.Drawables.Cards.Buttons;
@@ -14,50 +13,20 @@ using osu.Game.Online.API.Requests.Responses;
 using osu.Game.Overlays;
 using osu.Game.Resources.Localisation.Web;
 using osu.Game.Rulesets.Osu;
-using osu.Game.Tests.Resources;
 using osuTK;
 
 namespace osu.Game.Tests.Visual.Beatmaps
 {
     public class TestSceneBeatmapCardDownloadButton : OsuTestScene
     {
-        private TestDownloadButton downloadButton;
+        private DownloadButton downloadButton;
 
         [Cached]
         private OverlayColourProvider colourProvider = new OverlayColourProvider(OverlayColourScheme.Blue);
 
-        [Resolved]
-        private BeatmapManager beatmaps { get; set; }
-
         [Resolved]
         private OsuConfigManager config { get; set; }
 
-        [Test]
-        public void TestDownloadableBeatmap()
-        {
-            ensureSoleilyRemoved();
-            createButton(true);
-
-            assertDownloadVisible(true);
-            assertDownloadEnabled(true);
-            assertPlayVisible(false);
-            AddAssert("tooltip text correct", () => downloadButton.Download.TooltipText == BeatmapsetsStrings.PanelDownloadAll);
-
-            AddStep("set downloading state", () => downloadButton.State.Value = DownloadState.Downloading);
-            assertDownloadVisible(true);
-            assertDownloadEnabled(false);
-            assertPlayVisible(false);
-
-            AddStep("set importing state", () => downloadButton.State.Value = DownloadState.Importing);
-            assertDownloadVisible(true);
-            assertDownloadEnabled(false);
-            assertPlayVisible(false);
-
-            AddStep("set locally available state", () => downloadButton.State.Value = DownloadState.LocallyAvailable);
-            assertDownloadVisible(false);
-            assertPlayVisible(true);
-        }
-
         [Test]
         public void TestDownloadableBeatmapWithVideo()
         {
@@ -65,10 +34,10 @@ namespace osu.Game.Tests.Visual.Beatmaps
             assertDownloadEnabled(true);
 
             AddStep("prefer no video", () => config.SetValue(OsuSetting.PreferNoVideo, true));
-            AddAssert("tooltip text correct", () => downloadButton.Download.TooltipText == BeatmapsetsStrings.PanelDownloadNoVideo);
+            AddAssert("tooltip text correct", () => downloadButton.TooltipText == BeatmapsetsStrings.PanelDownloadNoVideo);
 
             AddStep("prefer video", () => config.SetValue(OsuSetting.PreferNoVideo, false));
-            AddAssert("tooltip text correct", () => downloadButton.Download.TooltipText == BeatmapsetsStrings.PanelDownloadVideo);
+            AddAssert("tooltip text correct", () => downloadButton.TooltipText == BeatmapsetsStrings.PanelDownloadVideo);
         }
 
         [Test]
@@ -76,74 +45,31 @@ namespace osu.Game.Tests.Visual.Beatmaps
         {
             createButton(false);
             assertDownloadEnabled(false);
-            AddAssert("tooltip text correct", () => downloadButton.Download.TooltipText == BeatmapsetsStrings.AvailabilityDisabled);
+            AddAssert("tooltip text correct", () => downloadButton.TooltipText == BeatmapsetsStrings.AvailabilityDisabled);
         }
 
-        [Test]
-        public void TestDownloadState()
-        {
-            ensureSoleilyRemoved();
-            createButtonWithBeatmap(createSoleily());
-            AddAssert("button state not downloaded", () => downloadButton.State.Value == DownloadState.NotDownloaded);
-            AddStep("import soleily", () => beatmaps.Import(TestResources.GetQuickTestBeatmapForImport()));
-
-            AddUntilStep("wait for beatmap import", () => beatmaps.GetAllUsableBeatmapSets().Any(b => b.OnlineID == 241526));
-            AddUntilStep("button state downloaded", () => downloadButton.State.Value == DownloadState.LocallyAvailable);
-
-            createButtonWithBeatmap(createSoleily());
-            AddUntilStep("button state downloaded", () => downloadButton.State.Value == DownloadState.LocallyAvailable);
-            ensureSoleilyRemoved();
-            AddUntilStep("button state not downloaded", () => downloadButton.State.Value == DownloadState.NotDownloaded);
-        }
-
-        private void ensureSoleilyRemoved()
-        {
-            AddUntilStep("ensure manager loaded", () => beatmaps != null);
-            AddStep("remove soleily", () =>
-            {
-                var beatmap = beatmaps.QueryBeatmapSet(b => b.OnlineID == 241526);
-
-                if (beatmap != null) beatmaps.Delete(beatmap);
-            });
-        }
-
-        private void assertDownloadVisible(bool visible) => AddUntilStep($"download {(visible ? "visible" : "not visible")}", () => downloadButton.Download.IsPresent == visible);
-        private void assertDownloadEnabled(bool enabled) => AddAssert($"download {(enabled ? "enabled" : "disabled")}", () => downloadButton.Download.Enabled.Value == enabled);
-
-        private void assertPlayVisible(bool visible) => AddUntilStep($"play {(visible ? "visible" : "not visible")}", () => downloadButton.Play.IsPresent == visible);
-
-        private static APIBeatmapSet createSoleily() => new APIBeatmapSet
-        {
-            OnlineID = 241526,
-            Availability = new BeatmapSetOnlineAvailability
-            {
-                DownloadDisabled = false,
-                ExternalLink = string.Empty,
-            },
-        };
-
-        private void createButtonWithBeatmap(APIBeatmapSet beatmap)
-        {
-            AddStep("create button", () =>
-            {
-                Child = downloadButton = new TestDownloadButton(beatmap)
-                {
-                    Anchor = Anchor.Centre,
-                    Origin = Anchor.Centre,
-                    Scale = new Vector2(2)
-                };
-            });
-        }
+        private void assertDownloadEnabled(bool enabled) => AddAssert($"download {(enabled ? "enabled" : "disabled")}", () => downloadButton.Enabled.Value == enabled);
 
         private void createButton(bool downloadable, bool hasVideo = false)
         {
             AddStep("create button", () =>
             {
-                Child = downloadButton = new TestDownloadButton(downloadable ? getDownloadableBeatmapSet(hasVideo) : getUndownloadableBeatmapSet())
+                var beatmapSet = downloadable ? getDownloadableBeatmapSet(hasVideo) : getUndownloadableBeatmapSet();
+                var downloadTracker = new BeatmapDownloadTracker(beatmapSet);
+
+                Child = new DependencyProvidingContainer
                 {
-                    Anchor = Anchor.Centre,
-                    Origin = Anchor.Centre,
-                    Scale = new Vector2(2)
+                    RelativeSizeAxes = Axes.Both,
+                    CachedDependencies = new (Type, object)[]
+                    {
+                        (typeof(BeatmapDownloadTracker), downloadTracker)
+                    },
+                    Child = downloadButton = new DownloadButton(downloadable ? getDownloadableBeatmapSet(hasVideo) : getUndownloadableBeatmapSet())
+                    {
+                        Anchor = Anchor.Centre,
+                        Origin = Anchor.Centre,
+                        Scale = new Vector2(2)
+                    }
                 };
             });
         }
@@ -175,18 +101,5 @@ namespace osu.Game.Tests.Visual.Beatmaps
 
             return beatmap;
         }
-
-        private class TestDownloadButton : DownloadButton
-        {
-            public new Bindable<DownloadState> State => base.State;
-
-            public new BeatmapCardIconButton Download => base.Download;
-            public new BeatmapCardIconButton Play => base.Play;
-
-            public TestDownloadButton(APIBeatmapSet beatmapSet)
-                : base(beatmapSet)
-            {
-            }
-        }
     }
 }

osu.Game/Beatmaps/Drawables/Cards/BeatmapCard.cs

@@ -25,6 +25,7 @@ using osu.Game.Overlays.BeatmapListing.Panels;
 using osu.Game.Resources.Localisation.Web;
 using osuTK.Graphics;
 using DownloadButton = osu.Game.Beatmaps.Drawables.Cards.Buttons.DownloadButton;
+using PlayButton = osu.Game.Beatmaps.Drawables.Cards.Buttons.PlayButton;
 
 namespace osu.Game.Beatmaps.Drawables.Cards
 {
@@ -47,7 +48,7 @@ namespace osu.Game.Beatmaps.Drawables.Cards
         private FillFlowContainer leftIconArea;
 
         private Container rightAreaBackground;
-        private Container rightAreaButtons;
+        private Container<BeatmapCardIconButton> rightAreaButtons;
 
         private Container mainContent;
         private BeatmapCardContentBackground mainContentBackground;
@@ -119,24 +120,35 @@ namespace osu.Game.Beatmaps.Drawables.Cards
                         }
                     }
                 },
-                rightAreaButtons = new Container
+                new Container
                 {
                     Name = @"Right (button) area",
                     Width = 30,
                     RelativeSizeAxes = Axes.Y,
                     Origin = Anchor.TopRight,
                     Anchor = Anchor.TopRight,
-                    Child = new FillFlowContainer
+                    Padding = new MarginPadding { Vertical = 17.5f },
+                    Child = rightAreaButtons = new Container<BeatmapCardIconButton>
                     {
-                        AutoSizeAxes = Axes.Both,
-                        Anchor = Anchor.Centre,
-                        Origin = Anchor.Centre,
-                        Direction = FillDirection.Vertical,
-                        Spacing = new Vector2(0, 14),
-                        Children = new Drawable[]
+                        RelativeSizeAxes = Axes.Both,
+                        Children = new BeatmapCardIconButton[]
                         {
-                            new FavouriteButton(beatmapSet) { Current = favouriteState },
+                            new FavouriteButton(beatmapSet)
+                            {
+                                Current = favouriteState,
+                                Anchor = Anchor.TopCentre,
+                                Origin = Anchor.TopCentre
+                            },
                             new DownloadButton(beatmapSet)
+                            {
+                                Anchor = Anchor.BottomCentre,
+                                Origin = Anchor.BottomCentre
+                            },
+                            new PlayButton(beatmapSet)
+                            {
+                                Anchor = Anchor.BottomCentre,
+                                Origin = Anchor.BottomCentre
+                            }
                         }
                     }
                 },
@@ -389,6 +401,13 @@ namespace osu.Game.Beatmaps.Drawables.Cards
 
             rightAreaBackground.FadeColour(downloadTracker.State.Value == DownloadState.LocallyAvailable ? colours.Lime0 : colourProvider.Background3, TRANSITION_DURATION, Easing.OutQuint);
             rightAreaButtons.FadeTo(IsHovered ? 1 : 0, TRANSITION_DURATION, Easing.OutQuint);
+
+            foreach (var button in rightAreaButtons)
+            {
+                button.IdleColour = downloadTracker.State.Value != DownloadState.LocallyAvailable ? colourProvider.Light1 : colourProvider.Background3;
+                button.HoverColour = downloadTracker.State.Value != DownloadState.LocallyAvailable ? colourProvider.Content1 : colourProvider.Foreground1;
+            }
+
             bool showProgress = downloadTracker.State.Value == DownloadState.Downloading || downloadTracker.State.Value == DownloadState.Importing;
 
             idleBottomContent.FadeTo(showProgress ? 0 : 1, TRANSITION_DURATION, Easing.OutQuint);

osu.Game/Beatmaps/Drawables/Cards/Buttons/BeatmapCardIconButton.cs

@@ -1,25 +1,44 @@
 // Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
 // See the LICENCE file in the repository root for full licence text.
 
-using System.Collections.Generic;
 using osu.Framework.Allocation;
-using osu.Framework.Extensions.IEnumerableExtensions;
 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.Game.Graphics.Containers;
 using osu.Game.Overlays;
 using osuTK;
 
 namespace osu.Game.Beatmaps.Drawables.Cards.Buttons
 {
-    public abstract class BeatmapCardIconButton : OsuHoverContainer
+    public abstract class BeatmapCardIconButton : OsuClickableContainer
     {
-        protected override IEnumerable<Drawable> EffectTargets => background.Yield();
+        private Colour4 idleColour;
 
-        private readonly Box background;
-        protected readonly SpriteIcon Icon;
+        public Colour4 IdleColour
+        {
+            get => idleColour;
+            set
+            {
+                idleColour = value;
+                if (IsLoaded)
+                    updateState();
+            }
+        }
+
+        private Colour4 hoverColour;
+
+        public Colour4 HoverColour
+        {
+            get => hoverColour;
+            set
+            {
+                hoverColour = value;
+                if (IsLoaded)
+                    updateState();
+            }
+        }
 
         private float iconSize;
 
@@ -33,18 +52,18 @@ namespace osu.Game.Beatmaps.Drawables.Cards.Buttons
             }
         }
 
+        protected readonly SpriteIcon Icon;
+
         protected BeatmapCardIconButton()
         {
+            Anchor = Origin = Anchor.Centre;
+
             Child = new CircularContainer
             {
                 RelativeSizeAxes = Axes.Both,
                 Masking = true,
                 Children = new Drawable[]
                 {
-                    background = new Box
-                    {
-                        RelativeSizeAxes = Axes.Both
-                    },
                     Icon = new SpriteIcon
                     {
                         Origin = Anchor.Centre,
@@ -60,11 +79,33 @@ namespace osu.Game.Beatmaps.Drawables.Cards.Buttons
         [BackgroundDependencyLoader]
         private void load(OverlayColourProvider colourProvider)
         {
-            Anchor = Origin = Anchor.Centre;
+            IdleColour = colourProvider.Light1;
+            HoverColour = colourProvider.Content1;
+        }
 
-            IdleColour = colourProvider.Background4;
-            HoverColour = colourProvider.Background1;
-            Icon.Colour = colourProvider.Content2;
+        protected override void LoadComplete()
+        {
+            base.LoadComplete();
+
+            Enabled.BindValueChanged(_ => updateState(), true);
+            FinishTransforms(true);
+        }
+
+        protected override bool OnHover(HoverEvent e)
+        {
+            updateState();
+            return base.OnHover(e);
+        }
+
+        protected override void OnHoverLost(HoverLostEvent e)
+        {
+            base.OnHoverLost(e);
+            updateState();
+        }
+
+        private void updateState()
+        {
+            Content.FadeColour(IsHovered && Enabled.Value ? HoverColour : IdleColour, BeatmapCard.TRANSITION_DURATION, Easing.OutQuint);
         }
     }
 }

osu.Game/Beatmaps/Drawables/Cards/Buttons/DownloadButton.cs

@@ -6,128 +6,62 @@
 using osu.Framework.Allocation;
 using osu.Framework.Bindables;
 using osu.Framework.Graphics;
-using osu.Framework.Graphics.Containers;
 using osu.Framework.Graphics.Sprites;
 using osu.Game.Online.API.Requests.Responses;
 using osu.Game.Configuration;
-using osu.Game.Graphics;
 using osu.Game.Online;
-using osu.Game.Overlays;
 using osu.Game.Resources.Localisation.Web;
 
 namespace osu.Game.Beatmaps.Drawables.Cards.Buttons
 {
-    public class DownloadButton : CompositeDrawable
+    public class DownloadButton : BeatmapCardIconButton
     {
-        protected readonly DownloadIcon Download;
-        protected readonly PlayIcon Play;
-        protected readonly Bindable<DownloadState> State = new Bindable<DownloadState>();
+        private readonly APIBeatmapSet beatmapSet;
+        private Bindable<bool> preferNoVideo = null!;
+        private Bindable<DownloadState> downloadState = new Bindable<DownloadState>();
 
         [Resolved]
-        private OsuColour colours { get; set; } = null!;
-
-        [Resolved]
-        private OverlayColourProvider colourProvider { get; set; } = null!;
+        private BeatmapManager beatmaps { get; set; } = null!;
 
         public DownloadButton(APIBeatmapSet beatmapSet)
         {
-            Anchor = Anchor.Centre;
-            Origin = Anchor.Centre;
-            AutoSizeAxes = Axes.Both;
+            Icon.Icon = FontAwesome.Solid.Download;
 
-            InternalChildren = new Drawable[]
-            {
-                Download = new DownloadIcon(beatmapSet),
-                Play = new PlayIcon(beatmapSet)
-            };
+            this.beatmapSet = beatmapSet;
         }
 
-        [BackgroundDependencyLoader(true)]
-        private void load(BeatmapDownloadTracker? tracker)
+        [BackgroundDependencyLoader]
+        private void load(OsuConfigManager config, BeatmapDownloadTracker downloadTracker)
         {
-            if (tracker != null)
-                ((IBindable<DownloadState>)State).BindTo(tracker.State);
+            preferNoVideo = config.GetBindable<bool>(OsuSetting.PreferNoVideo);
+            ((IBindable<DownloadState>)downloadState).BindTo(downloadTracker.State);
         }
 
         protected override void LoadComplete()
         {
             base.LoadComplete();
-
-            State.BindValueChanged(_ => updateState(), true);
+            preferNoVideo.BindValueChanged(_ => updateState());
+            downloadState.BindValueChanged(_ => updateState(), true);
             FinishTransforms(true);
         }
 
         private void updateState()
         {
-            Download.FadeTo(State.Value != DownloadState.LocallyAvailable ? 1 : 0, BeatmapCard.TRANSITION_DURATION, Easing.OutQuint);
-            Download.Enabled.Value = State.Value == DownloadState.NotDownloaded;
+            this.FadeTo(downloadState.Value != DownloadState.LocallyAvailable ? 1 : 0, BeatmapCard.TRANSITION_DURATION, Easing.OutQuint);
 
-            Play.FadeTo(State.Value == DownloadState.LocallyAvailable ? 1 : 0, BeatmapCard.TRANSITION_DURATION, Easing.OutQuint);
-        }
-
-        protected class DownloadIcon : BeatmapCardIconButton
-        {
-            private readonly APIBeatmapSet beatmapSet;
-            private Bindable<bool> preferNoVideo = null!;
-
-            [Resolved]
-            private BeatmapManager beatmaps { get; set; } = null!;
-
-            public DownloadIcon(APIBeatmapSet beatmapSet)
+            if (beatmapSet.Availability.DownloadDisabled)
             {
-                Icon.Icon = FontAwesome.Solid.Download;
-
-                this.beatmapSet = beatmapSet;
+                Enabled.Value = false;
+                TooltipText = BeatmapsetsStrings.AvailabilityDisabled;
+                return;
             }
 
-            [BackgroundDependencyLoader]
-            private void load(OsuConfigManager config)
-            {
-                preferNoVideo = config.GetBindable<bool>(OsuSetting.PreferNoVideo);
-            }
+            if (!beatmapSet.HasVideo)
+                TooltipText = BeatmapsetsStrings.PanelDownloadAll;
+            else
+                TooltipText = preferNoVideo.Value ? BeatmapsetsStrings.PanelDownloadNoVideo : BeatmapsetsStrings.PanelDownloadVideo;
 
-            protected override void LoadComplete()
-            {
-                base.LoadComplete();
-                preferNoVideo.BindValueChanged(_ => updateState(), true);
-            }
-
-            private void updateState()
-            {
-                if (beatmapSet.Availability.DownloadDisabled)
-                {
-                    Enabled.Value = false;
-                    TooltipText = BeatmapsetsStrings.AvailabilityDisabled;
-                    return;
-                }
-
-                if (!beatmapSet.HasVideo)
-                    TooltipText = BeatmapsetsStrings.PanelDownloadAll;
-                else
-                    TooltipText = preferNoVideo.Value ? BeatmapsetsStrings.PanelDownloadNoVideo : BeatmapsetsStrings.PanelDownloadVideo;
-
-                Action = () => beatmaps.Download(beatmapSet, preferNoVideo.Value);
-            }
-        }
-
-        protected class PlayIcon : BeatmapCardIconButton
-        {
-            private readonly APIBeatmapSet beatmapSet;
-
-            public PlayIcon(APIBeatmapSet beatmapSet)
-            {
-                this.beatmapSet = beatmapSet;
-
-                Icon.Icon = FontAwesome.Solid.AngleDoubleRight;
-                TooltipText = "Go to beatmap";
-            }
-
-            [BackgroundDependencyLoader(true)]
-            private void load(OsuGame? game)
-            {
-                if (game != null)
-                    Action = () => game.PresentBeatmap(beatmapSet);
-            }
+            Action = () => beatmaps.Download(beatmapSet, preferNoVideo.Value);
         }
     }
 }

osu.Game/Beatmaps/Drawables/Cards/Buttons/PlayButton.cs

@@ -0,0 +1,50 @@
+// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
+// See the LICENCE file in the repository root for full licence text.
+
+#nullable enable
+
+using osu.Framework.Allocation;
+using osu.Framework.Bindables;
+using osu.Framework.Graphics;
+using osu.Framework.Graphics.Sprites;
+using osu.Game.Online;
+using osu.Game.Online.API.Requests.Responses;
+
+namespace osu.Game.Beatmaps.Drawables.Cards.Buttons
+{
+    public class PlayButton : BeatmapCardIconButton
+    {
+        private readonly APIBeatmapSet beatmapSet;
+        private readonly Bindable<DownloadState> downloadState = new Bindable<DownloadState>();
+
+        public PlayButton(APIBeatmapSet beatmapSet)
+        {
+            this.beatmapSet = beatmapSet;
+
+            Icon.Icon = FontAwesome.Solid.AngleDoubleRight;
+            TooltipText = "Go to beatmap";
+        }
+
+        [BackgroundDependencyLoader(true)]
+        private void load(OsuGame? game, BeatmapDownloadTracker downloadTracker)
+        {
+            if (game != null)
+                Action = () => game.PresentBeatmap(beatmapSet);
+
+            ((IBindable<DownloadState>)downloadState).BindTo(downloadTracker.State);
+        }
+
+        protected override void LoadComplete()
+        {
+            base.LoadComplete();
+
+            downloadState.BindValueChanged(_ => updateState(), true);
+            FinishTransforms(true);
+        }
+
+        private void updateState()
+        {
+            this.FadeTo(downloadState.Value == DownloadState.LocallyAvailable ? 1 : 0, BeatmapCard.TRANSITION_DURATION, Easing.OutQuint);
+        }
+    }
+}