From e191c2c50e5652d32f04b7fccb7a7a57ed5c2e9d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 14 Oct 2019 21:32:16 +0900 Subject: [PATCH 1/7] Tidy up constants and method naming --- osu.Game/Overlays/Comments/CommentsHeader.cs | 16 +++++++--------- osu.Game/Overlays/Comments/HeaderButton.cs | 19 ++++++++----------- osu.Game/Overlays/Comments/SortSelector.cs | 4 ++-- 3 files changed, 17 insertions(+), 22 deletions(-) diff --git a/osu.Game/Overlays/Comments/CommentsHeader.cs b/osu.Game/Overlays/Comments/CommentsHeader.cs index 2bd2bf21a6..1df347eb82 100644 --- a/osu.Game/Overlays/Comments/CommentsHeader.cs +++ b/osu.Game/Overlays/Comments/CommentsHeader.cs @@ -15,10 +15,7 @@ namespace osu.Game.Overlays.Comments { public class CommentsHeader : CompositeDrawable { - private const int height = 40; - private const int spacing = 10; - private const int padding = 50; - private const int text_size = 14; + private const int font_size = 14; public readonly Bindable Sort = new Bindable(); public readonly BindableBool ShowDeleted = new BindableBool(); @@ -28,7 +25,8 @@ namespace osu.Game.Overlays.Comments public CommentsHeader() { RelativeSizeAxes = Axes.X; - Height = height; + Height = 40; + AddRangeInternal(new Drawable[] { background = new Box @@ -38,14 +36,14 @@ namespace osu.Game.Overlays.Comments new Container { RelativeSizeAxes = Axes.Both, - Padding = new MarginPadding { Horizontal = padding }, + Padding = new MarginPadding { Horizontal = 50 }, Children = new Drawable[] { new FillFlowContainer { AutoSizeAxes = Axes.Both, Direction = FillDirection.Horizontal, - Spacing = new Vector2(spacing, 0), + Spacing = new Vector2(10, 0), Anchor = Anchor.CentreLeft, Origin = Anchor.CentreLeft, Children = new Drawable[] @@ -54,7 +52,7 @@ namespace osu.Game.Overlays.Comments { Anchor = Anchor.CentreLeft, Origin = Anchor.CentreLeft, - Font = OsuFont.GetFont(size: text_size), + Font = OsuFont.GetFont(size: font_size), Text = @"Sort by" }, new SortSelector @@ -107,7 +105,7 @@ namespace osu.Game.Overlays.Comments { Anchor = Anchor.CentreLeft, Origin = Anchor.CentreLeft, - Font = OsuFont.GetFont(size: text_size), + Font = OsuFont.GetFont(size: font_size), Text = @"Show deleted" } }, diff --git a/osu.Game/Overlays/Comments/HeaderButton.cs b/osu.Game/Overlays/Comments/HeaderButton.cs index 231a364759..8789cf5830 100644 --- a/osu.Game/Overlays/Comments/HeaderButton.cs +++ b/osu.Game/Overlays/Comments/HeaderButton.cs @@ -13,10 +13,7 @@ namespace osu.Game.Overlays.Comments { public class HeaderButton : Container { - private const int height = 20; - private const int corner_radius = 3; - private const int margin = 10; - private const int duration = 200; + private const int transition_duration = 200; protected override Container Content => content; @@ -26,9 +23,9 @@ namespace osu.Game.Overlays.Comments public HeaderButton() { AutoSizeAxes = Axes.X; - Height = height; + Height = 20; Masking = true; - CornerRadius = corner_radius; + CornerRadius = 3; AddRangeInternal(new Drawable[] { background = new Box @@ -41,7 +38,7 @@ namespace osu.Game.Overlays.Comments AutoSizeAxes = Axes.Both, Anchor = Anchor.Centre, Origin = Anchor.Centre, - Margin = new MarginPadding { Horizontal = margin } + Margin = new MarginPadding { Horizontal = 10 } }, new HoverClickSounds(), }); @@ -55,18 +52,18 @@ namespace osu.Game.Overlays.Comments protected override bool OnHover(HoverEvent e) { - FadeInBackground(); + ShowBackground(); return base.OnHover(e); } protected override void OnHoverLost(HoverLostEvent e) { base.OnHoverLost(e); - FadeOutBackground(); + HideBackground(); } - protected void FadeInBackground() => background.FadeIn(duration, Easing.OutQuint); + protected void ShowBackground() => background.FadeIn(transition_duration, Easing.OutQuint); - protected void FadeOutBackground() => background.FadeOut(duration, Easing.OutQuint); + protected void HideBackground() => background.FadeOut(transition_duration, Easing.OutQuint); } } diff --git a/osu.Game/Overlays/Comments/SortSelector.cs b/osu.Game/Overlays/Comments/SortSelector.cs index 100ae83291..90e7defb9a 100644 --- a/osu.Game/Overlays/Comments/SortSelector.cs +++ b/osu.Game/Overlays/Comments/SortSelector.cs @@ -75,7 +75,7 @@ namespace osu.Game.Overlays.Comments public void Activate() { - FadeInBackground(); + ShowBackground(); text.Font = text.Font.With(weight: FontWeight.Bold); text.Colour = colours.BlueLighter; } @@ -83,7 +83,7 @@ namespace osu.Game.Overlays.Comments public void Deactivate() { if (!IsHovered) - FadeOutBackground(); + HideBackground(); text.Font = text.Font.With(weight: FontWeight.Medium); text.Colour = Color4.White; From 89f270a19a32df0696cbd244eb9e39d826220f4b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 14 Oct 2019 21:32:41 +0900 Subject: [PATCH 2/7] SortSelector -> SortTabControl --- osu.Game.Tests/Visual/Online/TestSceneCommentsHeader.cs | 2 +- osu.Game/Overlays/Comments/CommentsHeader.cs | 2 +- .../Overlays/Comments/{SortSelector.cs => SortTabControl.cs} | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) rename osu.Game/Overlays/Comments/{SortSelector.cs => SortTabControl.cs} (96%) diff --git a/osu.Game.Tests/Visual/Online/TestSceneCommentsHeader.cs b/osu.Game.Tests/Visual/Online/TestSceneCommentsHeader.cs index 949dbbe5c4..bc3e0eff1a 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneCommentsHeader.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneCommentsHeader.cs @@ -16,7 +16,7 @@ namespace osu.Game.Tests.Visual.Online { typeof(CommentsHeader), typeof(HeaderButton), - typeof(SortSelector), + typeof(SortTabControl), }; private readonly Bindable sort = new Bindable(); diff --git a/osu.Game/Overlays/Comments/CommentsHeader.cs b/osu.Game/Overlays/Comments/CommentsHeader.cs index 1df347eb82..81be16967f 100644 --- a/osu.Game/Overlays/Comments/CommentsHeader.cs +++ b/osu.Game/Overlays/Comments/CommentsHeader.cs @@ -55,7 +55,7 @@ namespace osu.Game.Overlays.Comments Font = OsuFont.GetFont(size: font_size), Text = @"Sort by" }, - new SortSelector + new SortTabControl { Anchor = Anchor.CentreLeft, Origin = Anchor.CentreLeft, diff --git a/osu.Game/Overlays/Comments/SortSelector.cs b/osu.Game/Overlays/Comments/SortTabControl.cs similarity index 96% rename from osu.Game/Overlays/Comments/SortSelector.cs rename to osu.Game/Overlays/Comments/SortTabControl.cs index 90e7defb9a..8dc1f14c3d 100644 --- a/osu.Game/Overlays/Comments/SortSelector.cs +++ b/osu.Game/Overlays/Comments/SortTabControl.cs @@ -15,7 +15,7 @@ using osuTK.Graphics; namespace osu.Game.Overlays.Comments { - public class SortSelector : OsuTabControl + public class SortTabControl : OsuTabControl { private const int spacing = 5; @@ -30,7 +30,7 @@ namespace osu.Game.Overlays.Comments Spacing = new Vector2(spacing, 0), }; - public SortSelector() + public SortTabControl() { AutoSizeAxes = Axes.Both; } From 4822496c130880a5f3c20e847942cbab870849a9 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 14 Oct 2019 21:34:16 +0900 Subject: [PATCH 3/7] Fix more naming --- osu.Game/Overlays/Comments/SortTabControl.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/osu.Game/Overlays/Comments/SortTabControl.cs b/osu.Game/Overlays/Comments/SortTabControl.cs index 8dc1f14c3d..3f1b5c54bd 100644 --- a/osu.Game/Overlays/Comments/SortTabControl.cs +++ b/osu.Game/Overlays/Comments/SortTabControl.cs @@ -37,23 +37,23 @@ namespace osu.Game.Overlays.Comments private class SortTabItem : TabItem { - private readonly TabContent content; + private readonly TabButton button; public SortTabItem(CommentsSortCriteria value) : base(value) { AutoSizeAxes = Axes.Both; - Child = content = new TabContent(value) + Child = button = new TabButton(value) { Active = { BindTarget = Active } }; } - protected override void OnActivated() => content.Activate(); + protected override void OnActivated() => button.Activate(); - protected override void OnDeactivated() => content.Deactivate(); + protected override void OnDeactivated() => button.Deactivate(); - private class TabContent : HeaderButton + private class TabButton : HeaderButton { private const int text_size = 14; @@ -64,7 +64,7 @@ namespace osu.Game.Overlays.Comments private readonly SpriteText text; - public TabContent(CommentsSortCriteria value) + public TabButton(CommentsSortCriteria value) { Add(text = new SpriteText { From 779445755034234c0c986dd8f874b0b2162ea35e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 14 Oct 2019 21:34:43 +0900 Subject: [PATCH 4/7] Remove more constants --- osu.Game/Overlays/Comments/SortTabControl.cs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/osu.Game/Overlays/Comments/SortTabControl.cs b/osu.Game/Overlays/Comments/SortTabControl.cs index 3f1b5c54bd..596822a86c 100644 --- a/osu.Game/Overlays/Comments/SortTabControl.cs +++ b/osu.Game/Overlays/Comments/SortTabControl.cs @@ -17,8 +17,6 @@ namespace osu.Game.Overlays.Comments { public class SortTabControl : OsuTabControl { - private const int spacing = 5; - protected override Dropdown CreateDropdown() => null; protected override TabItem CreateTabItem(CommentsSortCriteria value) => new SortTabItem(value); @@ -27,7 +25,7 @@ namespace osu.Game.Overlays.Comments { AutoSizeAxes = Axes.Both, Direction = FillDirection.Horizontal, - Spacing = new Vector2(spacing, 0), + Spacing = new Vector2(5, 0), }; public SortTabControl() @@ -55,8 +53,6 @@ namespace osu.Game.Overlays.Comments private class TabButton : HeaderButton { - private const int text_size = 14; - public readonly BindableBool Active = new BindableBool(); [Resolved] @@ -68,7 +64,7 @@ namespace osu.Game.Overlays.Comments { Add(text = new SpriteText { - Font = OsuFont.GetFont(size: text_size), + Font = OsuFont.GetFont(size: 14), Text = value.ToString() }); } From 4e6ab1dad397f0151861f3fc39bed5c3c9f0617f Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 14 Oct 2019 21:42:07 +0900 Subject: [PATCH 5/7] Tidy up state management via bindable usage --- osu.Game/Overlays/Comments/SortTabControl.cs | 51 +++++++++++--------- 1 file changed, 29 insertions(+), 22 deletions(-) diff --git a/osu.Game/Overlays/Comments/SortTabControl.cs b/osu.Game/Overlays/Comments/SortTabControl.cs index 596822a86c..f5423e692f 100644 --- a/osu.Game/Overlays/Comments/SortTabControl.cs +++ b/osu.Game/Overlays/Comments/SortTabControl.cs @@ -35,21 +35,20 @@ namespace osu.Game.Overlays.Comments private class SortTabItem : TabItem { - private readonly TabButton button; - public SortTabItem(CommentsSortCriteria value) : base(value) { AutoSizeAxes = Axes.Both; - Child = button = new TabButton(value) - { - Active = { BindTarget = Active } - }; + Child = new TabButton(value) { Active = { BindTarget = Active } }; } - protected override void OnActivated() => button.Activate(); + protected override void OnActivated() + { + } - protected override void OnDeactivated() => button.Deactivate(); + protected override void OnDeactivated() + { + } private class TabButton : HeaderButton { @@ -69,25 +68,33 @@ namespace osu.Game.Overlays.Comments }); } - public void Activate() + protected override void LoadComplete() { - ShowBackground(); - text.Font = text.Font.With(weight: FontWeight.Bold); - text.Colour = colours.BlueLighter; + base.LoadComplete(); + + Active.BindValueChanged(active => + { + updateBackgroundState(); + + text.Font = text.Font.With(weight: active.NewValue ? FontWeight.Bold : FontWeight.Medium); + text.Colour = active.NewValue ? colours.BlueLighter : Color4.White; + }, true); } - public void Deactivate() + protected override bool OnHover(HoverEvent e) { - if (!IsHovered) + updateBackgroundState(); + return true; + } + + protected override void OnHoverLost(HoverLostEvent e) => updateBackgroundState(); + + private void updateBackgroundState() + { + if (Active.Value || IsHovered) + ShowBackground(); + else HideBackground(); - - text.Font = text.Font.With(weight: FontWeight.Medium); - text.Colour = Color4.White; - } - - protected override void OnHoverLost(HoverLostEvent e) - { - if (!Active.Value) base.OnHoverLost(e); } } } From b7ddf160b46d874e20c928aae44595daccfe692a Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 14 Oct 2019 21:48:12 +0900 Subject: [PATCH 6/7] OnClick should actually handle the event --- osu.Game/Overlays/Comments/CommentsHeader.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Overlays/Comments/CommentsHeader.cs b/osu.Game/Overlays/Comments/CommentsHeader.cs index 81be16967f..1797a36b71 100644 --- a/osu.Game/Overlays/Comments/CommentsHeader.cs +++ b/osu.Game/Overlays/Comments/CommentsHeader.cs @@ -126,7 +126,7 @@ namespace osu.Game.Overlays.Comments protected override bool OnClick(ClickEvent e) { Checked.Value = !Checked.Value; - return base.OnClick(e); + return true; } } } From f0e970034950ff034e380be1ecf34947abf806fc Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 14 Oct 2019 21:49:02 +0900 Subject: [PATCH 7/7] Inline delegate event --- osu.Game/Overlays/Comments/CommentsHeader.cs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/osu.Game/Overlays/Comments/CommentsHeader.cs b/osu.Game/Overlays/Comments/CommentsHeader.cs index 1797a36b71..66fe7ff3fa 100644 --- a/osu.Game/Overlays/Comments/CommentsHeader.cs +++ b/osu.Game/Overlays/Comments/CommentsHeader.cs @@ -114,15 +114,10 @@ namespace osu.Game.Overlays.Comments protected override void LoadComplete() { - Checked.BindValueChanged(onCheckedChanged, true); + Checked.BindValueChanged(isChecked => checkboxIcon.Icon = isChecked.NewValue ? FontAwesome.Solid.CheckSquare : FontAwesome.Regular.Square, true); base.LoadComplete(); } - private void onCheckedChanged(ValueChangedEvent isChecked) - { - checkboxIcon.Icon = isChecked.NewValue ? FontAwesome.Solid.CheckSquare : FontAwesome.Regular.Square; - } - protected override bool OnClick(ClickEvent e) { Checked.Value = !Checked.Value;