Refactor: Remove jQuery DOM manipulation (Part 1) - Form resets, dead code, and cross-component toggles#76
Conversation
… code, and cross-component toggles
📝 WalkthroughWalkthroughSystematic migration across 21 components from jQuery DOM manipulation to Angular form APIs and state management. Removes global jQuery declarations, replaces jQuery form resets with ChangesjQuery to Angular form and state refactoring
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/app/multi-role-screen/multi-role-screen.component.ts (1)
134-138: ⚡ Quick winDecouple visibility logic from display text literal.
This condition depends on
"Role Selection"text, which is fragile if header text changes/localizes. Prefer a stable status key/enum fromsendHeaderStatusand deriveshowDbLabelfrom that key.♻️ Suggested adjustment
- if (data === "Role Selection") { - this.showDbLabel = false; - } else { - this.showDbLabel = true; - } + // Prefer key-based contract from emitter, e.g. `data.headerKey` + this.showDbLabel = data !== "Role Selection";🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/multi-role-screen/multi-role-screen.component.ts` around lines 134 - 138, The visibility check for showDbLabel relies on the display string "Role Selection" which is fragile; update the component to use a stable status key/enum from sendHeaderStatus instead: change the sendHeaderStatus payload to include a status identifier (e.g., HeaderStatus.RoleSelection) and in multi-role-screen.component.ts replace the string comparison with a check against that identifier (e.g., compare data.status or the enum value), then set this.showDbLabel based on the stable status key rather than the localized header text.src/app/supervisor-upload-schemes/supervisor-uploadSchemes.component.ts (1)
246-246: ⚡ Quick winConsider adding null safety check before resetForm().
While the
ViewChildshould be initialized when these methods are called, adding a null safety check would prevent runtime errors if the form reference is missing or the ViewChild query fails.🛡️ Suggested defensive check
successHandler(response) { this.schemeData = response; this.notUploaded = false; - this.schemeForm.resetForm(); + if (this.schemeForm) { + this.schemeForm.resetForm(); + } var obj = {createScheme() { this.create = true; this.showTable = false; - this.schemeForm.resetForm(); + if (this.schemeForm) { + this.schemeForm.resetForm(); + } }Also applies to: 278-278
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/supervisor-upload-schemes/supervisor-uploadSchemes.component.ts` at line 246, Add a null/undefined guard before calling schemeForm.resetForm() to avoid runtime errors if the ViewChild wasn't initialized: check that the ViewChild property (schemeForm) is truthy (e.g., if (this.schemeForm) ...) before invoking resetForm(); apply the same defensive check at the other occurrence (around line with the second resetForm call) so both calls safely handle a missing form reference.src/app/sio-blood-on-call-service/sio-blood-on-call-service.component.ts (1)
618-619: ⚡ Quick winDefensive null-checks on
resetForm()calls are good practice but the stated concern about step-based conditional existence is not supported.Both
patientDetailsFormandhospitalDetailFormare always rendered together within*ngIf="showBloodRequestForm"in the template. They do not conditionally exist at different times—when one is present in the DOM, so is the other. The forms' content is controlled via[hidden]flags, not conditional rendering, so both remain available to ViewChild when the parent container is rendered.That said, adding null-checks before
resetForm()remains valid defensive programming:+ if (this.hospitalDetailForm) { + this.hospitalDetailForm.resetForm(); + } + if (this.patientDetailsForm) { + this.patientDetailsForm.resetForm(); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/sio-blood-on-call-service/sio-blood-on-call-service.component.ts` around lines 618 - 619, Add defensive null-checks before calling resetForm on the form ViewChilds: ensure this.hospitalDetailForm and this.patientDetailsForm are truthy before invoking their resetForm() methods (e.g., guard each call with if (this.hospitalDetailForm) and if (this.patientDetailsForm)). This keeps existing behavior (they are typically rendered together under showBloodRequestForm) but prevents runtime errors if either ViewChild is unexpectedly undefined during component lifecycle.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/app/beneficiary-registration-104/beneficiary-registration-104.component.ts`:
- Around line 1613-1616: Reorder and guard the form reset: call
registrationForm.resetForm() first (with a null/undefined check on
registrationForm) before setting showForm = false, then set personIsSelf =
false; this ensures the template-controlled form exists when reset is invoked
and mirrors the correct pattern used elsewhere (use the existing
registrationForm reference and the showForm and personIsSelf flags).
In
`@src/app/sio-epidemic-outbreak-service/sio-epidemic-outbreak-service.component.ts`:
- Around line 382-385: The code sets this.showTable = true before calling
this.epidemicForm.resetForm(), which causes the ViewChild epidemicForm to be
removed by *ngIf and become undefined; move the reset call to run before
toggling visibility (i.e., call this.epidemicForm.resetForm() prior to setting
this.showTable = true) and add a defensive null/undefined check on epidemicForm
before calling resetForm() to avoid a TypeError (referencing epidemicForm and
showTable in sio-epidemic-outbreak-service.component.ts).
---
Nitpick comments:
In `@src/app/multi-role-screen/multi-role-screen.component.ts`:
- Around line 134-138: The visibility check for showDbLabel relies on the
display string "Role Selection" which is fragile; update the component to use a
stable status key/enum from sendHeaderStatus instead: change the
sendHeaderStatus payload to include a status identifier (e.g.,
HeaderStatus.RoleSelection) and in multi-role-screen.component.ts replace the
string comparison with a check against that identifier (e.g., compare
data.status or the enum value), then set this.showDbLabel based on the stable
status key rather than the localized header text.
In `@src/app/sio-blood-on-call-service/sio-blood-on-call-service.component.ts`:
- Around line 618-619: Add defensive null-checks before calling resetForm on the
form ViewChilds: ensure this.hospitalDetailForm and this.patientDetailsForm are
truthy before invoking their resetForm() methods (e.g., guard each call with if
(this.hospitalDetailForm) and if (this.patientDetailsForm)). This keeps existing
behavior (they are typically rendered together under showBloodRequestForm) but
prevents runtime errors if either ViewChild is unexpectedly undefined during
component lifecycle.
In `@src/app/supervisor-upload-schemes/supervisor-uploadSchemes.component.ts`:
- Line 246: Add a null/undefined guard before calling schemeForm.resetForm() to
avoid runtime errors if the ViewChild wasn't initialized: check that the
ViewChild property (schemeForm) is truthy (e.g., if (this.schemeForm) ...)
before invoking resetForm(); apply the same defensive check at the other
occurrence (around line with the second resetForm call) so both calls safely
handle a missing form reference.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9f3eabbe-b4f9-4ee0-822b-7b03af2df8a1
📒 Files selected for processing (20)
src/app/104/104.component.tssrc/app/beneficiary-registration-104/beneficiary-registration-104.component.tssrc/app/bp-screening-component/bp-screening-component.tssrc/app/case-sheet/case-sheet.component.tssrc/app/covid-19/covid-19.component.tssrc/app/diabetic-screening-component/diabetic-screening-component.tssrc/app/directory-services/directory-services.component.tssrc/app/multi-role-screen/multi-role-screen.component.htmlsrc/app/multi-role-screen/multi-role-screen.component.tssrc/app/outbound-search-records/outbound-search-records.component.tssrc/app/prescription/prescription.component.tssrc/app/service-role-selection/service-role-selection.component.tssrc/app/sio-blood-on-call-service/sio-blood-on-call-service.component.tssrc/app/sio-epidemic-outbreak-service/sio-epidemic-outbreak-service.component.tssrc/app/sio-food-safety-service/sio-food-safety-service.component.tssrc/app/sio-grievience-service/sio-grievience-service.component.tssrc/app/sio-organ-donation-service/sio-organ-donation-service.component.tssrc/app/sio-outbound-provider/sio-outbound-provider.component.tssrc/app/sio-services/sio-services.component.tssrc/app/supervisor-upload-schemes/supervisor-uploadSchemes.component.ts
💤 Files with no reviewable changes (10)
- src/app/prescription/prescription.component.ts
- src/app/service-role-selection/service-role-selection.component.ts
- src/app/sio-services/sio-services.component.ts
- src/app/directory-services/directory-services.component.ts
- src/app/outbound-search-records/outbound-search-records.component.ts
- src/app/sio-organ-donation-service/sio-organ-donation-service.component.ts
- src/app/104/104.component.ts
- src/app/diabetic-screening-component/diabetic-screening-component.ts
- src/app/bp-screening-component/bp-screening-component.ts
- src/app/sio-food-safety-service/sio-food-safety-service.component.ts
| this.showForm = false; | ||
| //this.showTableFlag = true; | ||
| jQuery("#registerForm").trigger("reset"); | ||
| this.registrationForm.resetForm(); | ||
| this.personIsSelf = false; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify template conditional rendering and reset ordering.
rg -n -C3 'registrationForm|showForm|\*ngIf' \
src/app/beneficiary-registration-104/beneficiary-registration-104.component.ts \
src/app/beneficiary-registration-104/beneficiary-registration-104.component.htmlRepository: PSMRI/Helpline104-UI
Length of output: 39041
Reorder form reset before hiding and add null-check for safety.
Line 1613–1615 hides the form via showForm = false before calling registrationForm.resetForm(). Since the form is guarded by *ngIf="showForm" in the template, this pattern is backwards and semantically incorrect. Additionally, if showForm is already false when this code path executes, registrationForm will be undefined.
The codebase already demonstrates the correct pattern elsewhere (line 1685): reset the form first, then hide it. Add a null-check for defensive safety.
Suggested fix
- this.showForm = false;
- //this.showTableFlag = true;
- this.registrationForm.resetForm();
+ if (this.registrationForm) {
+ this.registrationForm.resetForm();
+ }
+ this.showForm = false;
+ //this.showTableFlag = true;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| this.showForm = false; | |
| //this.showTableFlag = true; | |
| jQuery("#registerForm").trigger("reset"); | |
| this.registrationForm.resetForm(); | |
| this.personIsSelf = false; | |
| if (this.registrationForm) { | |
| this.registrationForm.resetForm(); | |
| } | |
| this.showForm = false; | |
| //this.showTableFlag = true; | |
| this.personIsSelf = false; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/app/beneficiary-registration-104/beneficiary-registration-104.component.ts`
around lines 1613 - 1616, Reorder and guard the form reset: call
registrationForm.resetForm() first (with a null/undefined check on
registrationForm) before setting showForm = false, then set personIsSelf =
false; this ensures the template-controlled form exists when reset is invoked
and mirrors the correct pattern used elsewhere (use the existing
registrationForm reference and the showForm and personIsSelf flags).
| this.showTable = true; | ||
| // this.showEpidemicForm = false; | ||
| jQuery("#epidemic_form").trigger("reset"); | ||
| this.epidemicForm.resetForm(); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether epidemic form is conditionally rendered with showTable.
rg -n -C3 'epidemic_Details_Form|epidemicForm|showTable|\*ngIf' \
src/app/sio-epidemic-outbreak-service/sio-epidemic-outbreak-service.component.ts \
src/app/sio-epidemic-outbreak-service/sio-epidemic-outbreak-service.component.htmlRepository: PSMRI/Helpline104-UI
Length of output: 16934
🏁 Script executed:
rg -n 'epidemicForm' src/app/sio-epidemic-outbreak-service/sio-epidemic-outbreak-service.component.tsRepository: PSMRI/Helpline104-UI
Length of output: 170
Reset form before hiding it to prevent undefined ViewChild reference.
The form template is conditionally rendered with *ngIf="!showTable" (line 1 of HTML). When this.showTable = true executes, Angular's change detection removes the form element from the DOM, making the epidemicForm ViewChild undefined. The subsequent resetForm() call on line 384 will throw a TypeError.
Reorder to reset the form before changing visibility:
Suggested fix
+ if (this.epidemicForm) {
+ this.epidemicForm.resetForm();
+ }
this.showTable = true;
// this.showEpidemicForm = false;
- this.epidemicForm.resetForm();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| this.showTable = true; | |
| // this.showEpidemicForm = false; | |
| jQuery("#epidemic_form").trigger("reset"); | |
| this.epidemicForm.resetForm(); | |
| } | |
| if (this.epidemicForm) { | |
| this.epidemicForm.resetForm(); | |
| } | |
| this.showTable = true; | |
| // this.showEpidemicForm = false; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/app/sio-epidemic-outbreak-service/sio-epidemic-outbreak-service.component.ts`
around lines 382 - 385, The code sets this.showTable = true before calling
this.epidemicForm.resetForm(), which causes the ViewChild epidemicForm to be
removed by *ngIf and become undefined; move the reset call to run before
toggling visibility (i.e., call this.epidemicForm.resetForm() prior to setting
this.showTable = true) and add a defensive null/undefined check on epidemicForm
before calling resetForm() to avoid a TypeError (referencing epidemicForm and
showTable in sio-epidemic-outbreak-service.component.ts).
1 participant
📋 Description
JIRA ID: #129
This PR represents the first major phase of eliminating legacy jQuery DOM manipulation across the Helpline104-UI application to reduce technical debt and safely prepare the codebase for the Angular 19 migration.
Key Changes:
trigger("reset")calls with Angular's nativeresetForm()API using@ViewChildtemplate references (e.g., withinsio-blood-on-call-service.component.tsandcase-sheet.component.ts)..css()styling calls (e.g., targeting#L1,#L2IDs) that were attempting to mutate elements no longer present in the DOM across thesio-organ-donation-serviceandsio-blood-on-call-servicecomponents.service-role-selectionwas triggeringjQuery("#db_label").show/hide()on an element living inmulti-role-screen. This has been refactored to use idiomatic Angular reactive state management (showDbLabelboolean property) governed by thesendHeaderStatusRxJS subscription and an*ngIfstructural directive.✅ Type of Change
ℹ️ Additional Information
jQuery()selectors to their respective native Angular properties and verifying that component logic (boolean flags controlling structural directives) remains functionally equivalent.@angular/corenode_modules installation error locally, static code analysis was relied upon to verify RxJS subscriptions and ViewChild resets.Summary by CodeRabbit