Conversation
There was a problem hiding this comment.
Pull request overview
This PR upgrades the Angular application to Angular 20, modernizes templates to Angular’s built-in control flow (@if/@for), and replaces Protractor E2E with Cypress while adjusting build/TS configuration accordingly.
Changes:
- Upgrade Angular/framework dependencies and adjust build configuration (Angular 20,
moduleResolution: bundler, production budgets, critical CSS inlining). - Migrate multiple templates from
*ngIf/*ngForto@if/@forand update some Angular APIs/tests (TestBed.inject). - Remove Protractor-based E2E setup and introduce Cypress configuration + a starter E2E spec.
Reviewed changes
Copilot reviewed 65 out of 68 changed files in this pull request and generated 38 comments.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.json | Switches TS module resolution and adjusts compiler options. |
| src/app/static/tools/phenomizer/phenomizer.component.ts | Updates component metadata formatting/control-flow readiness. |
| src/app/static/tools/phenogramviz/phenogramviz.component.ts | Updates component metadata formatting/control-flow readiness. |
| src/app/static/tools/loinc/loinc.component.ts | Updates component metadata formatting/control-flow readiness. |
| src/app/static/tools/hpobrowser/hpobrowser.component.ts | Updates component metadata formatting/control-flow readiness. |
| src/app/static/tools/genomiser/genomiser.component.ts | Updates component metadata formatting/control-flow readiness. |
| src/app/static/tools/external/external.component.ts | Updates component metadata formatting/control-flow readiness. |
| src/app/static/tools/exomiser/exomiser.component.ts | Updates component metadata formatting/control-flow readiness. |
| src/app/static/resources/publications/publications.service.spec.ts | Updates TestBed API usage. |
| src/app/static/resources/publications/publications.component.ts | Updates component metadata and imports list. |
| src/app/static/resources/publications/publications.component.html | Migrates template control flow to @if/@for. |
| src/app/static/resources/license/license.component.ts | Updates component metadata formatting. |
| src/app/static/resources/funding/funding.component.ts | Updates component metadata formatting. |
| src/app/static/resources/faq/faq.component.ts | Updates component metadata formatting. |
| src/app/static/resources/disclaimer/disclaimer.component.ts | Updates component metadata formatting. |
| src/app/static/resources/contact/contact.component.ts | Updates component metadata/imports. |
| src/app/static/resources/contact/contact.component.html | Migrates lists to @for. |
| src/app/static/resources/citation/citation.component.ts | Updates component metadata formatting. |
| src/app/static/news/news.component.ts | Updates component metadata/imports. |
| src/app/static/news/news.component.html | Migrates template control flow to @if/@for. |
| src/app/static/home/home.component.ts | Updates component metadata/imports. |
| src/app/static/home/home.component.html | Migrates template control flow to @if/@for. |
| src/app/static/feedback/feedback.component.ts | Updates component metadata formatting. |
| src/app/static/data/translation/translation.component.ts | Updates component metadata formatting. |
| src/app/static/data/ontology-download/ontology-download.component.ts | Updates component metadata/imports. |
| src/app/static/data/layperson/layperson.component.ts | Updates component metadata formatting. |
| src/app/static/data/indigenous/indigenous.component.ts | Updates component metadata formatting. |
| src/app/static/data/api-doc/api-doc.component.ts | Updates component metadata/imports. |
| src/app/static/data/annotations-download/annotations-download.component.ts | Updates component metadata/imports. |
| src/app/static/data/annotations-download/annotations-download.component.html | Migrates template control flow to @if. |
| src/app/static/about/about.component.ts | Updates component metadata/imports. |
| src/app/shared/search/search/search.component.ts | Updates component metadata/imports and animations block formatting. |
| src/app/shared/search/search/search.component.html | Migrates search dropdown template to @if/@for. |
| src/app/shared/navbar/navbar.component.ts | Updates component metadata/imports and animations block formatting. |
| src/app/shared/navbar/navbar.component.html | Migrates template control flow to @if. |
| src/app/shared/footer/footer.component.ts | Updates component metadata/imports. |
| src/app/shared/floating-feedback/floating-feedback.component.ts | Updates component metadata/imports. |
| src/app/shared/dialog-excel-download/dialog.service.spec.ts | Updates TestBed API usage. |
| src/app/shared/dialog-excel-download/dialog-excel-download.component.ts | Updates component metadata/imports. |
| src/app/error/no-page-found.component.ts | Updates component metadata/imports and router navigation API usage. |
| src/app/error/no-page-found.component.html | Migrates template control flow to @if. |
| src/app/browser/pages/term/term.component.ts | Updates component metadata/imports list formatting. |
| src/app/browser/pages/term/term.component.html | Migrates multiple template sections to @if/@for. |
| src/app/browser/pages/search-results/search-results.component.ts | Updates component metadata/imports list. |
| src/app/browser/pages/search-results/search-results.component.html | Migrates template control flow to @if. |
| src/app/browser/pages/profile-search/profile-search.component.ts | Updates component metadata/imports list formatting. |
| src/app/browser/pages/profile-search/profile-search.component.html | Migrates template control flow to @if/@for. |
| src/app/browser/pages/gene/gene.component.ts | Updates component metadata and removes ProtVista stylesheet reference. |
| src/app/browser/pages/gene/gene.component.html | Migrates template control flow to @if/@for. |
| src/app/browser/pages/gene/gene.component.css | Removes ProtVista-related styles. |
| src/app/browser/pages/disease/disease.component.ts | Updates component metadata/imports list formatting. |
| src/app/browser/pages/disease/disease.component.html | Migrates template control flow to @if/@for. |
| src/app/app.component.ts | Updates root component metadata/imports for standalone bootstrap. |
| src/app/app.component.html | Migrates mobile nav template control flow to @if. |
| protractor.conf.js | Removes Protractor config. |
| package.json | Upgrades Angular deps, removes Protractor, adds Cypress scripts/deps. |
| e2e/tsconfig.e2e.json | Removes Protractor E2E TS config. |
| e2e/app.po.ts | Removes Protractor page object. |
| e2e/app.e2e-spec.ts | Removes Protractor E2E spec. |
| cypress/tsconfig.json | Adds Cypress TS config. |
| cypress/support/e2e.ts | Adds Cypress support entrypoint. |
| cypress/support/commands.ts | Adds Cypress commands scaffold. |
| cypress/e2e/app.cy.ts | Adds initial Cypress E2E spec. |
| cypress.config.ts | Adds Cypress configuration. |
| browserslist | Updates targeted browsers list. |
| angular.json | Adjusts production budgets, critical CSS inlining, schematics defaults. |
| .gitignore | Adds Cypress artifacts to ignore list. |
Comments suppressed due to low confidence (1)
src/app/error/no-page-found.component.html:22
- Template markup looks unbalanced after the
@ifmigration (extra/missing closing tags). The file currently ends without closing the outer container<div>, which will cause an Angular template parse error. Please fix the tag nesting/closing around the@if (errorFlag)block.
<a mat-raised-button href="https://github.com/TheJacksonLaboratory/hpo-web/issues" target="__blank"
class="mat-btn">Report Issue</a>
</div>
</div>
</div>
</div>
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| imports: [CommonModule, RouterModule, MatButtonModule], | ||
| templateUrl: './no-page-found.component.html', | ||
| styleUrls: ['./no-page-found.component.css'] | ||
| selector: 'app-no-page-found', |
There was a problem hiding this comment.
imports is specified in the component metadata but standalone: true was removed. In Angular, imports is only valid for standalone components; additionally this component is used directly in the router config, so it needs to be standalone in a bootstrapApplication setup. Add standalone: true back (or remove imports and declare/provide it via an NgModule).
| selector: 'app-no-page-found', | |
| selector: 'app-no-page-found', | |
| standalone: true, |
| @Component({ | ||
| selector: 'app-search-bar', | ||
| standalone: true, | ||
| imports: [ | ||
| CommonModule, | ||
| selector: 'app-search-bar', | ||
| imports: [ | ||
| FormsModule, | ||
| RouterModule, | ||
| MatSelectModule, |
There was a problem hiding this comment.
imports is used in the component decorator but standalone: true was removed. Angular only allows imports on standalone components, and this component is imported by other standalone components. Please restore standalone: true (or migrate away from component-level imports).
| @Component({ | ||
| selector: 'app-publications', | ||
| standalone: true, | ||
| imports: [ | ||
| CommonModule, | ||
| selector: 'app-publications', | ||
| imports: [ | ||
| RouterModule, | ||
| ReactiveFormsModule, |
There was a problem hiding this comment.
The component decorator includes an imports list but no longer declares standalone: true. This is invalid unless the component is standalone, and this component is routed-to directly. Please add standalone: true back (or refactor to NgModules).
| standalone: true, | ||
| imports: [ | ||
| CommonModule, | ||
| selector: 'app-search-results', |
There was a problem hiding this comment.
This component still declares an imports array but no longer sets standalone: true. That is invalid component metadata in Angular. Please restore standalone: true (or remove imports and declare it in an NgModule).
| selector: 'app-search-results', | |
| selector: 'app-search-results', | |
| standalone: true, |
| @Component({ | ||
| selector: 'app-ontology', | ||
| standalone: true, | ||
| imports: [MatIconModule], | ||
| templateUrl: './ontology-download.component.html', | ||
| styleUrls: ['./ontology-download.component.scss'] | ||
| selector: 'app-ontology', | ||
| imports: [MatIconModule], | ||
| templateUrl: './ontology-download.component.html', | ||
| styleUrls: ['./ontology-download.component.scss'] |
There was a problem hiding this comment.
The decorator includes imports but is missing standalone: true. This is invalid component metadata in Angular and will break compilation/routing. Restore standalone: true or remove imports and declare via an NgModule.
| ], | ||
| templateUrl: './disease.component.html', | ||
| styleUrls: ['./disease.component.css'] | ||
| selector: 'app-disease', |
There was a problem hiding this comment.
The component decorator still uses imports but no longer declares standalone: true. In Angular, imports is only valid for standalone components, and this page is routed-to directly. Restore standalone: true (or refactor to NgModules).
| selector: 'app-disease', | |
| selector: 'app-disease', | |
| standalone: true, |
| @Component({ | ||
| selector: 'app-about', | ||
| standalone: true, | ||
| imports: [RouterModule, MatButtonModule, MatIconModule], | ||
| templateUrl: './about.component.html', | ||
| styleUrls: ['./about.component.scss'] | ||
| selector: 'app-about', | ||
| imports: [RouterModule, MatButtonModule, MatIconModule], | ||
| templateUrl: './about.component.html', | ||
| styleUrls: ['./about.component.scss'] |
There was a problem hiding this comment.
imports is present in the component metadata but standalone: true was removed. In Angular this is invalid unless the component is standalone (and this component is routed to directly). Restore standalone: true or move these dependencies to an NgModule.
| @Component({ | ||
| selector: 'app-license', | ||
| standalone: true, | ||
| imports: [], | ||
| templateUrl: './license.component.html', | ||
| styleUrls: ['./license.component.css'] | ||
| selector: 'app-license', | ||
| imports: [], | ||
| templateUrl: './license.component.html', | ||
| styleUrls: ['./license.component.css'] |
There was a problem hiding this comment.
The component decorator still has imports but no longer sets standalone: true. This is invalid unless the component is standalone, and it is referenced directly in routes. Restore standalone: true.
| </div> | ||
| </div> | ||
|
|
||
| <div class="no-result" *ngIf="notFoundFlag"> | ||
| <h3>Sorry no results have been found!</h3> | ||
| <div class="flex flex-row flex-wrap align-items-center justify-center"> | ||
| @if (showHint && searchstate !== 'active') { | ||
| <p class="hint w-full"> |
There was a problem hiding this comment.
The new control-flow rewrite appears to have broken the HTML structure: the template closes search-output/wrapper divs before the hint block, leaving the hint outside and the file missing final closing tags. This will likely cause template parse errors or broken layout—please fix the tag nesting and ensure all opened <div>s are closed.
| @Component({ | ||
| selector: 'app-funding', | ||
| standalone: true, | ||
| imports: [], | ||
| templateUrl: './funding.component.html', | ||
| styleUrls: ['./funding.component.css'] | ||
| selector: 'app-funding', | ||
| imports: [], | ||
| templateUrl: './funding.component.html', | ||
| styleUrls: ['./funding.component.css'] |
There was a problem hiding this comment.
imports is configured but standalone: true was removed. In Angular, imports is only valid on standalone components, and this component is routed-to directly. Restore standalone: true.
No description provided.