bootstrap icon indicating copy to clipboard operation
bootstrap copied to clipboard

uib-tab : deselect callback have wrong index with explicit Tab index set.

Open Rouche opened this issue 8 years ago • 3 comments

Bug description:

Edit: im unsure for the fix because it breaks some tests. Doc say: "The $selectedIndex can be used to determine which tab was attempted to be opened" If this $selectedIndex is supposed to be the one you assign in the template, then there really is a bug and the tests needs to be fixed too.

Lets say i have explicit tab index set on uib-tabs. I do have ng-if on some of them. Creating "holes" in indexes.

The deselect function sends the wrong index.

Link to minimally-working plunker that reproduces the issue:

None, pretty simple to understand looking at code.

Solution see second post.

Steps to reproduce the issue:

Theres even a test spec that assign index to 1, 3, 5, 7 deselect will then send 0, 1, 2, 3 as $selectedIndex wich is a bit counter intuitive.

<uib-tabset active="ctrl.index">
    <uib-tab index="1" heading="0" deselect="ctrl.selectTab($event, $selectedIndex)">
    </uib-tab>
    <uib-tab index="3" ng-if="false" heading="0" deselect="ctrl.selectTab($event, $selectedIndex)">
    </uib-tab>
    <uib-tab index="5" heading="0" deselect="ctrl.selectTab($event, $selectedIndex)">
    </uib-tab>
</uib-tabset>

Version of Angular, UIBS, and Bootstrap

Angular: 1.6.1

UIBS: 2.5.0

Bootstrap: 3.3.7

Rouche avatar Mar 27 '17 19:03 Rouche

Better fix so that $selectedIndex fits the one you set in template:

There is a workaround for this, but not perfect. So i guess since its a breaking change, i will have to cope with it,

It just does not feel right to receive a different index from the one that is set in the template. Fix:

        previousSelected.tab.onDeselect({
          $event: evt,
          $selectedIndex: typeof index === 'number' ? ctrl.tabs[index].index : undefined <==== Fix
        });

Some tests that verify on deselect needs to be adjusted too:

    it('should call deselect callback on deselect', function() {
      titles().eq(1).find('> a').click();
      expect(scope.deselectFirst).toHaveBeenCalled();
      expect(scope.deselectFirst.calls.argsFor(0)[0].target).toBe(titles().eq(1).find('> a')[0]);
      expect(scope.deselectFirst.calls.argsFor(0)[1]).toBe(2);
      titles().eq(0).find('> a').click();
      expect(scope.deselectSecond).toHaveBeenCalled();
      expect(scope.deselectSecond.calls.argsFor(0)[0].target).toBe(titles().eq(0).find('> a')[0]);
      expect(scope.deselectSecond.calls.argsFor(0)[1]).toBe(1);
      titles().eq(1).find('> a').click();
      expect(scope.deselectFirst.calls.count()).toBe(2);
      expect(scope.deselectFirst.calls.argsFor(1)[0].target).toBe(titles().eq(1).find('> a')[0]);
      expect(scope.deselectFirst.calls.argsFor(1)[1]).toBe(2);
    });

Rouche avatar Mar 28 '17 14:03 Rouche

Same problem

AndrewBoklashko avatar Oct 16 '17 18:10 AndrewBoklashko

Also encountering this.

Wes-Reid avatar Nov 01 '17 17:11 Wes-Reid