chore: migrate app to Angular 8#3
chore: migrate app to Angular 8#3christophechevalier wants to merge 1 commit intomaxime1992:masterfrom
Conversation
christophechevalier
commented
Jun 18, 2019
christophechevalier
commented
- upgrade all dependencies
- update typescript files to use new rxjs 6 syntax
- update and reorganize all imports modules
- update the names of the classes used in all templates
- remove some scripts in package.json like prettier temporarly (need fix)
- fix: auto focus input with using custom renderer to bypass Angular's templating and make custom UI changes
- cleaning some parts of the code
|
Hey @maxime1992 ! |
maxime1992
left a comment
maxime1992
left a comment
There was a problem hiding this comment.
Hey, thanks for the PR that's great :) !
A few things to modify though but nothing really difficult
package.json
Outdated
| "description": "Angular app to help you generate optimized teams for a sport (ex football).", | ||
| "keywords": [ | ||
| "angular", | ||
| "angular 8", |
package.json
Outdated
| "keywords": [ | ||
| "angular", | ||
| "angular 8", | ||
| "ngx-webstorage", |
There was a problem hiding this comment.
I don't think that ngx-webstorage is relevant in that case
| "dist": "ng build", | ||
| "prod": "ng build --prod", | ||
| "prod:src": "ng build --prod --sourcemaps" | ||
| }, |
There was a problem hiding this comment.
I'm fine removing lint-staged and the pre-commit hook.
But why have you removed prettier? The linting commands too?
There was a problem hiding this comment.
@maxime1992 Updated but as I say, team.component.scss and need-10-players.component.ts seems cause problem with Prettier. And I can't push without validation.
There was a problem hiding this comment.
Do you mean that you haven't been able to push those files or that it will fail on CI?
Push them using --no-verify (from memory) and I'll take a look
There was a problem hiding this comment.
I pushed with --no-verify.
There was a problem hiding this comment.
Not really and to be honest I don't have much time for this project.
I think you should just work on your own fork =) !
package.json
Outdated
| "start": "ng serve", | ||
| "test": "ng test", | ||
| "lint": "ng lint", | ||
| "pree2e": "webdriver-manager update --standalone false --gecko false", |
| "material-design-icons-iconfont": "5.0.1", | ||
| "rxjs": "6.5.0", | ||
| "zone.js": "0.9.1" | ||
| }, |
There was a problem hiding this comment.
ngx-webstorage is a dependency, not a dev one
| .pipe( | ||
| takeUntil(this.onDestroy$), | ||
| tap((players: IPlayer[]) => this.updateFormControl(players)), | ||
| tap((players: IPlayer[]) => { |
| .pipe( | ||
| takeUntil(this.onDestroy$), | ||
| tap((players: IPlayer[]) => this.updateFormControl(players)), | ||
| tap((players: IPlayer[]) => { |
There was a problem hiding this comment.
Merge the 3 taps in one please
src/app/players/players.component.ts
Outdated
| .do(grade => | ||
| this.playersService.changeUserGrade(player.name, +grade) | ||
| ); | ||
| filter(grade => grade), |
There was a problem hiding this comment.
Could you write that one as
filter(grade => !!grade)
that'll be clearer
There was a problem hiding this comment.
Sure. I can do that.
src/app/players/players.component.ts
Outdated
| this.isNewPlayerFormGroupVisible = true; | ||
| setTimeout(() => this.nameNewPlayer.focus(), 0); | ||
| setTimeout( | ||
| () => this.renderer.selectRootElement('#nameNewPlayer').focus(), |
There was a problem hiding this comment.
I'm really not convinced that's needed :think:
src/app/players/players.component.ts
Outdated
| } | ||
|
|
||
| trackByName(index, player: IPlayer) { | ||
| trackByName(player: IPlayer) { |
There was a problem hiding this comment.
When using trackBy, the first argument is the index so that's a regression
- upgrade all dependencies - update typescript files to use new rxjs 6 syntax - update and reorganize all imports modules - update the names of the classes used in all templates - remove some scripts in package.json like prettier temporarly (need fix) - fix: auto focus input with using custom renderer to bypass Angular's templating and make custom UI changes - cleaning some parts of the code
christophechevalier
force-pushed
the
upgrade-to-angular8
branch
from
40d4793 to
22b5c1b
Compare
2 participants
