ViewEvent API / Behaviour of not implemented features

[Smithor]

Second of the two problems from #7, not related to the TextField. I am moving this reply to a new issue since it is not related to the issue that spawned it.

Re: Action eventFor the second issue, my idea is that many different user input events can generate an action event (key presses, mouse clicks), so the action event is a separate kind with the parent being it's cause. However, maybe it makes sense for Action events to inherit attributes of the parent as a convenience?

At first I was like "I want to press enter and have the application do something."

	@Override
	protected void respondUI(ViewEvent anEvent) {
		if ((anEvent.isEnterKey())) {
			//Do thing
		}
	}

"It's in the contract, done, simple"

Having that not work was surprising, and wasn't great. The solution is quite simple, grab the parent and (if that parent is a Swing event) it works, but having a method like isEnterKey() not working when the action is created by an enter key is also not great.

This is what I've got essentially running right now to accomplish this.

	@Override
	protected void respondUI(ViewEvent anEvent) {
		if ((anEvent.getParentEvent() instanceof SwingEvent swingEvent && swingEvent.isEnterKey())) {
			//Do thing
		}
	}

After checking the source, I can see that the default implementation is for any non-implemented feature to print to the error console, but in my case, I did not actually get a print to console, making this a difficult error to chase down, when a NotImplementedException would have had me catch this right away. Having features not be implemented is totally fine, especially when having convenience methods in a flexible class, but a silent failure/ return of a totally valid value (such as getKeyCode() returning 0) is very misleading and makes it more difficult to chase down. Probably wouldn't have even posted the bug report in that case, would have caught the exception in the debugger, and then noticed the parent and would have seen that the parent did have the correct info.

As far as the Action Event inheriting info from parent, I think it could go one of three ways.

1. The API from ViewEvent can be used to see the details from the proxied parent event.
2. The ViewEvent methods throw a NotImplementedException and maybe it gives a hint to where to find the correct implementation (Such as, "check parent event for details").
3. The methods could be moved from ViewEvent (Where they are not implemented anyway), and added to SwingEvent (Where they are implemented/used). No confusion, and no loss of the methods, as they will only be present where they are actually used.

I personally would prefer option 1 or 2. One would be best, super smooth use of the library, but not sure how much work it would require. Option 2 wouldn't fix the problem, but it would at least make it so the user knows to look elsewhere for their solution. I can see option 3 not being perfect because you use the same contract for SwingEvent and CJEvent, so maybe those two classes need an abstract parent that creates that contract, rather than not being implemented in ViewEvent, but contract accessible. That could fix the problem, but would require a bit of refactoring.

[reportmill]

Okay, I modified SwingEvent so that Action events now assume the modifiers and key attributes of parent events (for some reason the browser code path was already doing this). I'll release this in an update in a couple hours. Thanks for pointing this out - I think this is a great improvement.

However, my idea with resetUI()/respondUI() is that everything is named based and respondUI() is the default callback handler for all UI Action events. So in my opinion it would be safer to have respondUI() look like this:

/**
 * Called when UI controls fire Action event.
 */
protected void respondUI(ViewEvent anEvent) {
    switch (anEvent.getName()) {
        case "MyTextField" -> {
            if (anEvent.isEnterKey())
                System.out.println("Do thing");
        }
    }
}

However, a safer convention, since it is less common for UI controls to need to handle key events might be to just register to handle the key event directly:

/**
 * Override to register a handler for MyTextField receiving key release events.
 */
@Override
protected void initUI() {
    getView("MyTextField").addEventHandler(this::handleMyTextFieldKeyReleaseEvent, MouseRelease);
}

/**
 * Called when MyTextField gets KeyRelease event.
 */
private void handleMyTextFieldKeyReleaseEvent(ViewEvent anEvent) {
    if (anEvent.isEnterKey()) {
        // Do thing
    }
}

[Smithor]

However, my idea with resetUI()/respondUI() is that everything is named based and respondUI() is the default callback handler for all UI Action events. So in my opinion it would be safer to have respondUI() look like this:

The name based situation caught me off-guard at first, but honestly, I'm quite a fan. It's how I somewhat ran stuff in my own projects, but the not having to carry around references (and being able to use the xml files to build the ui) is a wonderful change. I didn't use it in the given example because I didn't want to restrict the reach of the button push in that particular example, but it's used and will be used elsewhere in the project in this fashion.

However, a safer convention, since it is less common for UI controls to need to handle key events might be to just register to handle the key event directly:

I did something similar to this in the same window for handling/harvesting the content inside the TextFields, just as a lambda expression instead.

A little aside, thank you for working on this library. I've been lurking for a while now, and finally found an excuse to use the library. You will probably see a lot more stuff from me as I work through it all.

[reportmill]

I’ve really enjoyed the ViewOwner structure of createUI/initUI/resetUI/respondUI. The name based resetUI() with the “unified” setters is easy to read/code/modify. And the name based respondUI() shows you exactly how all the UI is wired up in one central dispatch table (simplified by the unified event handling). To me, this is one of the primary things that Swing was missing that drove people crazy.

You don’t have to thank me - I appreciate you so much for taking the time to work with SnapKit and give feedback. I haven’t had a lot of luck generating interest. :-)