Do not unfocus on click if the focused drawable changed during the click

[smoogipoo]

RFC.

I'm not happy with this because means that FocusedDrawable is null during OnClick() handling. What I'd really like to do is "snapshot" FocusedDrawable. I'm not sure/don't think anything expects FocusedDrawable to be correct during click handling, though.

[bdach]

• Tests failing.
• What's the relation between this and https://github.com/ppy/osu-framework/pull/6346, because it's unclear to me? Does this supersede that PR, compound it...? There's something in that pull about this "removing a schedule", is that the extent of the interaction?

What I'd really like to do is "snapshot" FocusedDrawable.

I would really much prefer that too, why isn't this done? At least conceptually it seems like it should be at most a minor adjustment to this diff - something like so?

diff --git a/osu.Framework/Input/MouseButtonEventManager.cs b/osu.Framework/Input/MouseButtonEventManager.cs
index d08a5aafc..4884224a1 100644
--- a/osu.Framework/Input/MouseButtonEventManager.cs
+++ b/osu.Framework/Input/MouseButtonEventManager.cs
@@ -155,17 +155,13 @@ private void handleClick(InputState state, List<Drawable>? targets)
                                    .Where(t => t.IsAlive && t.IsPresent && t.ReceivePositionalInputAt(state.Mouse.Position));

             Drawable focusedDrawableBeforeClick = InputManager.FocusedDrawable;
-            InputManager.FocusedDrawable = null;

             Drawable? clicked = PropagateButtonEvent(drawables, new ClickEvent(state, Button, MouseDownPosition));
             ClickedDrawable.SetTarget(clicked!);

             // Focus shall only change if it wasn't changed during the click (for example, using a button to open a menu).
-            if (InputManager.FocusedDrawable == null)
+            if (ReferenceEquals(InputManager.FocusedDrawable, focusedDrawableBeforeClick))
             {
-                // Restore the previous focus target (it may get unfocused below).
-                InputManager.FocusedDrawable = focusedDrawableBeforeClick;
-
                 if (ChangeFocusOnClick && clicked?.ChangeFocusOnClick != false)
                     InputManager.ChangeFocusFromClick(clicked);
             }

[smoogipoo]

Tests failing.

Whoops, I didn't notice. I'll need to think about this one a bit more because it's an interesting/weird case.

What's the relation between this and https://github.com/ppy/osu-framework/pull/6346

This is all about removing that schedule from #6346. The schedule was initially present because some tests would open the dropdown by clicking on a test button, which is exactly the interaction demonstrated by TestChangeFocusDuringInputHandling_ShouldRetainFocus in this PR. #6346 expands that interaction so it happens on every click of the header instead, rather than some special case.

But, #6346 works on its own regardless of this PR.

At least conceptually it seems like it should be at most a minor adjustment to this diff

The problem with your diff is if you click the button twice, the first time it will work correctly because the reference changes, but the second time the reference will not have changed and focus will be changed (suppose that button press action is Menu.Open() as in this PR's added test).

So by snapshotting I'm really imagining a system where FocusedDrawable remains unchanged during a click and the new value is transferred to it after the click concludes.

[smoogipoo]

Alright, I basically implemented what I meant by "snapshotting" in https://github.com/ppy/osu-framework/pull/6345/commits/055b53dd86e528a767fb6758fc483536c2e8fbf7, should fix the tests.

[peppy]

Going to get this one in for a release.