Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix item labels not generated while dropdown is in Ready state #5850

Merged
merged 5 commits into from
Jun 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
65 changes: 52 additions & 13 deletions osu.Framework.Tests/Visual/UserInterface/TestSceneDropdown.cs
Original file line number Diff line number Diff line change
Expand Up @@ -369,12 +369,19 @@ public void TestItemSource()
AddStep("close dropdown", () => InputManager.Key(Key.Escape));
}

/// <summary>
/// Adds an item before a dropdown is loaded, and ensures item labels are assigned correctly.
/// </summary>
/// <remarks>
/// Ensures item labels are assigned after the dropdown finishes loading (reaches <see cref="LoadState.Ready"/> state),
/// so any dependency from BDL can be retrieved first before calling <see cref="Dropdown{T}.GenerateItemText"/>.
/// </remarks>
[Test]
public void TestAccessBdlInGenerateItemText()
public void TestAddItemBeforeDropdownLoad()
{
BdlDropdown dropdown = null!;

AddStep("add dropdown that uses BDL", () => Add(dropdown = new BdlDropdown
AddStep("setup dropdown", () => Add(dropdown = new BdlDropdown
{
Width = 150,
Position = new Vector2(250, 350),
Expand All @@ -385,27 +392,59 @@ public void TestAccessBdlInGenerateItemText()
}

/// <summary>
/// Checks that <see cref="Dropdown{T}.GenerateItemText"/> is not called before load when initialising with <see cref="Dropdown{T}.Current"/>.
/// Adds an item after the dropdown is in <see cref="LoadState.Ready"/> state, and ensures item labels are assigned correctly and not ignored by <see cref="Dropdown{T}"/>.
/// </summary>
[Test]
public void TestBdlWithCurrent()
public void TestAddItemWhileDropdownIsInReadyState()
{
BdlDropdown dropdown = null!;

Bindable<TestModel> bindable = null!;
AddStep("setup dropdown", () =>
{
Add(dropdown = new BdlDropdown
{
Width = 150,
Position = new Vector2(250, 350),
});

dropdown.Items = new TestModel("test").Yield();
});

AddAssert("text is expected", () => dropdown.Menu.DrawableMenuItems.First(d => d.IsSelected).ChildrenOfType<SpriteText>().First().Text.ToString(), () => Is.EqualTo("loaded: test"));
}

/// <summary>
/// Sets a non-existent item dropdown and ensures its label is assigned correctly.
/// </summary>
/// <param name="afterBdl">Whether the non-existent item should be set before or after the dropdown's BDL has run.</param>
[Test]
public void TestSetNonExistentItem([Values] bool afterBdl)
{
BdlDropdown dropdown = null!;
Bindable<TestModel> bindable;

AddStep("add items to bindable", () => bindableList.AddRange(new[] { "one", "two", "three" }.Select(s => new TestModel(s))));
AddStep("create current", () => bindable = new Bindable<TestModel>(bindableList[1]));

AddStep("add dropdown that uses BDL", () => Add(dropdown = new BdlDropdown
AddStep("add dropdown that uses BDL", () =>
{
Width = 150,
Position = new Vector2(250, 350),
ItemSource = bindableList,
Current = bindable,
}));
bindable = new Bindable<TestModel>();

if (!afterBdl)
bindable.Value = new TestModel("non-existent item");

Add(dropdown = new BdlDropdown
{
Width = 150,
Position = new Vector2(250, 350),
ItemSource = bindableList,
Current = bindable,
});

if (afterBdl)
bindable.Value = new TestModel("non-existent item");
});

AddAssert("text is expected", () => dropdown.Menu.DrawableMenuItems.First(d => d.IsSelected).ChildrenOfType<SpriteText>().First().Text.ToString(), () => Is.EqualTo("loaded: two"));
AddAssert("text is expected", () => dropdown.SelectedItem.Text.Value.ToString(), () => Is.EqualTo("loaded: non-existent item"));
}

private void toggleDropdownViaClick(TestDropdown dropdown, string dropdownName = null) => AddStep($"click {dropdownName ?? "dropdown"}", () =>
Expand Down
4 changes: 2 additions & 2 deletions osu.Framework/Graphics/UserInterface/Dropdown.cs
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,8 @@ private void addDropdownItem(T value)
Menu.State = MenuState.Closed;
});

// inheritors expect that `virtual GenerateItemText` is only called when this dropdown is fully loaded.
if (IsLoaded)
// inheritors expect that `virtual GenerateItemText` is only called when this dropdown's BDL has run to completion.
if (LoadState >= LoadState.Ready)
item.Text.Value = GenerateItemText(value);

Menu.Add(item);
Expand Down