[
https://issues.apache.org/jira/browse/PIVOT-751?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13065903#comment-13065903
]
Greg Brown commented on PIVOT-751:
----------------------------------
As noted, the bogus event is fired because TerraTabPaneSkin sets the selected
index in response to the initial tab insert event, and then TabPane fires an
additional event based on incorrect information.
This represents an interesting design challenge. It is similar to the challenge
of keeping a ListView's model and selection state in sync. In Swing, this is
handled by the UI delegate (i.e. the skin). When the model fires events, the
skin is responsible for updating the component's selection state.
We intentionally chose not to do it this way since it seemed like the component
should be responsible for maintaining its own state internally. However, after
some further thought I'm wondering if the Swing approach might be better. We
had initially viewed the skin as the "view" and the component as the
"controller" - however, in retrospect, I think the component is actually the
"view-model", and the skin is the "view-controller". The component is part view
because it is the object that actually receives user input and paint calls
(though it delegates them to the skin). It is part model since it generally
hosts the model (either as an intrinsic property like "buttonData" or as an
attached model like "listData"). The skin is part view because it actually
handles the paint calls, and part controller since it handles user input and
other events and updates the component's (i.e. model's) state in response to
them.
When viewed as a controller, it seems valid for the skin to be responsible for
keeping the component's various models in sync. Since there will never be a
practical case where a component does not have a skin installed, this seems OK
to me. It means that, in this case, TabPane wouldn't fire an indirect selection
change event when a tab is inserted - it would be up to the skin to respond to
that event appropriately (which it already does). The skin should also do
something similar when a tab is removed.
What do you think? FYI, this would be a big change, as it affects the design of
many components. However, I think it could probably be restricted to TabPane,
Accordion, and CardPane for now and propagated to ListView, TableView, etc.
later.
> TabPaneSelectionListener#selectedIndexChanged called twice when first tab is
> inserted
> -------------------------------------------------------------------------------------
>
> Key: PIVOT-751
> URL: https://issues.apache.org/jira/browse/PIVOT-751
> Project: Pivot
> Issue Type: Bug
> Components: wtk
> Affects Versions: 2.0
> Reporter: Edvin Syse
> Priority: Minor
> Fix For: 2.0.1
>
> Attachments: PIVOT751.java, PIVOT751.txt
>
>
> If you add a tab to an _empty_ tabpane, two selectedIndexChanged events are
> called. I.e, when you add a TabPaneSelectionListener to a tabpane and call:
> tabpane.getTabs().add(component);
> .. the listener fires twice, with previousSelectedIndex set to -1 and then 0.
> The two events are fired by line 68 and 72 in TabPane.java:
> 68 tabPaneListeners.tabInserted(TabPane.this, index);
> 69
> 70 // Fire selection change event, if necessary
> 71 if (selectedIndex != previousSelectedIndex) {
> 72 tabPaneSelectionListeners.selectedIndexChanged(TabPane.this,
> selectedIndex);
> 73 }
> This could be fixed by taking into account that previousSelectedIndex might
> have been -1 when invoked, like this:
> if (selectedIndex != previousSelectedIndex && previousSelectedIndex > -1) {
> See this thread for more info:
> http://apache-pivot-users.399431.n3.nabble.com/TabPaneSelectionListener-selectedIndexChanged-called-twice-when-first-tab-is-inserted-td3022515.html
> PS: According to Chris, CardPane and Accordion behave in the same way, so any
> changes to TabPane should also be made to these for consistency.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
