Merge pull request #17956 from frenzibyte/sections-container-pagination

Fix page up / down in `SectionsContainer` with fixed header skipping content

osu.Game.Tests/Visual/UserInterface/TestSceneSectionsContainer.cs

@@ -3,45 +3,79 @@
 
 using System.Linq;
 using NUnit.Framework;
+using osu.Framework.Extensions.Color4Extensions;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Colour;
+using osu.Framework.Graphics.Containers;
 using osu.Framework.Graphics.Shapes;
 using osu.Framework.Testing;
+using osu.Framework.Utils;
+using osu.Game.Graphics;
 using osu.Game.Graphics.Containers;
+using osu.Game.Graphics.Sprites;
 using osuTK.Graphics;
+using osuTK.Input;
 
 namespace osu.Game.Tests.Visual.UserInterface
 {
     public class TestSceneSectionsContainer : OsuManualInputManagerTestScene
     {
-        private readonly SectionsContainer<TestSection> container;
+        private SectionsContainer<TestSection> container;
         private float custom;
-        private const float header_height = 100;
 
-        public TestSceneSectionsContainer()
+        private const float header_expandable_height = 300;
+        private const float header_fixed_height = 100;
+
+        [SetUpSteps]
+        public void SetUpSteps()
         {
-            container = new SectionsContainer<TestSection>
+            AddStep("setup container", () =>
             {
-                RelativeSizeAxes = Axes.Y,
+                container = new SectionsContainer<TestSection>
-                Width = 300,
-                Origin = Anchor.Centre,
-                Anchor = Anchor.Centre,
-                FixedHeader = new Box
                 {
-                    Alpha = 0.5f,
+                    RelativeSizeAxes = Axes.Y,
                     Width = 300,
-                    Height = header_height,
+                    Origin = Anchor.Centre,
-                    Colour = Color4.Red
+                    Anchor = Anchor.Centre,
-                }
+                };
-            };
+
-            container.SelectedSection.ValueChanged += section =>
+                container.SelectedSection.ValueChanged += section =>
-            {
+                {
-                if (section.OldValue != null)
+                    if (section.OldValue != null)
-                    section.OldValue.Selected = false;
+                        section.OldValue.Selected = false;
-                if (section.NewValue != null)
+                    if (section.NewValue != null)
-                    section.NewValue.Selected = true;
+                        section.NewValue.Selected = true;
-            };
+                };
-            Add(container);
+
+                Child = container;
+            });
+
+            AddToggleStep("disable expandable header", v => container.ExpandableHeader = v
+                ? null
+                : new TestBox(@"Expandable Header")
+                {
+                    RelativeSizeAxes = Axes.X,
+                    Height = header_expandable_height,
+                    BackgroundColour = new OsuColour().GreySky,
+                });
+
+            AddToggleStep("disable fixed header", v => container.FixedHeader = v
+                ? null
+                : new TestBox(@"Fixed Header")
+                {
+                    RelativeSizeAxes = Axes.X,
+                    Height = header_fixed_height,
+                    BackgroundColour = new OsuColour().Red.Opacity(0.5f),
+                });
+
+            AddToggleStep("disable footer", v => container.Footer = v
+                ? null
+                : new TestBox("Footer")
+                {
+                    RelativeSizeAxes = Axes.X,
+                    Height = 200,
+                    BackgroundColour = new OsuColour().Green4,
+                });
         }
 
         [Test]
@@ -71,7 +105,6 @@ namespace osu.Game.Tests.Visual.UserInterface
         {
             const int sections_count = 11;
             float[] alternating = { 0.07f, 0.33f, 0.16f, 0.33f };
-            AddStep("clear", () => container.Clear());
             AddStep("fill with sections", () =>
             {
                 for (int i = 0; i < sections_count; i++)
@@ -84,9 +117,9 @@ namespace osu.Game.Tests.Visual.UserInterface
                 AddUntilStep("correct section selected", () => container.SelectedSection.Value == container.Children[scrollIndex]);
                 AddUntilStep("section top is visible", () =>
                 {
-                    float scrollPosition = container.ChildrenOfType<UserTrackingScrollContainer>().First().Current;
+                    var scrollContainer = container.ChildrenOfType<UserTrackingScrollContainer>().Single();
-                    float sectionTop = container.Children[scrollIndex].BoundingBox.Top;
+                    float sectionPosition = scrollContainer.GetChildPosInContent(container.Children[scrollIndex]);
-                    return scrollPosition < sectionTop;
+                    return scrollContainer.Current < sectionPosition;
                 });
             }
 
@@ -101,15 +134,56 @@ namespace osu.Game.Tests.Visual.UserInterface
             AddUntilStep("correct section selected", () => container.SelectedSection.Value == container.Children[sections_count - 1]);
         }
 
-        private static readonly ColourInfo selected_colour = ColourInfo.GradientVertical(Color4.Yellow, Color4.Gold);
+        [Test]
+        public void TestNavigation()
+        {
+            AddRepeatStep("add sections", () => append(1f), 3);
+            AddUntilStep("wait for load", () => container.Children.Any());
+
+            AddStep("hover sections container", () => InputManager.MoveMouseTo(container));
+            AddStep("press page down", () => InputManager.Key(Key.PageDown));
+            AddUntilStep("scrolled one page down", () =>
+            {
+                var scroll = container.ChildrenOfType<UserTrackingScrollContainer>().First();
+                return Precision.AlmostEquals(scroll.Current, Content.DrawHeight - header_fixed_height, 1f);
+            });
+
+            AddStep("press page down", () => InputManager.Key(Key.PageDown));
+            AddUntilStep("scrolled two pages down", () =>
+            {
+                var scroll = container.ChildrenOfType<UserTrackingScrollContainer>().First();
+                return Precision.AlmostEquals(scroll.Current, (Content.DrawHeight - header_fixed_height) * 2, 1f);
+            });
+
+            AddStep("press page up", () => InputManager.Key(Key.PageUp));
+            AddUntilStep("scrolled one page up", () =>
+            {
+                var scroll = container.ChildrenOfType<UserTrackingScrollContainer>().First();
+                return Precision.AlmostEquals(scroll.Current, Content.DrawHeight - header_fixed_height, 1f);
+            });
+        }
+
+        private static readonly ColourInfo selected_colour = ColourInfo.GradientVertical(new OsuColour().Orange2, new OsuColour().Orange3);
         private static readonly ColourInfo default_colour = ColourInfo.GradientVertical(Color4.White, Color4.DarkGray);
 
         private void append(float multiplier)
         {
-            container.Add(new TestSection
+            float fixedHeaderHeight = container.FixedHeader?.Height ?? 0;
+            float expandableHeaderHeight = container.ExpandableHeader?.Height ?? 0;
+
+            float totalHeaderHeight = expandableHeaderHeight + fixedHeaderHeight;
+            float effectiveHeaderHeight = totalHeaderHeight;
+
+            // if we're in the "next page" of the sections container,
+            // height of the expandable header should not be accounted.
+            var scrollContent = container.ChildrenOfType<UserTrackingScrollContainer>().Single().ScrollContent;
+            if (totalHeaderHeight + scrollContent.Height >= Content.DrawHeight)
+                effectiveHeaderHeight -= expandableHeaderHeight;
+
+            container.Add(new TestSection($"Section #{container.Children.Count + 1}")
             {
                 Width = 300,
-                Height = (container.ChildSize.Y - header_height) * multiplier,
+                Height = (Content.DrawHeight - effectiveHeaderHeight) * multiplier,
                 Colour = default_colour
             });
         }
@@ -120,11 +194,50 @@ namespace osu.Game.Tests.Visual.UserInterface
             InputManager.ScrollVerticalBy(direction);
         }
 
-        private class TestSection : Box
+        private class TestSection : TestBox
         {
             public bool Selected
             {
-                set => Colour = value ? selected_colour : default_colour;
+                set => BackgroundColour = value ? selected_colour : default_colour;
+            }
+
+            public TestSection(string label)
+                : base(label)
+            {
+                BackgroundColour = default_colour;
+            }
+        }
+
+        private class TestBox : Container
+        {
+            private readonly Box background;
+            private readonly OsuSpriteText text;
+
+            public ColourInfo BackgroundColour
+            {
+                set
+                {
+                    background.Colour = value;
+                    text.Colour = OsuColour.ForegroundTextColourFor(value.AverageColour);
+                }
+            }
+
+            public TestBox(string label)
+            {
+                Children = new Drawable[]
+                {
+                    background = new Box
+                    {
+                        RelativeSizeAxes = Axes.Both,
+                    },
+                    text = new OsuSpriteText
+                    {
+                        Anchor = Anchor.Centre,
+                        Origin = Anchor.Centre,
+                        Text = label,
+                        Font = OsuFont.Default.With(size: 36),
+                    }
+                };
             }
         }
     }

osu.Game/Graphics/Containers/SectionsContainer.cs

@@ -149,13 +149,11 @@ namespace osu.Game.Graphics.Containers
         {
             lastKnownScroll = null;
 
-            float fixedHeaderSize = FixedHeader?.BoundingBox.Height ?? 0;
-
             // implementation similar to ScrollIntoView but a bit more nuanced.
             float top = scrollContainer.GetChildPosInContent(target);
 
-            float bottomScrollExtent = scrollContainer.ScrollableExtent - fixedHeaderSize;
+            float bottomScrollExtent = scrollContainer.ScrollableExtent;
-            float scrollTarget = top - fixedHeaderSize - scrollContainer.DisplayableContent * scroll_y_centre;
+            float scrollTarget = top - scrollContainer.DisplayableContent * scroll_y_centre;
 
             if (scrollTarget > bottomScrollExtent)
                 scrollContainer.ScrollToEnd();
@@ -270,9 +268,13 @@ namespace osu.Game.Graphics.Containers
         {
             if (!Children.Any()) return;
 
-            var newMargin = originalSectionsMargin;
+            // if a fixed header is present, apply top padding for it
+            // to make the scroll container aware of its displayable area.
+            // (i.e. for page up/down to work properly)
+            scrollContainer.Padding = new MarginPadding { Top = FixedHeader?.LayoutSize.Y ?? 0 };
 
-            newMargin.Top += (headerHeight ?? 0);
+            var newMargin = originalSectionsMargin;
+            newMargin.Top += (ExpandableHeader?.LayoutSize.Y ?? 0);
             newMargin.Bottom += (footerHeight ?? 0);
 
             scrollContentContainer.Margin = newMargin;