mabe02 / lanterna

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

Mouse Input needs to differentiate mouse down vs others #487

Closed ginkoblongata closed 4 years ago

ginkoblongata commented 4 years ago

In the components such as CheckBox, Menu, etc, the method handleKeyStroke does not differentiate between mouse down and mouse up and mouse drag.

The effect is that CheckBox can only be Activated by clicking on it, but never Deactivated.

Some context: Using MouseCaptureMode.CLICK_RELEASE_DRAG in Konsole on OpenSuse Both MouseDown and MouseUp cause the CheckBox to be activated as they are KeyType.MouseEvent.

(Mode MouseCaptureMode.CLICK is not sufficient for the application, however it also does not appear to yield MouseEvents in Konsole)

Looks like need to have the terminal MouseCaptureMode accessible to the Component.

AbstractInteractableComponent may be a place to put a back pointer to the terminal.

I am not familiar enough with the codebase yet for other suggestions.

ginkoblongata commented 4 years ago

For the component, it could somehow check the MouseCaptureMode and perform different logic for the mode.

However, it may be sufficient to just narrow down the various handleKeyStroke() methods to check specifically for MouseActionType.CLICK_DOWN, rather than only the broader KeyType.MouseEvent.

Looking at CheckBox, but there are a bunch of components with this similar logic that would likely all be affected.

This looks like it works in the case of the CheckBox component.

    @Override
    public Result handleKeyStroke(KeyStroke keyStroke) {
        boolean isMouseActivation = false;
        if (keyStroke instanceof MouseAction) {
            MouseAction action = (MouseAction)keyStroke;
            isMouseActivation = action.getActionType() == MouseActionType.CLICK_DOWN;
        }
        boolean isKeyboardActivation = (keyStroke.getKeyType() == KeyType.Character && keyStroke.getCharacter() == ' ') || keyStroke.getKeyType() == KeyType.Enter;

        if ((isKeyboardActivation || isMouseActivation) && isFocused()) {
            setChecked(!isChecked());
            return Result.HANDLED;
        }
        return super.handleKeyStroke(keyStroke);
    }

Similar approach probably can be used for others, like Menu, etc.

avl42 commented 4 years ago

Would you please create a PR for it?

The more further components get fixed in the PR for MouseCaptureMode.CLICK_RELEASE_DRAG, the better :-)

PS: my local (freshly pulled - master) checkout of lanterna doesn't yet have any check for MouseActions at all in class CheckBox ... - is it in a specific branch?

ginkoblongata commented 4 years ago

I am looking at 3.1.0-SNAPSHOT.

I will see what I can do.

ginkoblongata commented 4 years ago

I have created this pull request: https://github.com/mabe02/lanterna/pull/488

I was thinking to add some more to it (did not do table component yet) but would like some comments around the mouse behavior, I see that it was purposefully taking two clicks to cause action buttons to fire the event and for CheckBox to be toggled, that seemed to lack basis, maybe I missed something.

ginkoblongata commented 4 years ago

I have created another pull request for this issue: https://github.com/mabe02/lanterna/pull/493

Context & discussion in the pull request.

mabe02 commented 4 years ago

Look good, I've merged the PR. Should I close this issue too?

ginkoblongata commented 4 years ago

Thanks!