[ES-2857] [ES-2952] [#1812] [#1803] UI validation and other fixes.#153
Merged
Conversation
… support for deselect option from dropdown. Signed-off-by: GurukiranP <talk2gurukiran@gmail.com>
WalkthroughBumps package version; centralizes file-required validation in validateFileField(), makes dropdowns conditionally required when related uploads exist, propagates language context to regex validators, triggers events for all fields on language change, and adjusts minor UI/CSS and wrapper dataset attributes. ChangesForm Validation Integration for File Upload and Dropdown Components
Sequence DiagramsequenceDiagram
participant User as User
participant DocType as DocType Select
participant Upload as FileUploadComponent
participant Validator as validateFileField
participant Dropdown as DropdownComponent
User->>Upload: upload file / delete file
Upload->>Validator: update fileData (value/format) -> validateFileField()
DocType->>Upload: change docType selection
Upload->>Validator: set fileData.docType -> validateFileField()
Validator->>DocType: dispatch "input" event
Validator->>Dropdown: recompute required state -> toggle "error" class
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
json-form-builder/src/components/FileUploadComponent.ts (1)
76-87:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPrefilled docType is not synced to parent file state.
At Line 79,
docTypeis only copied on"change". Prefilled/default select values won’t trigger this event, sofileData.docTypecan remain empty and required-file validation can be skipped for optional file fields.Suggested fix
if (docTypeFieldEl) { const selectEl = (docTypeFieldEl as HTMLDivElement).querySelector("select"); + const syncDocTypeToFileData = () => { + if (!selectEl) return; + state.formData[field.id] ??= {} as FileUploadData; + const fileData = state.formData[field.id] as FileUploadData; + fileData.docType = selectEl.value || ""; + }; selectEl?.addEventListener("change", () => { - state.formData[field.id] ??= {} as FileUploadData; - - const fileData = state.formData[field.id] as FileUploadData; - fileData.docType = selectEl.value; + syncDocTypeToFileData(); // validate file field immediately validateFileField(); }); + + // keep parent state in sync with prefilled/default dropdown value + syncDocTypeToFileData(); }🤖 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 `@json-form-builder/src/components/FileUploadComponent.ts` around lines 76 - 87, The prefilled select value isn't synced because you only set fileData.docType inside the select "change" handler; update the code so when you obtain docTypeFieldEl/selectEl you also ensure state.formData[field.id] is initialized (as FileUploadData) and immediately assign fileData.docType = selectEl.value, then keep the existing change listener to update on user edits and call validateFileField() after the immediate assignment to ensure validation runs for prefilled values (references: docTypeFieldEl, selectEl, state.formData[field.id], FileUploadData, validateFileField).
🤖 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 `@json-form-builder/src/components/FileUploadComponent.ts`:
- Line 295: The code assigns to state.lastErrors![field.id] without ensuring
state.lastErrors exists; initialize it before writing to avoid runtime
exceptions. Update the logic around the assignment (the block that sets
lastError for a field) to check if state.lastErrors is falsy and create an empty
object (with the correct typed shape) first, then assign
state.lastErrors[field.id] = lastError; keep the existing field.id and lastError
variables and preserve types when initializing.
---
Outside diff comments:
In `@json-form-builder/src/components/FileUploadComponent.ts`:
- Around line 76-87: The prefilled select value isn't synced because you only
set fileData.docType inside the select "change" handler; update the code so when
you obtain docTypeFieldEl/selectEl you also ensure state.formData[field.id] is
initialized (as FileUploadData) and immediately assign fileData.docType =
selectEl.value, then keep the existing change listener to update on user edits
and call validateFileField() after the immediate assignment to ensure validation
runs for prefilled values (references: docTypeFieldEl, selectEl,
state.formData[field.id], FileUploadData, validateFileField).
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 598bc9ca-27b9-4950-8733-8485b893e254
⛔ Files ignored due to path filters (1)
json-form-builder/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
json-form-builder/package.jsonjson-form-builder/src/components/DropdownComponent.tsjson-form-builder/src/components/FileUploadComponent.ts
Signed-off-by: GurukiranP <talk2gurukiran@gmail.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
json-form-builder/src/components/FileUploadComponent.ts (1)
308-317:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winInitialize
state.lastErrorsbefore assignment (line 315).Line 315 uses the non-null assertion operator (
state.lastErrors![field.id]) without ensuringstate.lastErrorsis initialized. This is inconsistent with the fix applied at line 295 and will throw a runtime error iflastErrorsis undefined.🐛 Proposed fix
if (shouldRequireFile && !file) { const result = handleRequiredValidation(state, errorContainer); + state.lastErrors ||= {}; state.lastErrors![field.id] = result.lastError; return; }Or use the same pattern as line 295-296 without the non-null assertion:
if (shouldRequireFile && !file) { const result = handleRequiredValidation(state, errorContainer); - state.lastErrors![field.id] = result.lastError; + state.lastErrors ||= {}; + state.lastErrors[field.id] = result.lastError; return; }🤖 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 `@json-form-builder/src/components/FileUploadComponent.ts` around lines 308 - 317, The code assigns into state.lastErrors using a non-null assertion (state.lastErrors![field.id]) without ensuring state.lastErrors is initialized; before setting state.lastErrors[field.id] when shouldRequireFile is true (in the validation block that uses file, shouldRequireFile, and handleRequiredValidation), ensure state.lastErrors is an object (e.g. initialize state.lastErrors = state.lastErrors ?? {} or otherwise guard so you don't use the non-null assertion). Update the validation block to initialize or guard state.lastErrors prior to assigning the result.lastError for field.id.
🤖 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.
Outside diff comments:
In `@json-form-builder/src/components/FileUploadComponent.ts`:
- Around line 308-317: The code assigns into state.lastErrors using a non-null
assertion (state.lastErrors![field.id]) without ensuring state.lastErrors is
initialized; before setting state.lastErrors[field.id] when shouldRequireFile is
true (in the validation block that uses file, shouldRequireFile, and
handleRequiredValidation), ensure state.lastErrors is an object (e.g. initialize
state.lastErrors = state.lastErrors ?? {} or otherwise guard so you don't use
the non-null assertion). Update the validation block to initialize or guard
state.lastErrors prior to assigning the result.lastError for field.id.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 58e01d6e-4263-4a5c-9639-50929e5805d9
📒 Files selected for processing (1)
json-form-builder/src/components/FileUploadComponent.ts
Signed-off-by: GurukiranP <talk2gurukiran@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@json-form-builder/src/components/DropdownComponent.ts`:
- Around line 108-111: Restore schema-level required validation by combining the
original field.required with the conditional upload/doc-type rule: compute the
effective required flag using field.required OR (isFileUploaded || hasDocType)
(or equivalent logic) and use that combined value where shouldBeRequired is
currently used (referencing shouldBeRequired, isFileUploaded, hasDocType,
select.value and field.required). Ensure the dropdown still fails validation
when field.required is true even if no file/upload flags are set, while also
remaining required when either isFileUploaded or hasDocType is true.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ab544008-7b04-4212-b74c-54e0714762f5
📒 Files selected for processing (8)
json-form-builder/src/JsonFormBuilder.tsjson-form-builder/src/components/DropdownComponent.tsjson-form-builder/src/components/FileUploadComponent.tsjson-form-builder/src/components/PasswordComponent.tsjson-form-builder/src/components/PhoneInputComponent.tsjson-form-builder/src/components/TextInputComponent.tsjson-form-builder/src/components/TextareaComponent.tsjson-form-builder/src/utils/responsive-style.ts
💤 Files with no reviewable changes (1)
- json-form-builder/src/components/FileUploadComponent.ts
Signed-off-by: GurukiranP <talk2gurukiran@gmail.com>
gk-XL7
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Chores
Bug Fixes
Style