Unexpected tab selection change event in beforeEnter
Closed this issue · 3 comments
@steffen-harbich-itc commented on Fri Aug 10 2018
I am trying to make a primary navigation using the Tabs component and routing. Please see my attached example project reproducing the issue. A selection change event is fired unexpectedly although I unregistered the listener before selecting the tab. To reproduce, start the maven goal "spring-boot:run", open the site at localhost:8080 and do:
- click on tab SubView3 (navigates to /subView3)
- click on link above tabs, produces logs:
Before enter: SubView1Sub1
Tab selected: SubView1
Before enter: SubView1
Instead of just navigating to the sub sub view 1, additionally it is called a tab selection change event on tab SubView1 which opens that view. But that selection change event is not expected since in MainView#beforeEnter I unregister the event listener before selecting the tab.
Tested with 10.0.2 as well as 11.0.0.beta1, same behavior.
Example project:
my-starter-project.zip
@denis-anisimov commented on Fri Aug 10 2018
The reason of this behavior is: selection event is fired from the client side.
That's why you get the event even though you have unregister the listener.
You don't get the event when you set selection from the server side. Because at this time listener is
unregistered.
But then you register it back and a selection event from the client side triggers your listener.
It looks like we get selection events for the "selected" property even if it has the same value. "selected" property is synchronized with the client side and an event is sent from the client side every time when selection is changed even if the value is the same.
This should not happen.
This is definitely a bug and needs to be fixed somehow.
Actually.........
I don't think this is a bug anymore.
The problem is the addSelectedChangeListener API method.
You expect that this method works from the server side.
But this is not true.
This method adds a listener for SelectedChangeEvent.
This event is a @DomEvent which is always fired from the client side.
And as I said in your case you are getting this event not at the point where you expected (when you have removed your listener there is no any event. The event is fired from the client side when the listener is added back already).
So there is a confusion in selection listener: it always gets events from the client-side and current behavior is expected.
If you want to listen for selection on the server side (it also allows listen from the client side but doesn't duplicate event) then you should add a property change listener for "selected" property:
tabs.getElement().addPropertyChangeListener("selected", ...).
In this case the even will be fired from the server side (if your change is on the server side) and should not be fired from the client side one more time since the value set from the server side is the same.
I just verified that.
(The event will be also fired from the client side if it's a source of the event).
I think we should improve the javadocs for the addSelectedChangeListener method.
But otherwise there is no bug.
Though this is definitely a confusing behavior.
Please reopen if you disagree and let me know if you have an idea how to make it behave properly.
Ok, it works with property change listener on tabs element. But this is totally unexpected for me as a user of the framework. I don't expect to get an event from the client-side after I made an tab selection change on the server-side.
Hmm...
So are you saying that there shouldn't be SelectedChangeEvent event at all?
Just to clarify: SelectedChangeEvent is a pure client side event. So it cannot be fired from the server side.
If you don't expect the client side event when you do a server side change then there shouldn't be any SelectedChangeEvent event at all in case the selection is done from the server side.
It might be it makes sense. I'm not sure.
If you think there should not be SelectedChangeEvent event on server change then could you please create a separate ticket?
I think there will be some discussion about this. It's more convenient to do this discussion in the separate ticket.
Thanks!