-
Notifications
You must be signed in to change notification settings - Fork 67
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
AppLayout does not provide UI in afterNavigation method #5449
Comments
The UI should be available via |
The AppLayout afterNavigation method does not have an event. The issue targets the AppLayout method, not the AfterNavObserver one. |
Oh.. now I see what you mean.. you are using the protected method which probably should never been protected.. this is way to early in the router life circle and won't have access to the UI |
Jep, the method is used by the starter apps as an example. Might be good to also make it private or rename it to something more disambigious, when not using the ANO as a basis for this functionality. |
Note, you can implement the true afterNavigation by implementing AfterNavigationObserver in your MainLayout. This method in AppLayout is a bit badly named as it is not based on same mechanism. |
I guess it would make sense to make it private. |
The default implementation sets the mobile behavior of the Drawer, I would assume that method is protected in order to override that behavior if needed. |
Description
The design of the afterNavigation method in the
AppLayout
is contradictory to the one of theAfterNavigationObserver
. While in the method of theANO
interface the UI can be accessed viagetUI()
, the method of theAppLayout
does not yet provide it. This is due to the place where the method is called by the class.This may lead to confusion for devs, as they might expect the same behavior based on the same method naming and similar documentation.
Expected outcome
The UI should be available when calling getUI inside the AppLayouts
afterNavigation
.Possible solutions:
getUI
in theAppLayout
to returnUI.getCurrent()
as an alternative, if the component has not yet been attached to the UI. Not sure if this could lead to a false UI in any normal case?AppLayout
implement theAfterNavigationObserver
interface and move the call ofafterNavigation
there. This could be classified as a breaking change as it might interfere with any manual implementation of the interface. It would however align more with our base interfaces and the intended usage.Minimal reproducible example
Steps to reproduce
getUI()
inside the pre-implementedafterNavigation
method. The returned value is an empty optional.Environment
Vaadin version(s): 24.1.8
OS: Windows
Browsers
Issue is not browser related
The text was updated successfully, but these errors were encountered: