ionic-team / ionic-framework

A powerful cross-platform UI toolkit for building native-quality iOS, Android, and Progressive Web Apps with HTML, CSS, and JavaScript.
https://ionicframework.com
MIT License
51k stars 13.51k forks source link

bug: ion-breadcrumb breaks when used on ngFor #23573

Closed p-gaspar closed 3 years ago

p-gaspar commented 3 years ago

Bug Report

Ionic version: [ ] 4.x [ ] 5.x [X] 6.x

Current behavior: Currently when dynamically using ion-breadcrumb with an *ngFor tag, the breadcrumbs do not show the separator and do not collapse (basically ignores the maxItems property). I have also tried wrapping each breadcrumb in an ng-container but it doesn't work either.

Expected behavior: It should normally by showing the separators and collapsing if it exceeds the maxItems property

Steps to reproduce: To reproduce, just use ion-breadcrumbs with an *ngFor on the ion-breadcrumb

Related code:

This does not work correctly (doesn't collapse or show separators):

<ion-breadcrumbs [maxItems]="2">
    <ion-breadcrumb *ngFor="let breadcrumb of path">
      {{ breadcrumb.title }}
    </ion-breadcrumb>
  </ion-breadcrumbs>

This does not work correctly on the ngFor items, but shows separators between "Breadcrumb 1", "Breadcrumb 2" and "Breadcrumb 3":

<ion-breadcrumbs [maxItems]="2">
    <ion-breadcrumb *ngFor="let breadcrumb of path">
      {{ breadcrumb.title }}
    </ion-breadcrumb>

    <ion-breadcrumb>
      Breadcrumb 1
    </ion-breadcrumb>

    <ion-breadcrumb>
      Breadcrumb 2
    </ion-breadcrumb>

    <ion-breadcrumb>
      Breadcrumb 3
    </ion-breadcrumb>
  </ion-breadcrumbs>

Ionic info:

Ionic:

   Ionic CLI                     : 6.12.4 (C:\Users\pedro\AppData\Roaming\npm\node_modules\@ionic\cli)
   Ionic Framework               : @ionic/angular 6.0.0-beta.1
   @angular-devkit/build-angular : 12.1.0
   @angular-devkit/schematics    : 11.0.7
   @angular/cli                  : 12.1.0
   @ionic/angular-toolkit        : 2.3.3

Capacitor:

   Capacitor CLI   : 2.4.6
   @capacitor/core : 2.4.6

Utility:

   cordova-res : 0.15.3
   native-run  : 1.3.0

System:

   NodeJS : v14.17.2 (C:\Program Files\nodejs\node.exe)
   npm    : 6.14.13
   OS     : Windows 10
brilyanlaia commented 3 years ago

hey, do you still have the same issue?

I think it's working fine for me.

image image

willmartian commented 3 years ago

Hi @p-gaspar, I am unable to reproduce this issue in the current beta release of v6, so I am going to go ahead and close this. Thanks!

p-gaspar commented 3 years ago

Hey @willmartindev , @brilyanlaia The issue still persists on my end. The example @brilyanlaia provided works but the problem lays when the data is dynamic, such as the example in #23694

Here's another example:

<!-- This works -->
<ion-breadcrumbs [maxItems]="2">
  <ion-breadcrumb *ngFor="let breadcrumb of [1, 2, 3]">
    Page {{ breadcrumb }}
  </ion-breadcrumb>
</ion-breadcrumbs>
<!-- This does not -->
<ion-breadcrumbs [maxItems]="2">
  <ion-breadcrumb *ngFor="let breadcrumb of getPath()">
    {{ breadcrumb.page }}
  </ion-breadcrumb>
</ion-breadcrumbs>

Here's my component that defines the example getPath() method:

getPath(): { id: number, page: string}[] {
  return [
    {
      id: 1,
      page: 'Page 1'
    },
    {
      id: 2,
      page: 'Page 2'
    },
    {
      id: 3,
      page: 'Page 3'
    },
  ];
}

This is the result: breadcrumb

Ionic Info:

Ionic:

   Ionic CLI                     : 6.12.4 (C:\Users\pedro\AppData\Roaming\npm\node_modules\@ionic\cli)
   Ionic Framework               : @ionic/angular 6.0.0-beta.3
   @angular-devkit/build-angular : 12.1.0
   @angular-devkit/schematics    : 11.0.7
   @angular/cli                  : 12.1.0
   @ionic/angular-toolkit        : 2.3.3

Capacitor:

   Capacitor CLI   : 2.4.6
   @capacitor/core : 2.4.6

Utility:

   cordova-res : 0.15.3
   native-run  : 1.3.0

System:

   NodeJS : v14.17.2 (C:\Program Files\nodejs\node.exe)
   npm    : 6.14.13
   OS     : Windows 10

I hope this example allows you to reproduce this on your end. Thanks!

p-gaspar commented 3 years ago

Hey, I found a workaround (on Angular) to this problem if you are working with dynamic data on the breadcrumbs (such as an ngFor, like the example I provided above).

In my case, I created a new boolean property in the component. Right before changing/updating the property bound to the *ngFor, set the boolean property to false. Then, right after updating, create a 1ms timeout that sets the boolean property to true on completion.

In the template, on the , add an *ngIf bound this boolean property. The idea is to remove the ion-breadcrumbs from the DOM before updating the data, and adding it immediately after updating the data.

Here's a snippet:

page.ts

public path: string[];
public showBreadcrumbs = false;

....

updatePath() {
    this.showBreadcrumbs = false;
    this.path = this.exampleService.getExamplePath(.....);
    setTimeout(() => this.showBreadcrumbs = true, 1);
}

page.html

<ion-breadcrumbs [maxItems]="2" *ngIf="showBreadcrumbs">
    <ion-breadcrumb (click)="...." *ngFor="let breadcrumb of path">
        {{ breadcrumb }}
    </ion-breadcrumb>
</ion-breadcrumbs>

This is not a perfect solution, however. Since we are adding and removing the element from the DOM, there could be some jittering due to the height changes. However, it's a fair tradeoff since without this workaround, the breadcrumbs break and do not collapse.

Hope this can help those with the same issue 🤞

ionitron-bot[bot] commented 3 years ago

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.