Skip to content

Commit 31357ec

Browse files
nabramovitznorman-abramovitz
authored andcommitted
Switch Variables tab Add form to on-submit validation
Replace the reactive (per-keystroke) Angular form-state validation with a validate-on-click flow that pairs better with the L5 inline-form pattern. Why: reactive validation produced two surprises in the new L5 layout. First, "Name is required" appeared after a type-then-erase ("xx" then backspace) because Angular's `dirty` flag never resets to `pristine` — the field was visually identical to its initial state but flagged invalid. Second, inline error messages below the input pushed the form's box height past the 42px input, breaking the row's pixel stability. What changed: - Drop the `<form #addForm>` wrapper, the `required`/`[appUnique]` validators on the Name input, and `[disabled]="!addForm.valid"` on the ✓ button. - Validation now runs in the new `validateAndSave()` click handler: required (non-empty, non-whitespace), pattern (`/^[A-Za-z_][A-Za-z0-9_]*$/` — CF env var convention), and uniqueness against `envVarNames()`. On any failure the click handler populates the new `nameError` signal; on success it calls `envVarsDataSource.saveAdd()`. - Render the error in an absolute-positioned `<div>` above the Name input, sitting in the L5 row's 12px top-padding zone — out of flow, so the row stays at 68px in every state (closed, open empty, open with required error, open with pattern error, open with duplicate error). - Clear the error reactively as the user types (`(input)`) and on cancel (×) so a stale error doesn't carry across open/close cycles. - Drop the `UniqueDirective` import along with the validator removal; uniqueness is now checked manually in `validateAndSave()`. - Drop `[addForm]="addForm"` from the `<app-list>` — the legacy list no longer needs to know about the form since it lives in the L5 sub-nav. Tests: 6 new vitest cases covering validateAndSave (empty, whitespace, space in name, leading digit, valid input clears prior error) and clearNameError.
1 parent 2beb91d commit 31357ec

3 files changed

Lines changed: 119 additions & 22 deletions

File tree

src/frontend/packages/cloud-foundry/src/features/applications/application/application-tabs-base/tabs/variables-tab/variables-tab.component.html

Lines changed: 22 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -7,35 +7,38 @@
77
model bindings the legacy in-toolbar form used; save/cancel
88
call dataSource methods directly. -->
99
<div subNavForm class="flex items-center gap-2">
10-
<form #addForm="ngForm" novalidate class="flex items-center gap-2">
11-
<div class="flex flex-col">
12-
<input class="bg-white rounded-md border border-gray-300 px-3 py-2 text-base text-slate-800 placeholder:text-gray-400 placeholder:opacity-70 focus:outline-none focus:ring-1 focus:ring-primary" id="envVarName" name="envVarName" #envVarName="ngModel" placeholder="Name"
13-
[(ngModel)]="envVarsDataSource.addItem.name" required [appUnique]="envVarNames()">
14-
@if (envVarName.errors?.required && envVarName.dirty) {
15-
<div class="text-red-500 text-xs mt-0.5">Name is required</div>
16-
}
17-
@if (envVarName.errors?.appUnique && envVarName.dirty) {
18-
<div class="text-red-500 text-xs mt-0.5">{{envVarName.errors.appUnique.message}}</div>
19-
}
20-
</div>
21-
<div class="flex flex-col">
22-
<input class="bg-white rounded-md border border-gray-300 px-3 py-2 text-base text-slate-800 placeholder:text-gray-400 placeholder:opacity-70 focus:outline-none focus:ring-1 focus:ring-primary" id="envVarValue" name="envVarValue" placeholder="Value"
23-
[(ngModel)]="envVarsDataSource.addItem.value">
24-
</div>
25-
</form>
10+
<!-- Validation runs on ✓ click, not reactively. Errors land in
11+
an absolute-positioned label above the Name input (sitting
12+
in the L5 row's top padding) so the row stays pixel-stable
13+
whether the form is closed, open and valid, or open and
14+
invalid. -->
15+
<div class="relative">
16+
@if (nameError()) {
17+
<div class="absolute -top-3 left-1 text-red-500 text-[11px] leading-none whitespace-nowrap">
18+
{{ nameError() }}
19+
</div>
20+
}
21+
<input class="bg-white rounded-md border border-gray-300 px-3 py-2 text-base text-slate-800 placeholder:text-gray-400 placeholder:opacity-70 focus:outline-none focus:ring-1 focus:ring-primary"
22+
id="envVarName" name="envVarName" placeholder="Name"
23+
[(ngModel)]="envVarsDataSource.addItem.name"
24+
(input)="clearNameError()">
25+
</div>
26+
<input class="bg-white rounded-md border border-gray-300 px-3 py-2 text-base text-slate-800 placeholder:text-gray-400 placeholder:opacity-70 focus:outline-none focus:ring-1 focus:ring-primary"
27+
id="envVarValue" name="envVarValue" placeholder="Value"
28+
[(ngModel)]="envVarsDataSource.addItem.value">
2629
<button id="addFormButtonAdd" class="btn btn-icon btn-success h-9"
2730
type="button"
28-
(click)="envVarsDataSource.saveAdd()" [disabled]="!addForm.valid">
31+
(click)="validateAndSave()">
2932
<span class="material-icons">done</span>
3033
</button>
3134
<button id="addFormButtonCancel" class="btn btn-icon h-9"
3235
type="button"
33-
(click)="envVarsDataSource.cancelAdd()">
36+
(click)="clearNameError(); envVarsDataSource.cancelAdd()">
3437
<span class="material-icons">clear</span>
3538
</button>
3639
</div>
3740
</app-list-sub-nav>
38-
<app-list [addForm]="addForm" [suppressAddButton]="true"></app-list>
41+
<app-list [suppressAddButton]="true"></app-list>
3942
</div>
4043

4144
<div class="pt-5">

src/frontend/packages/cloud-foundry/src/features/applications/application/application-tabs-base/tabs/variables-tab/variables-tab.component.spec.ts

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,4 +56,55 @@ describe('VariablesTabComponent', () => {
5656
it('envVarNames returns empty array when app signal is undefined', () => {
5757
expect(component.envVarNames()).toEqual([]);
5858
});
59+
60+
describe('validateAndSave()', () => {
61+
beforeEach(() => {
62+
// The data source's addItem is the model the form binds to. Stub it
63+
// so validateAndSave can read .name without dragging in the full
64+
// legacy paginator pipeline.
65+
(component.envVarsDataSource as any).addItem = { name: '', value: '' };
66+
});
67+
68+
it('flags Name is required when the name is empty', () => {
69+
(component.envVarsDataSource as any).addItem.name = '';
70+
component.validateAndSave();
71+
expect(component.nameError()).toBe('Name is required');
72+
});
73+
74+
it('flags Name is required when the name is whitespace-only', () => {
75+
(component.envVarsDataSource as any).addItem.name = ' ';
76+
component.validateAndSave();
77+
expect(component.nameError()).toBe('Name is required');
78+
});
79+
80+
it('flags an invalid pattern when the name contains spaces', () => {
81+
(component.envVarsDataSource as any).addItem.name = 'bad name';
82+
component.validateAndSave();
83+
expect(component.nameError()).toMatch(/letters, digits, and underscores/i);
84+
});
85+
86+
it('flags an invalid pattern when the name starts with a digit', () => {
87+
(component.envVarsDataSource as any).addItem.name = '1FOO';
88+
component.validateAndSave();
89+
expect(component.nameError()).toMatch(/letters, digits, and underscores/i);
90+
});
91+
92+
it('accepts a valid name and clears any prior error', () => {
93+
component.nameError.set('Name is required');
94+
(component.envVarsDataSource as any).addItem.name = 'MY_VAR';
95+
// The legacy data source's saveAdd dispatches ngrx actions; stub it
96+
// so the test stays scoped to validation behavior.
97+
(component.envVarsDataSource as any).saveAdd = () => undefined;
98+
component.validateAndSave();
99+
expect(component.nameError()).toBe('');
100+
});
101+
});
102+
103+
describe('clearNameError()', () => {
104+
it('resets the error signal when called', () => {
105+
component.nameError.set('Name is required');
106+
component.clearNameError();
107+
expect(component.nameError()).toBe('');
108+
});
109+
});
59110
});

src/frontend/packages/cloud-foundry/src/features/applications/application/application-tabs-base/tabs/variables-tab/variables-tab.component.ts

Lines changed: 46 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { CommonModule } from '@angular/common';
2-
import { Component, OnInit, ChangeDetectionStrategy, computed, inject, Signal } from '@angular/core';
2+
import { Component, OnInit, ChangeDetectionStrategy, computed, inject, signal, Signal, WritableSignal } from '@angular/core';
33
import { toSignal } from '@angular/core/rxjs-interop';
44
import { FormsModule } from '@angular/forms';
55
import { Store } from '@ngrx/store';
@@ -17,7 +17,6 @@ import {
1717
ListSubNavAddAction,
1818
ListSubNavComponent,
1919
} from '../../../../../../../../core/src/shared/components/list-sub-nav/list-sub-nav.component';
20-
import { UniqueDirective } from '../../../../../../../../core/src/shared/components/unique.directive';
2120
import { stratosEndpointGuidKey } from '../../../../../../../../store/src/entity-request-pipeline/pipeline.types';
2221
import {
2322
ListAppEnvVar,
@@ -49,7 +48,6 @@ export interface VariableTabAllEnvVarType {
4948
ListComponent,
5049
ListSubNavComponent,
5150
CodeBlockComponent,
52-
UniqueDirective,
5351
]
5452
})
5553
export class VariablesTabComponent implements OnInit {
@@ -103,6 +101,51 @@ export class VariablesTabComponent implements OnInit {
103101
invoke: () => this.envVarsDataSource.startAdd(),
104102
};
105103

104+
/**
105+
* Validation error for the Name input — populated by validateAndSave()
106+
* when the user clicks the ✓ button with an invalid Name. Empty string
107+
* = no error to display. Cleared on every keystroke so the user sees
108+
* the error disappear as they correct the input.
109+
*
110+
* Validation runs on submit, not reactively per-keystroke, to keep the
111+
* L5 row pixel-stable: error sits in the row's top padding via absolute
112+
* positioning and only renders after the user attempts to save.
113+
*/
114+
readonly nameError: WritableSignal<string> = signal('');
115+
116+
/** CF env var names follow shell-variable convention: must start with a
117+
* letter or underscore, and contain only letters, digits, and
118+
* underscores. */
119+
private static readonly NAME_PATTERN = /^[A-Za-z_][A-Za-z0-9_]*$/;
120+
121+
/** Validate the Add Variable form and either save or surface an error
122+
* in the absolute-positioned slot above the Name input. */
123+
validateAndSave(): void {
124+
const name = (this.envVarsDataSource.addItem?.name ?? '').trim();
125+
if (!name) {
126+
this.nameError.set('Name is required');
127+
return;
128+
}
129+
if (!VariablesTabComponent.NAME_PATTERN.test(name)) {
130+
this.nameError.set('Use letters, digits, and underscores only; must start with a letter or underscore');
131+
return;
132+
}
133+
if (this.envVarNames().includes(name)) {
134+
this.nameError.set(`'${name}' is already in use`);
135+
return;
136+
}
137+
this.nameError.set('');
138+
this.envVarsDataSource.saveAdd();
139+
}
140+
141+
/** Clear any pending validation error so it doesn't linger as the user
142+
* edits. Bound to the Name input's (input) event. */
143+
clearNameError(): void {
144+
if (this.nameError()) {
145+
this.nameError.set('');
146+
}
147+
}
148+
106149
ngOnInit() {
107150
// appEnvVars is the paginator-backed ngrx path for all env var sections —
108151
// kept on the legacy path intentionally (Task 5 decision).

0 commit comments

Comments
 (0)