Skip to content

Conversation

@peppy
Copy link
Member

@peppy peppy commented Aug 4, 2025

Closes #34483.

Have checked all usages of the class and this looks like the behaviour we want.

@peppy peppy added the quick fix Tasks which were taken on because they take no time to fix label Aug 4, 2025
@bdach bdach self-requested a review August 4, 2025 07:48
Copy link
Collaborator

@bdach bdach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test failures relevant, right clicking on the user's best score does not open a context menu anymore

@peppy
Copy link
Member Author

peppy commented Aug 4, 2025

The setup we have for context menus is super annoying to handle isn't it..

@bdach
Copy link
Collaborator

bdach commented Aug 4, 2025

now right clicking doesn't work on any of the other scores, and the context menu for the only score that it works for now is very weirdly offset

Screen.Recording.2025-08-04.at.10.42.44.mov

@peppy peppy force-pushed the wedge-background-click-block branch from 436785b to 3557207 Compare August 4, 2025 08:52
@pull-request-size pull-request-size bot added size/M and removed size/L labels Aug 4, 2025
@peppy
Copy link
Member Author

peppy commented Aug 4, 2025

tableflip

Comment on lines +152 to 156
// Required because wedge background blocks input from passing through
// to the main context menu container above.
new OsuContextMenuContainer
{
RelativeSizeAxes = Axes.X,
AutoSizeAxes = Axes.Y,
Shear = -OsuGame.SHEAR,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is only one step removed from a thing #33687 does which I've called "horrible". I guess at least this example doesn't have a "shear again after un-shearing in parent" applied to a child which is 🤯 tier to me, but I'm not sure that's much of a redeeming factor either

so I guess to not be a hypocrite I should either approve both of these, or neither. I'd personally lean towards "neither" and think about how to address this recurring pain point in framework but if I get overruled by general consensus then that'll be that.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like at some point we'll break and fix context menu container, which will involve a full pass over which ones are no longer necessary. So while adding this is ugly as hell, it's not making things worse from a code maintenance angle because it can be removed in one line.

@bdach
Copy link
Collaborator

bdach commented Aug 4, 2025

test failure semi-relevant, fixed e.g. by

diff --git a/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs b/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs
index 466fbf92a8..4491061195 100644
--- a/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs
+++ b/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs
@@ -679,8 +679,6 @@ public void TestDeleteScoreAfterPlaying()
             AddStep("click delete", () =>
             {
                 var dropdownItem = Game
-                                   .ChildrenOfType<BeatmapLeaderboardWedge>().First()
-                                   .ChildrenOfType<OsuContextMenu>().First()
                                    .ChildrenOfType<DrawableOsuMenuItem>().First(i => i.Item.Text.ToString() == "Delete");
 
                 InputManager.MoveMouseTo(dropdownItem);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:song-select quick fix Tasks which were taken on because they take no time to fix size/M

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

[song select ui] hovering over space right below own top score shows details of the score hovered over

2 participants