mabe02 / lanterna

Java library for creating text-based GUIs
GNU Lesser General Public License v3.0
2.23k stars 243 forks source link

Fix: pass correct target focus to previous focus #562

Closed zyuiop closed 11 months ago

zyuiop commented 2 years ago

Hi,

When using this library, I noticed that when overriding afterLeaveFocus(direction: Interactable.FocusChangeDirection, nextInFocus: Interactable), the nextInFocus variable was actually set to the current Interactable and not the target one.

This patch fixes that.

Thank you

mabe02 commented 11 months ago

Thank you!

mabe02 commented 11 months ago

Oops, noticed too late that there's an extra file included by mistake in the PR; I'll remove it

avl42 commented 11 months ago

Isn't that wrong?

When you set focus on a new component, then the currently focused component ought to receive the "Focus Leave" event...

What problem was this meant to fix?

mabe02 @.***> schrieb am Fr., 14. Juli 2023, 15:16:

Oops, noticed too late that there's an extra file included by mistake in the PR; I'll remove it

— Reply to this email directly, view it on GitHub https://github.com/mabe02/lanterna/pull/562#issuecomment-1635852028, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABIDBMR2BQRHEOUV22DHOV3XQFBEBANCNFSM5VWOZ3OQ . You are receiving this because you are subscribed to this thread.Message ID: @.***>

zyuiop commented 11 months ago

The focused interactable receives the event as you say, but the nextInFocus should be the newly focused element, not the current one.

mabe02 commented 11 months ago

I first thought it was wrong too, but then I re-read it and changed my mind. Let me check again...

mabe02 commented 11 months ago

Looks correct. Javadoc:

    /**
     * Method called when keyboard focus moves away from this component
     * @param direction What direction is focus going in
     * @param nextInFocus Which component is receiving focus next (or {@code null} if none)
     */
    void onLeaveFocus(FocusChangeDirection direction, Interactable nextInFocus);

Diff

         if(focusedInteractable != null) {
-            focusedInteractable.onLeaveFocus(direction, focusedInteractable);
+            focusedInteractable.onLeaveFocus(direction, toFocus);
         }

We were passing in the component itself to nextInFocus, but it should be the component that is taking over focus.

avl42 commented 11 months ago

Oups, sorry....

I misunderstood the onLeaveFocus-call.

The correction seems correct (now to me, too).

mabe02 @.***> schrieb am Sa., 15. Juli 2023, 14:36:

Looks correct. Javadoc:

/**
 * Method called when keyboard focus moves away from this component
 * @param direction What direction is focus going in
 * @param nextInFocus Which component is receiving focus next (or ***@***.*** null} if none)
 */
void onLeaveFocus(FocusChangeDirection direction, Interactable nextInFocus);

Diff

     if(focusedInteractable != null) {
  • focusedInteractable.onLeaveFocus(direction, focusedInteractable);
  • focusedInteractable.onLeaveFocus(direction, toFocus); }

We were passing in the component itself to nextInFocus, but it should be the component that is taking over focus.

— Reply to this email directly, view it on GitHub https://github.com/mabe02/lanterna/pull/562#issuecomment-1636755008, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABIDBMQEX7BOJHIKQ6O5ODLXQKFENANCNFSM5VWOZ3OQ . You are receiving this because you commented.Message ID: @.***>