taiga-family / taiga-ui

Angular UI Kit and components library for awesome people
https://taiga-ui.dev
Apache License 2.0
3.27k stars 457 forks source link

Dynamic tabs, routerLink-linkActive[BUG] #575

Closed mvn-h24 closed 3 years ago

mvn-h24 commented 3 years ago

🐞 Bug report

Description

Got troubles with performance, got a notice what a 'page has slowing down your browser', when render dynamic tabs with routerLink and routerLinkActive, this behavior not observe when render the buttons.

Reproduction

Example on stackblitz Need to reload page several time, to render the More button after that we can reproduce on stackblitz, click one dynamic link X#tab, next one, and when click some link from the more button, the browser send a notice what it`s 'page has slowing down your browser'. Again, this repoduced like it has been description here, but when i reproduce it by a similar way on local project , i got a notice after first click on dynamic X#tab.

Expected behavior

Without browser wrong notices

Versions

waterplea commented 3 years ago

@LowRideSky thank you for reporting this. Could you try it with the latest version 2.13.1? We recently fixed potential loop in recalculations for tabs.

mvn-h24 commented 3 years ago

@LowRideSky thank you for reporting this. Could you try it with the latest version 2.13.1? We recently fixed potential loop in recalculations for tabs.

I has been update app deps by stackblitz "Update all deps to latest", in result got compile erros? image and i has been updated dependecies (@taiga-ui/cdk, @taiga-ui/core, @taiga-ui/icons, @taiga-ui/kit)in local project, remove and install, and bug still repuduce when click on link from exactly from "More" button, but now with usual dynamic X#tab works well Local deps list from, package.json:

"dependencies": {
    "@angular/animations": "^12.0.0",
    "@angular/common": "^12.0.0",
    "@angular/compiler": "^12.0.0",
    "@angular/core": "^12.0.0",
    "@angular/forms": "^12.0.0",
    "@angular/localize": "^12.1.2",
    "@angular/platform-browser": "^12.0.0",
    "@angular/platform-browser-dynamic": "^12.0.0",
    "@angular/router": "^12.0.0",
    "@nestjs/common": "^7.0.0",
    "@nestjs/config": "^1.0.0",
    "@nestjs/core": "^7.0.0",
    "@nestjs/jwt": "^8.0.0",
    "@nestjs/passport": "^8.0.0",
    "@nestjs/platform-express": "^7.0.0",
    "@ngrx/effects": "^12.2.0",
    "@ngrx/router-store": "^12.2.0",
    "@ngrx/store": "^12.2.0",
    "@nrwl/angular": "12.5.7",
    "@taiga-ui/cdk": "^2.13.1",
    "@taiga-ui/core": "^2.13.1",
    "@taiga-ui/icons": "^2.13.1",
    "@taiga-ui/kit": "^2.13.1",
    "@typegoose/typegoose": "^7.6.3",
    "bcryptjs": "^2.4.3",
    "class-transformer": "^0.4.0",
    "class-validator": "^0.13.1",
    "lodash": "^4.17.21",
    "mongoose": "^5.10.0",
    "nestjs-typegoose": "^7.1.38",
    "passport": "^0.4.1",
    "passport-jwt": "^4.0.0",
    "reflect-metadata": "^0.1.13",
    "rxjs": "~6.6.0",
    "tslib": "^2.0.0",
    "zone.js": "~0.11.4"
  },
  "devDependencies": {
    "@angular-devkit/build-angular": "^12.0.0",
    "@angular-eslint/eslint-plugin": "~12.0.0",
    "@angular-eslint/eslint-plugin-template": "~12.0.0",
    "@angular-eslint/template-parser": "~12.0.0",
    "@angular/cli": "^12.0.0",
    "@angular/compiler-cli": "^12.0.0",
    "@angular/language-service": "^12.0.0",
    "@nestjs/schematics": "^7.0.0",
    "@nestjs/testing": "^7.0.0",
    "@ngrx/store-devtools": "^12.2.0",
    "@nrwl/cli": "12.5.7",
    "@nrwl/cypress": "12.5.7",
    "@nrwl/eslint-plugin-nx": "12.5.7",
    "@nrwl/jest": "12.5.7",
    "@nrwl/linter": "12.5.7",
    "@nrwl/nest": "12.5.7",
    "@nrwl/node": "12.5.7",
    "@nrwl/tao": "12.5.7",
    "@nrwl/workspace": "12.5.7",
    "@types/bcryptjs": "^2.4.2",
    "@types/express": "^4.17.13",
    "@types/faker": "^5.5.7",
    "@types/jest": "26.0.8",
    "@types/lodash": "^4.14.171",
    "@types/mongoose": "^5.10.0",
    "@types/node": "14.14.33",
    "@types/passport-jwt": "^3.0.6",
    "@typescript-eslint/eslint-plugin": "4.19.0",
    "@typescript-eslint/parser": "4.19.0",
    "cypress": "^7.3.0",
    "dotenv": "8.2.0",
    "eslint": "7.22.0",
    "eslint-config-prettier": "8.1.0",
    "eslint-plugin-cypress": "^2.10.3",
    "faker": "^5.5.3",
    "jest": "27.0.3",
    "jest-preset-angular": "9.0.3",
    "prettier": "^2.3.1",
    "ts-jest": "27.0.3",
    "ts-node": "~9.1.1",
    "typescript": "~4.2.4"
  }
mvn-h24 commented 3 years ago

Looking like i has been something wrong with dependecies update with stackblitz in previous example. So now has been maked new fork from taiga-starter and updated deps and now its compiled. And bug still reproduced, with "More" button. New example

mvn-h24 commented 3 years ago

Added async variant for tabs component, and "exact" option for first tab-link whose forward on root page and now all working well, on stakcblitz.

mvn-h24 commented 3 years ago

@LowRideSky thank you for reporting this. Could you try it with the latest version 2.13.1? We recently fixed potential loop in recalculations for tabs.

Don`t know what happened with my local project, but now tabs works well. May be it was browser cache? I just added 'exact' option for every link-tab in local project. Think, the main trouble in my project in doubled data for some tabs in template, found it problem recently now. Think, main problem of this issue got a resolved, thanks.


May be next report, when select "tab which has a doubled brother" from "More" button, the "More" button not opens when click on it.

mvn-h24 commented 3 years ago

I got a try to reproduce the doubled tabs and now page got critical loading again, stackblitz example

waterplea commented 3 years ago

Thank you, I'm able to reproduce it. Will try to fix it for the next release

waterplea commented 3 years ago

Ok, so the problem here is active tab from "More" is always put to the visible list. Since you have multiple same tabs they start to fight for that spot in an infinite loop. I wonder what's the best way to handle this, as it is obviously an incorrect configuration but it shouldn't lock the tab but instead it should somehow warn you 🤔

waterplea commented 3 years ago

I'm afraid I cannot come up with a good enough solution without adding unpleasant extra code just for the warning. I think it should be clear one shouldn't add multiple of the same tabs and expect the component to sort out which one to consider active. Plus this is only an issue when one same tab is visible and other is hidden in the "More" section. So I'm afraid I'm gonna leave it as is.