search

mabe02 / lanterna

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

use mouse down for activation of components #488

Closed ginkoblongata closed 4 years ago

ginkoblongata commented 4 years ago

Seeking comment.

Looks like the code was taking a direction that I'm not sure I understand the rationale, it would take two clicks for things, one for the focus and another for the activation.

Since mouse is newer terminals, not sure the case where this would be needed or desired.

This pull request collapses those, so clicking a button once causes the action. Clicking a checkbox once causes the focus and the checked state to toggle. Clicking the RadioButton causes the focus and that button state to be true, etc.

avl42 commented 4 years ago

I think, it makes sense to let certain widgets already do their action on first click, and depending on the widget it isn't even necessary to set focus on it at all, so let the "set focus" be the default (for interactive widgets), and for specifc widgets either override it for a specific action (buttons, checkboxes), or just continue having them focused (text boxes, although they could in future also set the cursor to (or nearest to) the clicked position, which is quite non-trivial to implement.)

There might also be specific widgets, that may still use first click to focus them, and only every further click (i.e. a click, while they're already focused) to trigger further action. That would be widgets that would be primarly keyboard-controlled, so a user could click on it for focus and then continue with keyboard, without having that click already do anything else - TextBox would be an example for that, in my humble opinion.

ginkoblongata commented 4 years ago

Seeking coment:

For certain components, doing the action right away on mouse click is good. They still should also then be focused so that the keyboard interaction works seamlessly. I believe that I have done that approach across the widgets in this pull request.

Looks like space and enter were 'actioning' keys on some components, some others were only enter so I made them have space as well for consistency.

I also added some clipping logic to prevent some exceptions when the mouse event locations would map to imaginary items in the lists and tables and text boxes.

For the table test, I needed to have it rendered for the accurate clipping log, seems to work on my machine (mvn clean install), not sure if popping up that swing window for couple milliseconds will be a build problem.

How would you like to proceed? I think that I am basically done with this issue. There is another enhancement I had in mind but I think maybe it would be best to put into a new request. For the Menu, when the user clicks 'outside' it seems it should perform the logic which the ESC key performs as well.

ginkoblongata commented 4 years ago

Looks like there is still a thing left to do. The mouse down is not taking into account being scrolled downward in TextBox, ActionListBox, CheckBoxList, RadioBoxList.

ginkoblongata commented 4 years ago

Ok, looks like it's a wrap, got the mouse down working for list style widgets.

I am wondering how to proceed to have this pull request merged. I was thinking it would be best for it to merge before proceeding onto other things so it doesn't topple.

mabe02 commented 4 years ago

Could you change this PR to go against release/3.1 branch instead of master?

ginkoblongata commented 4 years ago

Could you change this PR to go against release/3.1 branch instead of master?

Yes.

mabe02 commented 4 years ago

Thanks, that's merged now

ginkoblongata commented 4 years ago

Ok, great! Thanks.