vaadin / flow

Vaadin Flow is a Java framework binding Vaadin web components to Java. This is part of Vaadin 10+.
Apache License 2.0
600 stars 165 forks source link

ComponentEvent should expose UI #18818

Open mstahv opened 6 months ago

mstahv commented 6 months ago

UI reference is often needed for various tasks. Because of the previous Optional buzz Component.getUI returns an Optional and leads to obsolete "rituals" in UI code or usaged of somewhat less elegant UI.getCurrent() that draws the UI from a thread local.

Because UI is pretty much always present when ComponentEvent is fired, it could maybe provide a shorthand for it.

For example this snippet in an event listener (e being ComponentEvent):

e.getSource().getUI().ifPresent(ui -> ui.navigate(GridView.class));

... would become

e.getUI().navigate(GridView.class);

And the UI exprience would become better if something really goes wrong: the user/developer would see an exception instead of simply non-functional UI (exception eated).

Describe alternatives you've considered

Keep writing the ugly code.

Additional context

About the potential implementation: there are some edge case where UI might still be null, e.g. setting a value for a textfield that is not yet attached. In these cases in 99.9% of the time the value coming from thread local would still be valid. I'd use that as a fallback and document the behaviour in JavaDoc.

mstahv commented 6 months ago

Drafted what the change could be in https://github.com/vaadin/flow/commit/3203659f29933187c281962d1d620d0aedf0f2fb

In case people use events in "creative ways" a more stable solution might be to do the same in the constructor and save it to a field in ComponentEvent. But This would then be a bit of an overhed (until JIT compilation optimisations kick in 🤷‍♂️).

mstahv commented 5 months ago

Related: https://github.com/viritin/flow-viritin/commit/c3a5692e42bda27d67f45503c426087160765fe4

That is basically another kind of trial to workaround this. Basically exposing the navigate method to component level.