Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resetting components tabs no longer removes tab from UI - 1.1.16-11 #1419

Closed
slubowsky opened this issue Dec 30, 2016 · 16 comments
Closed

Resetting components tabs no longer removes tab from UI - 1.1.16-11 #1419

slubowsky opened this issue Dec 30, 2016 · 16 comments

Comments

@slubowsky
Copy link

Look like fix to #696 (thanks BTW) may have broken something.
Changing a components tabs no longer causes the old tabs to go away.

HTML

<tabset>
	<tab *ngFor="let tabz of tabs"
        ....

Component
private tabs: any[];
...
in ngOnInit
this.tabs = [...]

at some point in the future
this.tabs = [...]

Original tabs from onInit should go away and be replaced with new tabs. Instead original tabs are still on screen along with new ones.

(Actual use case is my workaround described in #699 where I veto tab selection by replacing the current tab state with the previous tab state. This seems to be a bug in any case but perhaps its time for me to revisit that, maybe something has changed since then)

@JuhaKoivisto
Copy link

The same problem here. Works with 1.1.16, but not with 1.1.16-11.

@slubowsky slubowsky changed the title Resetting components tabs no longer removes tab from UI Resetting components tabs no longer removes tab from UI - 1.1.16-11 Jan 3, 2017
@slubowsky
Copy link
Author

Ironically my workaround to allow me to grab new versions of ng2-bootstrap is now to add:

(TabDirective as any).prototype.ngOnDestroy = function () {
  this.tabset.removeTab(this);
};

@philjones88
Copy link
Contributor

similar issue with 1.1.16-11, when I remove any entry essentially from tabs the tab stays there.

@valorkin
Copy link
Member

valorkin commented Jan 13, 2017

@slubowsky I had to remove this because of another issue, with multiply events triggering on page destroy >.<

@slubowsky
Copy link
Author

@valorkin I know. I authored that issue :) but I have a workaround in place for that one already (see that issue where someone discusses something similar to what I did) so basically just reverting back to that state for now.

@slubowsky
Copy link
Author

Why not just have TabDirective.OnDestroy pass a flag that says not to event the removal or something simple like that?

@xwb1989
Copy link

xwb1989 commented Jan 13, 2017

@slubowsky 's suggestion, in my opinion, is the best way to solve it.

Here are my analysis and current workaround:

I think the root of the problem is there are two sets of 'tabs' being maintained, one is the 'tabs' provided/maintained by our client code(let's call it ClientTab), another is the set of TabDirective maintained by the library.

Analysis:

In order for the library to function properly, we have to guarantee that these two sets of 'tabs' are synced probably, which means:

  1. When a ClientTab is created, a corresponding TabDirective shall also be created.
  2. When a ClientTab is destroyed, its corresponding TabDirective shall also be destroyed.
  3. When a TabDirective is closed by a user action (clicking on the remove button), its corresponding ClientTab shall also be destroyed.

In the previous version, 1 and 2 are well maintained. The problem is with 3, as the removed (and the cascading select ) event fires when

  1. user clicks on the remove button
  2. or the TabDirective is destroyed when the whole containing view is being destroyed (for example, switching to another route).

and our client code is unable (at least not as I can perceive) to distinguish these two scenarios, thereafter, we cannot maintain our ClientTab states properly. For example, if:

  1. we have some tabs open and one active tab
  2. switch to another route
  3. then switch back, we want to see the same set of tabs and the same active tab.

This was broken because in the previous version, when switching to another route, removed and select events got fired inevitably and our ClientTab states got messed up.

By removing the ngOnDestroy in TabDirective, we are able to fix this problem. But, by doing so, we break the rule 2:

  1. When a ClientTab is destroyed, its corresponding TabDirective shall also be destroyed.

because when a TabDirective got destroyed with the ClientTab, it failed to remove itself from its TabsetComponent 's state and therefore got recreated later.

Current workaround:

When a ClientTab got removed, we remove its corresponding TabDirective from the TabsetComponent manually. Assuming:

  1. TabDirective and ClientTab have the one-to-one relationship and their positions are the same in the lists they reside.
  2. We can get a hold of the TabsetComponent by using the ViewChild API.

then

let idx = ...
clientTabsList.split(idx, 1);
tabset.removeTab(tabset.tabs[idx])

Notice this remove operation will fire the removed event and the possibly cascading select event as well, we need to handle them probably.

@JuhaKoivisto
Copy link

Thanks for the guidance, guys!

In Typescript I am doing this to get hold of Tabset component:

export class PriorityDataComponent implements OnInit {
@ViewChild(TabsetComponent)
private ts: TabsetComponent;

and this to replace old tabs with new ones:

       ..................

// Clear old tabs
while (this.ts.tabs[0] != null){
this.ts.removeTab(this.ts.tabs[0])
}

// Insert new tabs
this.tabs = [];

                newValue.forEach(p => {
                    let tab = new PriorityTab(p);
                    this.tabs.push(tab)});

                this.setActiveTab(0);

       ..................

Sorry I that I did not know how to format the code properly!

@pjlamb12
Copy link

I'm having this issue on 1.3.1 as well. Any signs of it being officially fixed?

@valorkin
Copy link
Member

Not yet

@tommueller
Copy link

tommueller commented Mar 13, 2017

What's the current state on this? I am also facing this issue right now!
Is there a workaround that I can put in place?

edit: I fixed this by accident. Since my data loads async I remove the tabset (using *ngIf) during loading, to show a progress bar. after loading finishs the tabset gets build newly and all the old tabs are gone!

<tabset *ngIf="!loading">
</tabset>

So just use a boolean on ngIf that you toggle when you are changing your data!

@pageii
Copy link

pageii commented Mar 17, 2017

While trying to hack TabDirective, I found another Bootstrap feature that could achieve similar results: Button>Radio. This alternate approach addresses the issue of old tabs not being removed after routing to a new component like so:

Import ButtonsModule as usual,

//Assuming you use Angular-cli
import { ButtonsModule } from 'ng2-bootstrap';

@NgModule({
  imports: [ButtonsModule.forRoot(),...]
})

Then simply use it like so:

<div class="btn-group">
  <label
      *ngFor="let item of collection"
      (click)="routeSomewhere(item.title)"
      class="btn btn-primary" 
      [(ngModel)]="radioModel"
      btnRadio="{{item.title}}">
      {{item.title}}
      </label>
</div>

We add a (click) event to fire off a method routeSomewhere that in turn does the routing. The radio buttons act like tabs because only one can be active. It has the extra benefit of showing the active state / selected button. Hope that helps.

@bongiova61
Copy link

Any news about this bug? I still have it in version 1.6.6

@Adondriel
Copy link

The fact this still exists, is stupid... I think i'll use some other framework.

@valorkin
Copy link
Member

valorkin commented Jun 2, 2017

fix published in v1.7+

@valorkin valorkin closed this as completed Jun 2, 2017
@valorkin
Copy link
Member

valorkin commented Jun 2, 2017

fixed by #1941

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests