Flatten the eForm fill layout across every eForm-filling screen - #8015
Merged
Conversation
Replace the per-question mat-card + tinted mat-card-header with a bare .eform-field block: bold label above, muted description, then the control. dataItem.color survives as a 3px left accent bar instead of a tint bar, and is absent (with no indent) when the field has no colour. Remove the seven duplicated mat-labels from the leaf field components. Four were generic placeholders (Value/Text/Select date); three were bound to fieldValueObj.valueReadable, i.e. the selected value that mtx-select already renders in the control. element-date's mat-error moves outside mat-form-field because theme-workspace hides .mat-mdc-form-field-error-wrapper entirely. The cases page carried its own copy of the same boxed "Submitted date" + inner mat-label pattern, so it is flattened here too; the section nav becomes a flat link list. Adds the first unit tests for these components — they render every eForm field type for five screens and had no coverage at any level. Deletes three dead stylesheets whose selectors match nothing rendered. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015sXLtgzZU8QL9m84GqMkoJ
There was a problem hiding this comment.
Pull request overview
This PR redesigns the eForm “fill” UI across all eForm-filling screens by replacing the old mat-card/tinted-header layout with a flat, consistent question block (label + optional description + control), and updates navigation/validation rendering to match the agreed design.
Changes:
- Introduces global
.eform-section,.eform-field, and.eform-navstyling to support the flattened layout across all consumers. - Updates case-edit section rendering and multiple leaf field templates to remove duplicated
mat-labelusage and to render errors outsidemat-form-fieldwhere needed. - Adds first unit-test coverage for the structural/behavioral contract of
case-edit-switchandcase-edit-element, and removes dead/unused component styles.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| eform-client/src/scss/styles.scss | Adds global eForm fill layout + nav styling (.eform-section, .eform-field, .eform-nav). |
| eform-client/src/app/modules/cases/components/case-edit/case-edit.component.html | Flattens “Submitted date” block and updates right-side section navigation link markup. |
| eform-client/src/app/common/modules/eform-cases/components/case-edit/case-elements/element-text/element-text.component.html | Removes duplicated mat-label from text field renderer. |
| eform-client/src/app/common/modules/eform-cases/components/case-edit/case-elements/element-singleselect/element-singleselect.component.html | Removes duplicated mat-label from single-select renderer. |
| eform-client/src/app/common/modules/eform-cases/components/case-edit/case-elements/element-signature/element-signature.component.ts | Drops unused styleUrls reference. |
| eform-client/src/app/common/modules/eform-cases/components/case-edit/case-elements/element-signature/element-signature.component.scss | Removes dead stylesheet content. |
| eform-client/src/app/common/modules/eform-cases/components/case-edit/case-elements/element-picture/element-picture.component.ts | Drops unused styleUrls reference. |
| eform-client/src/app/common/modules/eform-cases/components/case-edit/case-elements/element-picture/element-picture.component.scss | Removes dead stylesheet content. |
| eform-client/src/app/common/modules/eform-cases/components/case-edit/case-elements/element-number/element-number.component.html | Removes duplicated mat-label from number renderer. |
| eform-client/src/app/common/modules/eform-cases/components/case-edit/case-elements/element-number-stepper/element-number-stepper.component.html | Removes duplicated mat-label from number-stepper renderer. |
| eform-client/src/app/common/modules/eform-cases/components/case-edit/case-elements/element-entityselect/element-entityselect.component.html | Removes duplicated mat-label from entity-select renderer. |
| eform-client/src/app/common/modules/eform-cases/components/case-edit/case-elements/element-entitysearch/element-entitysearch.component.html | Removes duplicated mat-label from entity-search renderer. |
| eform-client/src/app/common/modules/eform-cases/components/case-edit/case-elements/element-date/element-date.component.html | Removes duplicated mat-label and moves validation message outside mat-form-field. |
| eform-client/src/app/common/modules/eform-cases/components/case-edit/case-edit-switch/case-edit-switch.component.ts | Removes component-scoped stylesheet usage. |
| eform-client/src/app/common/modules/eform-cases/components/case-edit/case-edit-switch/case-edit-switch.component.spec.ts | Adds unit tests pinning the redesigned question-block contract. |
| eform-client/src/app/common/modules/eform-cases/components/case-edit/case-edit-switch/case-edit-switch.component.scss | Removes dead stylesheet content. |
| eform-client/src/app/common/modules/eform-cases/components/case-edit/case-edit-switch/case-edit-switch.component.html | Replaces mat-card field rendering with flat .eform-field blocks and accent-bar behavior. |
| eform-client/src/app/common/modules/eform-cases/components/case-edit/case-edit-element/case-edit-element.component.ts | Adds showSectionTitle input to control section heading rendering. |
| eform-client/src/app/common/modules/eform-cases/components/case-edit/case-edit-element/case-edit-element.component.spec.ts | Adds unit tests for anchors, approval group behavior, recursion, and event forwarding. |
| eform-client/src/app/common/modules/eform-cases/components/case-edit/case-edit-element/case-edit-element.component.html | Flattens section wrapper markup and updates extra-fields blocks to match new layout. |
| eform-client/src/app/common/modules/eform-cases/components/case-edit-nav/case-edit-nav.component.html | Updates section navigation markup to use .eform-nav__* classes. |
Suppressed comments (7)
eform-client/src/app/modules/cases/components/case-edit/case-edit.component.html:51
- The section nav uses an without an href. Anchors without href are not keyboard-focusable by default and can be missed by assistive tech, so this is an accessibility regression compared to a real link/button.
<a
class="eform-nav__link"
(click)="goToSection('#section' + element.id)"
>
eform-client/src/app/common/modules/eform-cases/components/case-edit/case-elements/element-number/element-number.component.html:5
- With the mat-label removed, this input is now unlabeled from an accessibility standpoint. Add an aria-label so the control has an accessible name.
<mat-form-field>
<input
matInput
type="text"
lang="de"
eform-client/src/app/common/modules/eform-cases/components/case-edit/case-elements/element-number-stepper/element-number-stepper.component.html:5
- With the mat-label removed, this number input no longer exposes an accessible name. Add an aria-label to keep the control accessible without reintroducing a visible floating label.
<mat-form-field>
<input
matInput
type="number"
[(ngModel)]="fieldValueObj.value"
eform-client/src/app/common/modules/eform-cases/components/case-edit/case-elements/element-date/element-date.component.html:7
- With the mat-label removed, the date input lacks an accessible name. Add an aria-label so assistive tech can identify the purpose of the picker.
<input
[required]="fieldValueObj.mandatory"
matInput
[matDatepicker]="picker"
[value]="fieldValueObj.value"
eform-client/src/app/common/modules/eform-cases/components/case-edit/case-elements/element-entityselect/element-entityselect.component.html:8
- After removing the mat-label, this entity select has no accessible name. For accessibility, the select should have a mat-label or an explicit aria-label/aria-labelledby tied to the question label shown above the control.
<mat-form-field>
<mtx-select
bindValue="id"
bindLabel="text"
(change)="onSelectedChanged($event)"
[items]="items">
</mtx-select>
</mat-form-field>
eform-client/src/app/common/modules/eform-cases/components/case-edit/case-elements/element-singleselect/element-singleselect.component.html:7
- Removing the mat-label leaves this without an accessible name. Angular Material form-fields generally require a label (mat-label) or an explicit aria-label/aria-labelledby on the control for screen readers.
<mat-form-field *ngIf="fieldValueObj !== undefined">
<mtx-select
bindValue="key"
bindLabel="value"
(change)="onSelectedChanged($event)"
[items]="fieldValueObj.keyValuePairList">
</mtx-select>
eform-client/src/app/common/modules/eform-cases/components/case-edit/case-elements/element-entitysearch/element-entitysearch.component.html:9
- With the mat-label removed, this entity search select no longer exposes an accessible name to screen readers. It should have an aria-label/aria-labelledby that corresponds to the question label rendered above.
<mat-form-field>
<mtx-select
bindValue="id"
bindLabel="text"
[typeahead]="typeahead"
(change)="onSelectedChanged($event)"
[items]="items">
</mtx-select>
</mat-form-field>
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
2
to
5
| <a | ||
| mat-button | ||
| class="eform-nav__link" | ||
| (click)="goToSection('#section' + element.id)" | ||
| class="d-flex w-100" | ||
| > |
Comment on lines
+1369
to
+1375
| &__label + .mat-mdc-form-field, | ||
| &__label + element-comment, | ||
| &__label + element-date, | ||
| &__label + element-number, | ||
| &__label + element-number-stepper, | ||
| &__label + element-singleselect, | ||
| &__label + element-text { |
Comment on lines
+1
to
+20
| import {Component, Input, NO_ERRORS_SCHEMA} from '@angular/core'; | ||
| import {ComponentFixture, TestBed} from '@angular/core/testing'; | ||
| import {By} from '@angular/platform-browser'; | ||
| import {CaseEditSwitchComponent} from './case-edit-switch.component'; | ||
| import {DataItemDto} from 'src/app/common/models'; | ||
|
|
||
| /** | ||
| * Structural contract of the redesigned eForm question block. | ||
| * | ||
| * These assertions exist because the block replaced a mat-card with a tinted | ||
| * mat-card-header, and because seven leaf components lost a duplicated | ||
| * mat-label. Nothing else in the repo covers this markup: no Playwright spec | ||
| * references a case-elements id or mat-card-header. | ||
| */ | ||
|
|
||
| @Component({selector: 'element-picture', template: '', standalone: false}) | ||
| class StubPictureComponent { | ||
| @Input() fieldValues: any; | ||
| @Input() fieldId: any; | ||
| } |
Comment on lines
+13
to
+20
| <input | ||
| required | ||
| matInput | ||
| [matDatepicker]="picker" | ||
| [value]="replyElement.doneAt" | ||
| (dateChange)="replyElement.doneAt = $event.value" | ||
| (click)="picker.open()" | ||
| > |
Comment on lines
1
to
4
| <mat-form-field> | ||
| <mat-label>{{'Text' | translate}}</mat-label> | ||
| <input | ||
| matInput | ||
| type="text" |
… fix spacing C1 — restore the mat-label on element-singleselect, element-entityselect and element-entitysearch. The premise for removing them was wrong: none of the three binds a value into mtx-select, so the label was the only thing rendering a previously submitted answer. Removing it blanked every answered dropdown on the cases and compliance review screens — invisible on the calendar modal, which always opens an unanswered case. Each template now carries a comment saying why it is not a duplicate. C2 — case-edit-element.component.spec.ts imported FormsModule without MatRadioModule, so NgModel matched [(ngModel)] on an unknown <mat-radio-group> with no ControlValueAccessor and threw NG01203. Two tests were failing and test-angular-unit was red. I4/I5 — sibling sections had no separation (a descendant rule cannot separate two <app-case-edit-element> hosts), and .eform-field:last-child reset the margin on the last question of every section, butting it against the next heading. Margins now sit on .eform-section. The leaf field components are custom elements defaulting to display:inline, which made half the spacing rules inert; they are display:block now. I3 — the h3 label was not programmatically associated with anything, leaving every control unlabelled to a screen reader. The block is now role="group" with aria-labelledby pointing at the label. M2 — the accent custom property was guarded only by dataItem.color while the class also excluded FieldContainer, so a coloured container wrote a property that inherited into its nested fields. Both now go through isAccented(). M3 — delete two more 0-byte stylesheets and their styleUrls entries. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015sXLtgzZU8QL9m84GqMkoJ
Closes the coverage gap review flagged: case-edit-switch's spec stubs the leaf components under NO_ERRORS_SCHEMA, so it cannot see whether a mat-label actually renders. This mounts the real components. Asserts text/number/number-stepper/date render none, and — more importantly — that element-singleselect still renders its label showing the saved answer. That second assertion exists specifically to stop the C1 regression coming back: the label looks like a duplicate of the question name above it, but it is the only renderer of valueReadable, because nothing binds a value into mtx-select. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015sXLtgzZU8QL9m84GqMkoJ
The dropdown rendered its group headers LIGHTER than its options — the
inverse of the design reference, where a native <select> gives bold flush
headers with indented options. Root cause: styles.scss:1635 sets
.ng-dropdown-panel .ng-option-label { font-weight: 600 !important }.
ng-select only renders .ng-option-label when no custom template is supplied,
so the options (no template) were hit by it and went bold, while the group
header (which had an ng-optgroup-tmp) escaped it and fell back to mtx's 500.
Adding an ng-option-tmp is therefore the only way out that does not fight
!important with more !important. The classes are styled here rather than in
the component because mtx-select forces appendTo: 'body', so the panel is not
a descendant of the modal and no component-scoped rule can reach it. This
follows the existing tl-batch-action-* pattern, which solved the same problem.
Measured in the browser: header 700 at 17px, options 400 at 33px — a 16px
indent, matching the reference.
Also restores .eform-nav__link / .eform-nav__group, which an earlier SCSS
rewrite in this branch spliced out. They are used by 5 templates and had no
CSS at all, so every section jump-link was rendering unstyled.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015sXLtgzZU8QL9m84GqMkoJ
The 20px on the option label was tuned against theme-workspace, but
theme-eform is the DEFAULT (app.component.ts:43) and behaves differently:
mtx-select ships `.ng-option.ng-option-child { padding-left: 32px }` in its
own ViewEncapsulation.None styles, which theme-workspace happens to cancel by
overriding the row padding to 12px and theme-eform does not. So the label
padding added to a row that was already indented — measured 36px of indent
under eform versus the intended 16px under workspace.
The indent now sits on the row at a flat 32px, which both themes compose to
the same 16px relative to the 16px optgroup row. Verified by measuring under
both body classes: 32px row padding, 16px indent, header 700, option 400 in
each.
Scoped with :has() so no other grouped dropdown is affected, at specificity
(0,5,0) to outrank theme-workspace's own (0,4,1) row rule. The option label
also gains an explicit colour, matching its group-label sibling — inherited,
it resolved through a --mat-sys-* token this bundle never emits.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015sXLtgzZU8QL9m84GqMkoJ
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
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.
Redesigns how a filled-in eForm renders, to match the reference design in
lorem-ipsum/eform/.Spec:
docs/superpowers/specs/2026-08-22-eform-fill-modal-redesign-design.mdMockup used to agree the design:
lorem-ipsum/eform/kvittering-redesign.htmlWhat changes
Each eForm question used to render as a
mat-cardwith a tintedmat-card-headerbar, and the control underneath carried a floatingmat-labelrepeating the question name — so the label appeared twice ("Udført dato" in the bar, "Vælg dato*" inside the field).It now renders as a flat block: bold label above, muted description, then the control.
dataItem.colorsurvives as a 3px left accent bar instead of a tint bar, and is absent — with no indent shift — when the field has no colour.mat-label. Four were generic placeholders (Value,Text,Select date); three were bound tofieldValueObj.valueReadable, i.e. the selected valuemtx-selectalready renders in the control.element-date'smat-errormoves outsidemat-form-field, becausetheme-workspacesets.mat-mdc-form-field-error-wrapper { display: none }— the message is invisible under that theme today.mat-labelpattern, so it is flattened too, and its section nav becomes a flat link list.Field chrome itself is deliberately untouched: it comes ambiently from whichever theme is active. The new SCSS uses only tokens defined in both
_eform.scssand_workspace.scss, with no literal hex.Blast radius
case-edit-switch/case-edit-elementare shared by five screens: the calendar complete modal, cases page, compliance case modal, compliance case page and BC case page. All five change together — deliberately, so two eForm-filling designs don't coexist.No existing test asserts on
mat-card-headeror on any removedmat-label(verified by grep across Playwright, Cypress and Jest in both repos), so nothing needed updating.Tests
These components render every eForm field type for five screens and had no coverage at any level. This adds the first:
case-edit-switch.component.spec.ts— label/description rendering, accent-bar presence and absence, nomat-card-header,SaveButtonskipped,FieldContainerexempt, and a table-driven case asserting all 17 leaf renderers (the 14 field types the calendar eForm never exercises included).case-edit-element.component.spec.ts— pins the behaviour the five screens depend on: the#section{id}anchorcase-edit-navscrolls to, the approval radio group, nested-section recursion.Both run in CI via the existing
test-angular-unitjob.Also removed
Three dead stylesheets whose selectors match nothing rendered (
case-edit-switchtargeting.mat-mdc-card-title;element-pictureandelement-signaturedefining an unused.card-img-adaptivewith every media query commented out), plus a stray>that rendered a literal character next to every comment field.Ordering
The companion plugin PR (calendar modal + 3 plugin screens) must not merge before this one: plugin CI always builds against frontend
stable, and the plugin template binds[showSectionTitle], an@Inputadded here.🤖 Generated with Claude Code
https://claude.ai/code/session_015sXLtgzZU8QL9m84GqMkoJ