gwtproject / gwt

GWT Open Source Project
http://www.gwtproject.org
1.52k stars 376 forks source link

CustomButton discards control, alt, shift and meta key info in ClickEvent #9537

Open pghj opened 7 years ago

pghj commented 7 years ago

GWT version: All Browser (with version): All Operating System: All


Description

CustomButton (and all extending classes) discard info about control, alt, shift and meta keys being held down. This is not documented and confusing because the fired ClickEvent exposes methods like isShiftKeyDown().

This happens because CustomButton generates a synthetic event in onClick() with all flags set to false:

NativeEvent evt = Document.get().createClickEvent(1, 0, 0, 0, 0, false,
        false, false, false);

CustomButton invokes the onClick() method from onBrowserEvent(Event event) without passing on (information from) the source event.

I think that creating a separate onClick(boolean control, boolean alt, boolean shift, boolean meta) and keeping onClick() only to notify extending classes would fix this.

UPDATE: to not alter the documented behaviour of onClick() ("The default behaviour is to fire the click event to listeners.") it might be better to temporarily store the info about control/alt/shift in an instance variable and let onClick() pick it up and fire the event.

Steps to reproduce

Click a PushButton while holding down shift and observe event dispatched through addClickHandler(ClickEvent).

Known workarounds

Nothing remotely easy.

tbroyer commented 7 years ago

onClick could probably use com.google.gwt.user.client.Event#getCurrentEvent() to get the current click event and its key info; though I'd actually be inclined to just say that it's a limitation of the current implementation with no intention to change it (though why not document it, sure).

BTW, most PushButton uses can be replaced with a Button and appropriate CSS (if you ask me, we should have deprecated it years ago, along with CustomButton and ToggleButton; ToggleButton would need a bit more effort but could be replaced with a Checkbox and appropriate CSS).

I'll let others chime in and triage the issue.