From 50a478ea5f3a5538eca5d9254644f9286680809c Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 14 Apr 2022 20:05:13 +0900 Subject: [PATCH 01/12] Add failing test coverage of searching too fast not hiding filtered settings --- .../Visual/Settings/TestSceneSettingsPanel.cs | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/osu.Game.Tests/Visual/Settings/TestSceneSettingsPanel.cs b/osu.Game.Tests/Visual/Settings/TestSceneSettingsPanel.cs index f9c9b2a68b..0de1cedd0c 100644 --- a/osu.Game.Tests/Visual/Settings/TestSceneSettingsPanel.cs +++ b/osu.Game.Tests/Visual/Settings/TestSceneSettingsPanel.cs @@ -4,10 +4,12 @@ using System.Linq; using NUnit.Framework; using osu.Framework.Allocation; +using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Testing; using osu.Game.Graphics.UserInterface; using osu.Game.Overlays; +using osu.Game.Overlays.Settings; using osu.Game.Overlays.Settings.Sections; using osu.Game.Overlays.Settings.Sections.Input; using osuTK.Input; @@ -34,6 +36,26 @@ namespace osu.Game.Tests.Visual.Settings }); } + [Test] + public void TestQuickFiltering() + { + AddStep("set filter", () => + { + settings.SectionsContainer.ChildrenOfType().First().Current.Value = "scaling"; + }); + + AddUntilStep("wait for items to load", () => settings.SectionsContainer.ChildrenOfType().Any()); + + AddAssert("ensure all items match filter", () => settings.SectionsContainer + .ChildrenOfType().Where(f => f.IsPresent) + .All(section => + section.ChildrenOfType().Where(f => f.IsPresent) + .OfType() + .Where(f => !(f is IHasFilterableChildren)) + .All(f => f.FilterTerms.Any(t => t.Contains("scaling"))) + )); + } + [Test] public void ToggleVisibility() { From 296420ae08b41305e179dfcd0cf0f935971e8ed1 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 14 Apr 2022 20:06:00 +0900 Subject: [PATCH 02/12] Fix settings textbox focus operation running too late --- osu.Game/Overlays/SettingsPanel.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Overlays/SettingsPanel.cs b/osu.Game/Overlays/SettingsPanel.cs index b11b6fde27..ea386e26c6 100644 --- a/osu.Game/Overlays/SettingsPanel.cs +++ b/osu.Game/Overlays/SettingsPanel.cs @@ -163,6 +163,7 @@ namespace osu.Game.Overlays Sidebar?.MoveToX(0, TRANSITION_LENGTH, Easing.OutQuint); this.FadeTo(1, TRANSITION_LENGTH, Easing.OutQuint); + searchTextBox.TakeFocus(); searchTextBox.HoldFocus = true; } @@ -213,7 +214,6 @@ namespace osu.Game.Overlays loading.Hide(); searchTextBox.Current.BindValueChanged(term => SectionsContainer.SearchTerm = term.NewValue, true); - searchTextBox.TakeFocus(); loadSidebarButtons(); }); From 9c7af9898213d92e214e6e8683a52c8265292f29 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 14 Apr 2022 20:06:15 +0900 Subject: [PATCH 03/12] Fix alpha cross-talk causing settings to potentially not filter when they should --- osu.Game/Overlays/Settings/SettingsItem.cs | 7 +++---- osu.Game/Overlays/Settings/SettingsSection.cs | 7 +++---- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/osu.Game/Overlays/Settings/SettingsItem.cs b/osu.Game/Overlays/Settings/SettingsItem.cs index 098090bf78..2d139a01e9 100644 --- a/osu.Game/Overlays/Settings/SettingsItem.cs +++ b/osu.Game/Overlays/Settings/SettingsItem.cs @@ -100,10 +100,9 @@ namespace osu.Game.Overlays.Settings public IEnumerable Keywords { get; set; } - public bool MatchingFilter - { - set => Alpha = value ? 1 : 0; - } + public override bool IsPresent => MatchingFilter; + + public bool MatchingFilter { get; set; } public bool FilteringActive { get; set; } diff --git a/osu.Game/Overlays/Settings/SettingsSection.cs b/osu.Game/Overlays/Settings/SettingsSection.cs index 2539c32806..a39aacf6b8 100644 --- a/osu.Game/Overlays/Settings/SettingsSection.cs +++ b/osu.Game/Overlays/Settings/SettingsSection.cs @@ -21,6 +21,8 @@ namespace osu.Game.Overlays.Settings protected FillFlowContainer FlowContent; protected override Container Content => FlowContent; + public override bool IsPresent => MatchingFilter; + private IBindable selectedSection; private OsuSpriteText header; @@ -36,10 +38,7 @@ namespace osu.Game.Overlays.Settings private const int header_size = 24; private const int border_size = 4; - public bool MatchingFilter - { - set => this.FadeTo(value ? 1 : 0); - } + public bool MatchingFilter { get; set; } public bool FilteringActive { get; set; } From 21a5a2fd697322acdbc535797b9b6727a7023e97 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 14 Apr 2022 20:36:48 +0900 Subject: [PATCH 04/12] Add failing test coverage of incorrect current section after quick setting search --- osu.Game.Tests/Visual/Settings/TestSceneSettingsPanel.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/osu.Game.Tests/Visual/Settings/TestSceneSettingsPanel.cs b/osu.Game.Tests/Visual/Settings/TestSceneSettingsPanel.cs index 0de1cedd0c..66242b19cd 100644 --- a/osu.Game.Tests/Visual/Settings/TestSceneSettingsPanel.cs +++ b/osu.Game.Tests/Visual/Settings/TestSceneSettingsPanel.cs @@ -54,6 +54,8 @@ namespace osu.Game.Tests.Visual.Settings .Where(f => !(f is IHasFilterableChildren)) .All(f => f.FilterTerms.Any(t => t.Contains("scaling"))) )); + + AddAssert("ensure section is current", () => settings.CurrentSection.Value != null); } [Test] From 316c0845ec8f56c92d36ea702038b3ec545e24e1 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 14 Apr 2022 20:36:11 +0900 Subject: [PATCH 05/12] Fix current section not being correct after early search --- osu.Game/Overlays/SettingsPanel.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/osu.Game/Overlays/SettingsPanel.cs b/osu.Game/Overlays/SettingsPanel.cs index ea386e26c6..8c1129508a 100644 --- a/osu.Game/Overlays/SettingsPanel.cs +++ b/osu.Game/Overlays/SettingsPanel.cs @@ -287,7 +287,10 @@ namespace osu.Game.Overlays set { SearchContainer.SearchTerm = value; - InvalidateScrollPosition(); + + // Schedule required as search term takes a frame to update. + // Without this sections may not be in the correct state to ascertain CurrentSection. + Schedule(InvalidateScrollPosition); } } From c57a918bbba4d3cb329b0c5a130e825d06571f7f Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 14 Apr 2022 23:02:00 +0900 Subject: [PATCH 06/12] Mark settings elements as matching filter by default to handle cases where no filtering is provided --- osu.Game/Overlays/Settings/SettingsItem.cs | 2 +- osu.Game/Overlays/Settings/SettingsSection.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Overlays/Settings/SettingsItem.cs b/osu.Game/Overlays/Settings/SettingsItem.cs index 2d139a01e9..329917d50a 100644 --- a/osu.Game/Overlays/Settings/SettingsItem.cs +++ b/osu.Game/Overlays/Settings/SettingsItem.cs @@ -102,7 +102,7 @@ namespace osu.Game.Overlays.Settings public override bool IsPresent => MatchingFilter; - public bool MatchingFilter { get; set; } + public bool MatchingFilter { get; set; } = true; public bool FilteringActive { get; set; } diff --git a/osu.Game/Overlays/Settings/SettingsSection.cs b/osu.Game/Overlays/Settings/SettingsSection.cs index a39aacf6b8..cd012f10c7 100644 --- a/osu.Game/Overlays/Settings/SettingsSection.cs +++ b/osu.Game/Overlays/Settings/SettingsSection.cs @@ -38,7 +38,7 @@ namespace osu.Game.Overlays.Settings private const int header_size = 24; private const int border_size = 4; - public bool MatchingFilter { get; set; } + public bool MatchingFilter { get; set; } = true; public bool FilteringActive { get; set; } From a0ac316d6e9eed23f86bfe334d34fab6129d834e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 15 Apr 2022 01:41:55 +0900 Subject: [PATCH 07/12] Fix test matching conditions --- osu.Game.Tests/Visual/Settings/TestSceneSettingsPanel.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/Settings/TestSceneSettingsPanel.cs b/osu.Game.Tests/Visual/Settings/TestSceneSettingsPanel.cs index 66242b19cd..71c001887a 100644 --- a/osu.Game.Tests/Visual/Settings/TestSceneSettingsPanel.cs +++ b/osu.Game.Tests/Visual/Settings/TestSceneSettingsPanel.cs @@ -49,7 +49,8 @@ namespace osu.Game.Tests.Visual.Settings AddAssert("ensure all items match filter", () => settings.SectionsContainer .ChildrenOfType().Where(f => f.IsPresent) .All(section => - section.ChildrenOfType().Where(f => f.IsPresent) + section.Children.Where(f => f.IsPresent) + .OfType() .OfType() .Where(f => !(f is IHasFilterableChildren)) .All(f => f.FilterTerms.Any(t => t.Contains("scaling"))) From 6081c29ae17fe8e202d0bc2ebc9b050a6c7a70b8 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 15 Apr 2022 10:18:29 +0900 Subject: [PATCH 08/12] Remove ununsed using statement --- osu.Game.Tests/Visual/Settings/TestSceneSettingsPanel.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/osu.Game.Tests/Visual/Settings/TestSceneSettingsPanel.cs b/osu.Game.Tests/Visual/Settings/TestSceneSettingsPanel.cs index 71c001887a..6e2ba88ea0 100644 --- a/osu.Game.Tests/Visual/Settings/TestSceneSettingsPanel.cs +++ b/osu.Game.Tests/Visual/Settings/TestSceneSettingsPanel.cs @@ -4,7 +4,6 @@ using System.Linq; using NUnit.Framework; using osu.Framework.Allocation; -using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Testing; using osu.Game.Graphics.UserInterface; From 423fae52f7269ea0ce638afb3b0128e3dc41583a Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 15 Apr 2022 10:19:36 +0900 Subject: [PATCH 09/12] Add back `base.IsPresent` call --- osu.Game/Overlays/Settings/SettingsItem.cs | 2 +- osu.Game/Overlays/Settings/SettingsSection.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Overlays/Settings/SettingsItem.cs b/osu.Game/Overlays/Settings/SettingsItem.cs index 329917d50a..1c5668479f 100644 --- a/osu.Game/Overlays/Settings/SettingsItem.cs +++ b/osu.Game/Overlays/Settings/SettingsItem.cs @@ -100,7 +100,7 @@ namespace osu.Game.Overlays.Settings public IEnumerable Keywords { get; set; } - public override bool IsPresent => MatchingFilter; + public override bool IsPresent => base.IsPresent && MatchingFilter; public bool MatchingFilter { get; set; } = true; diff --git a/osu.Game/Overlays/Settings/SettingsSection.cs b/osu.Game/Overlays/Settings/SettingsSection.cs index cd012f10c7..ace618aa46 100644 --- a/osu.Game/Overlays/Settings/SettingsSection.cs +++ b/osu.Game/Overlays/Settings/SettingsSection.cs @@ -21,7 +21,7 @@ namespace osu.Game.Overlays.Settings protected FillFlowContainer FlowContent; protected override Container Content => FlowContent; - public override bool IsPresent => MatchingFilter; + public override bool IsPresent => base.IsPresent && MatchingFilter; private IBindable selectedSection; From 749b86f0f41c7bed292d3eff81b40ceaf5b20fc4 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 22 Apr 2022 00:48:41 +0900 Subject: [PATCH 10/12] Use new framework exposed event instead of schedule --- osu.Game/Overlays/SettingsPanel.cs | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/osu.Game/Overlays/SettingsPanel.cs b/osu.Game/Overlays/SettingsPanel.cs index 7cf233a061..a5a6f9bce7 100644 --- a/osu.Game/Overlays/SettingsPanel.cs +++ b/osu.Game/Overlays/SettingsPanel.cs @@ -284,14 +284,7 @@ namespace osu.Game.Overlays public string SearchTerm { get => SearchContainer.SearchTerm; - set - { - SearchContainer.SearchTerm = value; - - // Schedule required as search term takes a frame to update. - // Without this sections may not be in the correct state to ascertain CurrentSection. - Schedule(InvalidateScrollPosition); - } + set => SearchContainer.SearchTerm = value; } protected override FlowContainer CreateScrollContentContainer() @@ -310,6 +303,8 @@ namespace osu.Game.Overlays Colour = colourProvider.Background4, RelativeSizeAxes = Axes.Both }; + + SearchContainer.FilterCompleted += InvalidateScrollPosition; } protected override void UpdateAfterChildren() From 06594ca0c8aa9635f3403db0575f322ad0c34d59 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 22 Apr 2022 15:30:12 +0900 Subject: [PATCH 11/12] Refine assertion to ensure correct section is current, rather than any section --- osu.Game.Tests/Visual/Settings/TestSceneSettingsPanel.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/Settings/TestSceneSettingsPanel.cs b/osu.Game.Tests/Visual/Settings/TestSceneSettingsPanel.cs index 8d91b55877..cdeaafd828 100644 --- a/osu.Game.Tests/Visual/Settings/TestSceneSettingsPanel.cs +++ b/osu.Game.Tests/Visual/Settings/TestSceneSettingsPanel.cs @@ -55,7 +55,7 @@ namespace osu.Game.Tests.Visual.Settings .All(f => f.FilterTerms.Any(t => t.Contains("scaling"))) )); - AddAssert("ensure section is current", () => settings.CurrentSection.Value != null); + AddAssert("ensure section is current", () => settings.CurrentSection.Value is GraphicsSection); } [Test] From f282e5b0133785d8c9e6474cbe04bf5c79dc4811 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 22 Apr 2022 15:32:23 +0900 Subject: [PATCH 12/12] Remove one more unnecessary schedule --- osu.Game/Graphics/Containers/SectionsContainer.cs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/osu.Game/Graphics/Containers/SectionsContainer.cs b/osu.Game/Graphics/Containers/SectionsContainer.cs index 540ca85809..823ba33216 100644 --- a/osu.Game/Graphics/Containers/SectionsContainer.cs +++ b/osu.Game/Graphics/Containers/SectionsContainer.cs @@ -195,11 +195,8 @@ namespace osu.Game.Graphics.Containers protected void InvalidateScrollPosition() { - Schedule(() => - { - lastKnownScroll = null; - lastClickedSection = null; - }); + lastKnownScroll = null; + lastClickedSection = null; } protected override void UpdateAfterChildren()