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

Add items to the NgbAccordion #4505

Closed
vladkasianenko opened this issue May 17, 2023 · 3 comments · May be fixed by MrDari/HeroTest#5
Closed

Add items to the NgbAccordion #4505

vladkasianenko opened this issue May 17, 2023 · 3 comments · May be fixed by MrDari/HeroTest#5

Comments

@vladkasianenko
Copy link

vladkasianenko commented May 17, 2023

  @ViewChild('accordion', { static: true }) accordion: NgbAccordionDirective;

  private _activeTab: string;

  @Input()
  set activeTab(value: string) {
    this._activeTab = value;

    if (!!this.accordion && !!value) {
      this.accordion.toggle(value);
    }
  }

  get activeTab(): string {
    return this._activeTab;
  }

Gives me this error (I changed the name of some properties):
image

However, in the NgbCarousel there's a property slides which is extremely useful.

    @ViewChild('carousel', { static: true }) carousel: NgbCarousel;

    private _activeTab: string;

    @Input()
    set activeTab(value: string) {
        this._activeTab = value;

        if (!!this.carousel?.slides && !!value) {
            this.carousel.select(`slide-${value}`);
        }
    }

    get activeTab(): string {
        return this._activeTab;
    }

As you can see, here I'm validating slides property: !!this.carousel?.slides.

Would it be possible to create something similar at the NgbAccordion: !!this.accordion?.items?

@maxokorokov
Copy link
Member

maxokorokov commented May 17, 2023

Hmm, slides is not a part of a public API, so you shouldn't really depend on it.

For the accordion, it looks more like a bug to me, because of the issue with this unsafe thing:

return this._items.find((item) => item.id === itemId);

this._items is undefined, but I can't reproduce it easily on my side. It seems you're setting [activeTab] too early, before the @ViewChildren in the accordion actually gets the list of items.

When are you doing it? Would you have a stackblitz?


  1. I think I'll first add checks to ensure all these never fail even though accordion is not finished initialising:
accordion.toggle('one');
accordion.collapse('one');
accordion.expand('one');
accordion.expandAll();
accordion.collapseAll();
accordion.isExpanded('one');
  1. Will think of exposing getItems(): NgbAccordionItem[], but the first one should fix your issue already

maxokorokov added a commit to maxokorokov/ng-bootstrap that referenced this issue May 17, 2023
Make sure to not fail when `QueryList` is `undefined`

ng-bootstrap#4505
maxokorokov added a commit that referenced this issue May 17, 2023
Make sure to not fail when `QueryList` is `undefined`

#4505
@vladkasianenko
Copy link
Author

vladkasianenko commented May 17, 2023

Yeah, I'm calling it in the @input() setter, so basically it's too early. This is why I'm depending on the slides - waiting for the slides to appear. And expecting to do that same with items on the Accrodion.

Currently I have workaround, but it's a dirty hack with tech debt:

   if (!!this.accordion && !!value) {
      // TODO:https://github.com/ng-bootstrap/ng-bootstrap/issues/4505
      try {
        if (!this.accordion.isExpanded(value)) {
          this.accordion.toggle(value);
        }
      }
      catch { }
    }

@maxokorokov
Copy link
Member

So if the .toggle()/.isExpanded() access is fixed, I don't think we'll expose access to items as a feature.

I'll close this for now, please reopen if necessary.

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

Successfully merging a pull request may close this issue.

2 participants