Skip to content

fix(edit-content): show validation border on required file/image fields#35254

Merged
oidacra merged 3 commits intomainfrom
issue-33682-image-field-validation
Apr 10, 2026
Merged

fix(edit-content): show validation border on required file/image fields#35254
oidacra merged 3 commits intomainfrom
issue-33682-image-field-validation

Conversation

@oidacra
Copy link
Copy Markdown
Member

@oidacra oidacra commented Apr 7, 2026

Summary

  • Show red validation border on required binary, file, and image fields when Save is clicked with empty input
  • Align border styling across all field types using PrimeNG CSS variables for consistency
  • Fix premature validation in file/image fields caused by onTouched() being called during store initialization

Closes #33682

Changes

File Change
dot-edit-content-binary-field.component.scss Use PrimeNG CSS variables (--p-inputtext-border-color, --p-inputtext-border-radius, --p-inputtext-invalid-border-color) for border styling; add :host.ng-invalid.ng-touched rule for error state
dot-file-field.component.html Use PrimeNG CSS variables for border; add [class.border-color-error] binding via $hasError() input
dot-file-field.component.scss Add .border-color-error class using --p-inputtext-invalid-border-color
dot-file-field.component.ts Remove onTouched() from handleStoreValueChange to prevent premature touched state on init
dot-edit-content-binary-field.component.spec.ts Add validation tests for required field error state

Acceptance Criteria

  • When the user clicks Save without selecting an image/file, the validation border is displayed immediately
  • The behavior matches other input fields (Text Field), ensuring consistent UX
  • The validation border does not appear before the user triggers Save
  • The fix works for binary, file, and image field types
  • All field types use PrimeNG CSS variables for consistent styling
  • Unit tests added for validation behavior

Test Plan

  • Create a content type with required binary, file, image, and text fields
  • Navigate to create new content of that type
  • Verify all fields have the same gray border style on load (no error indicators)
  • Click Save without filling any fields
  • Verify all fields show red border and "This field is mandatory" message
  • Verify the red border color is consistent across all field types
  • Upload a file to one field, verify its red border clears

Visual Changes

CleanShot 2026-04-10 at 14 07 43@2x CleanShot 2026-04-10 at 14 07 48@2x

This PR fixes: #33682

@claude
Copy link
Copy Markdown
Contributor

claude bot commented Apr 7, 2026

Claude finished @oidacra's task in 3m 40s —— View job


PR Review

  • Gather context and read changed files
  • Analyze validation logic and edge cases
  • Review SCSS changes
  • Check test coverage
  • Post findings

To address the Copilot comment first: the validation message IS displayed. dot-edit-content-file-field.component.html renders the "This field is mandatory" message in the footer when fieldHasError is true (lines 19–25). That concern can be dismissed.


Issues

1. Inconsistent mechanisms for the same problem

Binary field and file field now use two completely different approaches for validation feedback:

  • Binary field (dot-edit-content-binary-field): Native CVA + Angular's ng-invalid/ng-touched CSS classes on :host. The component calls onTouched() on value change (line 204 of the binary field TS), so Angular form state drives the CSS.
  • File field (dot-file-field): External $hasError input threaded from BaseWrapperField → parent template → child component. The CVA never calls onTouched().

DotFileFieldComponent already implements ControlValueAccessor and registers itself as one. It could use :host.ng-invalid.ng-touched exactly as the binary field does, eliminating the need to thread $hasError down as an input. The current approach adds an extra data flow path for what Angular's form integration already handles.

2. File field CVA never calls onTouched() — behavioral inconsistency

After removing onTouched() from handleStoreValueChange, DotFileFieldComponent has no code path that ever calls this.onTouched(). The Angular forms spec requires CVAs to call onTouched() when the user interacts with the control. This has a concrete behavioral difference:

  • Binary field: user uploads a file, then removes it → onTouched() was called on upload → field is now ng-invalid.ng-touched → red border shows immediately.
  • File field: user uploads a file, then removes it → onTouched() never called → control.touched stays false → $hasError stays false → no error border until Save.

The fix overcorrects. The original bug was calling onTouched() during store initialization (before skip(1) or equivalent). The binary field avoids this by subscribing to value$ with skip(1), which skips the init emission but still calls onTouched() on actual user interactions. The file field fix removes all calls entirely. Fix this →

3. Leaky abstraction in BaseControlValueAccessor

// base-control-value-accesor.ts:13
abstract $hasError?: InputSignal<boolean>;

This abstract property is optional (?), so it doesn't enforce a contract. The binary field component doesn't implement it at all (it uses the CSS approach instead), while the file field implements it as a required input with an alias. The base class declares something it never uses. This should either be removed from the base class or used consistently.

4. control.events subscription starts after first render

BaseWrapperField sets up the $hasError subscription inside afterNextRender(). If markAllAsTouched() is called before the component completes its first render (unlikely but possible in testing or server-side), the TouchedChangeEvent would be missed. The updateState() initial call inside afterNextRender handles the snap-to-current-state correctly, but missed events during that window wouldn't retrigger it. Low risk in practice.

5. Missing tests for DotFileFieldComponent validation

The new tests cover binary field validation only. There are no tests that:

  • Verify DotFileFieldComponent applies the border-color-error CSS class when hasError input is true
  • Verify DotEditContentFileFieldComponent computes and propagates $hasError correctly to the child

The border-color-error class binding is a one-liner in the template, but since this is the actual fix for file/image fields, a targeted test would be valuable. Fix this →


Minor

  • In dot-edit-content-binary-field.component.scss, the uploading state border (border: 1px dashed var(--p-inputtext-border-color) at line 30) uses the border shorthand. If a field is simultaneously ng-invalid.ng-touched and in uploading state, the specificity of :host.ng-invalid.ng-touched .binary-field__container wins for border-color, but the uploading rule still sets border-style: dashed. You'd get a dashed red border during upload in an error state. Unlikely edge case, but unexpected visually.

  • binary-field__drop-zone at line 59 still uses legacy SCSS variables ($input-border-color, $border-radius-md, etc.) while the parent container now uses PrimeNG CSS variables. Pre-existing inconsistency, but worth tracking.


Clean

  • Removing onTouched() from handleStoreValueChange in the file field is the right direction — calling it during initialization is the confirmed bug. The implementation is correct for the Save-button-only UX.
  • The afterNextRender + merge(valueChanges, statusChanges, events) approach in BaseWrapperField is solid for driving the $hasError signal.
  • The new binary field validation tests are well-structured with a dedicated MockRequiredFormComponent host.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the edit-content binary/image field styling so required validation failure is visually indicated, aligning the field’s UX with the form’s invalid/touched behavior when saving.

Changes:

  • Add invalid-state SCSS selectors to render a red border when the binary field’s host control is invalid and touched/dirty.

@oidacra oidacra self-assigned this Apr 8, 2026
oidacra added 2 commits April 10, 2026 13:10
Add SCSS rules to show red border on the binary/image field container
when the form control is invalid and touched/dirty (e.g., on Save
with empty required image field).

Closes #33682
- Use PrimeNG CSS variables for consistent border styling across binary,
  file, and image fields (--p-inputtext-border-color, --p-inputtext-border-radius,
  --p-inputtext-invalid-border-color)
- Fix premature validation in dot-file-field by removing onTouched() call
  from handleStoreValueChange (was marking control as touched on init)
- Add validation border CSS class binding via $hasError() input for file fields
- Add validation unit tests for binary field component

Closes #33682
@oidacra oidacra force-pushed the issue-33682-image-field-validation branch from 2584d23 to 22e5972 Compare April 10, 2026 18:08
@oidacra oidacra changed the title fix(edit-content): show validation border on required image field fix(edit-content): show validation border on required file/image fields Apr 10, 2026
@oidacra oidacra marked this pull request as ready for review April 10, 2026 18:09
@oidacra oidacra enabled auto-merge April 10, 2026 18:11
@oidacra oidacra added this pull request to the merge queue Apr 10, 2026
Merged via the queue into main with commit e4ae614 Apr 10, 2026
28 checks passed
@oidacra oidacra deleted the issue-33682-image-field-validation branch April 10, 2026 21:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI: Safe To Rollback Area : Frontend PR changes Angular/TypeScript frontend code

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

[TASK] Fix validation behavior for Image Field on Save

4 participants