#10727 closed defect (fixed)
ScrollingTabContainer: tab click calls selectChild twice
Reported by: | Adam Peller | Owned by: | bill |
---|---|---|---|
Priority: | high | Milestone: | 1.4.2 |
Component: | Dijit | Version: | 1.4.0 |
Keywords: | Cc: | ||
Blocked By: | Blocking: |
Description (last modified by )
In test_TabContainer, click on any tab to select a new tab. If you put a log in selectChild, you'll see it gets called twice. While it should not be catastrophic, some apps may not be expecting this condition and it can cause regressions. Thanks to Phil Berkland @ IBM for spotting this.
StackController.onButtonClick calls container.selectChild. That publishes an event which triggers onButtonClick a second time via ScrollingTabController.onSelectChild:280
tab.onClick(null);
Change History (7)
comment:1 Changed 11 years ago by
Description: | modified (diff) |
---|---|
Owner: | set to Shane O'Sullivan |
Summary: | ScrollingTabContainer tab click calls selectChild twice → ScrollingTabContainer: tab click calls selectChild twice |
comment:2 follow-up: 4 Changed 11 years ago by
Did you test where one of the tabs was not currently visible, and had to be scrolled to? The reason for that approach is to not show the change in the tab until it is visible.
However, I can see that this is a problem. If it works just fine with those lines commented out, then it should be ok to make that change.
comment:3 Changed 11 years ago by
Milestone: | tbd → 1.4.2 |
---|
put this in for consideration for 1.4.2, since we've got a couple of other Tab-related things in there already
comment:4 Changed 11 years ago by
Owner: | changed from Shane O'Sullivan to bill |
---|---|
Status: | new → assigned |
Replying to shaneosullivan:
Did you test where one of the tabs was not currently visible, and had to be scrolled to? The reason for that approach is to not show the change in the tab until it is visible.
However, I can see that this is a problem. If it works just fine with those lines commented out, then it should be ok to make that change.
Yes, I tested that, selecting "tab 3" manually and then doing dijit.byId("mainTabContainer").selectChild(dijit.byId("embedded"))
but it's hard to see what's happening because it moves so fast.
I see what you are saying about delaying the CSS change until the tab is visible, but actually that isn't working currently because StackController.onSelectChild() changes the CSS of the two tabs instantly, before the animation even starts.
So, I'll remove the onClick calls.
comment:5 Changed 11 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
comment:6 Changed 11 years ago by
(In [21395]) Test file updates to match template changes for ScrollingTabControllerButton widget. Refs #10527.
Also, selecting a pane from the menu no longer focuses on the tab label. I think this is OK since keyboard users will never use the menu anyway, but I needed to remove menu --> tab label focus test. Refs #10727.
Shane, do we need the tab.onClick() calls in ScrollingTabController.onSelectChild()? They don't seem appropriate.
I tried commenting them out and then running:
from firebug, it seems to work.
Obviously the checked (aka selected) state of the TabButton's needs to change, so that the selected tab is white, but that's handled in StackController.onSelectChild().