Skip to main content

Search and Top Navigation

#4578 closed bug (fixed)

Opened June 03, 2009 10:29PM UTC

Closed May 16, 2011 04:31PM UTC

Last modified January 23, 2017 12:48PM UTC

adding tab moves targeted panel

Reported by: gpbmike Owned by:
Priority: minor Milestone: 1.9.0
Component: ui.tabs Version: 1.7.1
Keywords: Cc:
Blocked by: Blocking:
Description

using the 'add' method will move the targeted panel after the tabs ul.

creating tabs doesn't move any targeted panels, so i wouldn't expect adding a tab to move a panel either.

Attachments (0)
Change History (9)

Changed June 09, 2009 08:12AM UTC by jzaefferer comment:1

milestone: TBD1.8

Changed June 12, 2009 05:51AM UTC by klaus.hartl comment:2

Could you explain that a little more? Do you refer to the situation where you add a tab that targets a panel (e.g. div) that exists in the DOM already?

Changed January 07, 2010 06:16PM UTC by sigmasquirrel comment:3

I'd like to patch this bug but I'm unsure of what the correct behavior should be.

The issue occurs when using a markup structure that doesn't match the default. If you add a new tab to the end of the list (which is the default behavior if you don't specify an index) specifying the id of a content div that is already in the DOM, it will be inserted after the UL containing the tabs. If you do the same, but specify an index that's not the end of the list, it will be inserted after an existing content element, which may or may not match the behavior of the first case.

Example:

http://bryanrsmith.com/tabs.html

Note that when you insert a new tab anywhere other than the end of the list, it's added to #content-container, however, if you insert a new tab at the end of the list, it's appended to the ul's parent, which is not the correct location.

The offending code (from line 468 of jquery.ui.tabs.js)

// try to find an existing element before creating a new one
var $panel = $('#' + id);
if (!$panel.length) {
	$panel = $(o.panelTemplate).attr('id', id).data('destroy.tabs', true);
}
$panel.addClass('ui-tabs-panel ui-widget-content ui-corner-bottom ui-tabs-hide');

if (index >= this.lis.length) {
	$li.appendTo(this.list);
	$panel.appendTo(this.list[0].parentNode); // Bad: maybe this.list[0].parentNode is not where we're keeping our content elements
}
else {
	$li.insertBefore(this.lis[index]);
	$panel.insertBefore(this.panels[index]); // Correct, as long as there is at least 1 existing tab
}

It seems to me that the correct behavior is to not try to re-insert the content div if it already exists in the DOM-- just leave it where it is. An alternative may be to add a parameter for a DOM element for the content div to be appended to. This would also allow the plugin to work consistently when adding a remote tab to an instance of tabs that currently has 0 tabs, in which case we have no way to know where the new tab should be inserted.

This is the only place in the tabs plugin I've found that causes problems when using a non-standard markup structure. Any thoughts on how to handle it?

Changed January 13, 2010 11:40PM UTC by scottgonzalez comment:4

Changed February 16, 2011 12:27PM UTC by tfotherby comment:5

I got bitten by this bug. It was very useful to know there was a simple workaround which is to supply a value for the

index
parameter.

i.e.

Broken (due to missing

index
parameter):

$('#tabs').tabs( "add" , '#portfolio-image-tab' , 'Portfolio Image');

Working (due to supplied

index
parameter):

$('#tabs').tabs( "add" , '#portfolio-image-tab' , 'Portfolio Image', 2);

Changed March 28, 2011 07:43PM UTC by gnarf comment:6

status: newopen

Taking a peek into this source makes me wonder if the

 $panel.appendTo() 
call in that if couldn't use
 this.element 
and see this bug disappear.. What is the "parentNode" functionality in there right now doing, basically selecting
 this.element 
right??

Changed May 11, 2011 06:04PM UTC by davidmurdoch comment:7

_comment0: Pull request for this fix is here: https://github.com/jquery/jquery-ui/pull/2511305327404915916

Pull request for this fix is here: https://github.com/jquery/jquery-ui/pull/251 (closed)

Changed May 13, 2011 10:57PM UTC by davidmurdoch comment:8

new pull request for this fix here: https://github.com/jquery/jquery-ui/pull/274

Changed May 16, 2011 04:31PM UTC by David Murdoch comment:9

resolution: → fixed
status: openclosed

Tabs: When adding a new tab with an existing panel, don't move it. Fixes #4578 - adding tab moves targeted panel.

Changeset: 965cb7359ea704715839e3c171ae751d6a32dde9