Skip to content

Commit efa9f93

Browse files
diamonwigginsCopilot
diamonwiggins
and
Copilot
authored
feat(ui): highlight required items (#2546)
* required config visual indicators * f * address review comments * fix radio button focus * move validation to api * f * fix web lint and api integration tests * Update web/src/components/wizard/tests/ConfigurationStep.test.tsx Co-authored-by: Copilot <[email protected]> * fix lint * resolve conflcits * cleanup coments * address feedback * fix integration tests * f * use refs --------- Co-authored-by: Copilot <[email protected]>
1 parent 3330e80 commit efa9f93

File tree

12 files changed

+685
-45
lines changed

12 files changed

+685
-45
lines changed

‎Makefile‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ initial-release: check-env-EC_VERSION check-env-APP_VERSION
202202

203203
.PHONY: rebuild-release
204204
rebuild-release: export EC_VERSION = $(VERSION)-$(CURRENT_USER)
205-
rebuild-release: export RELEASE_YAML_DIR = e2e/kots-release-install
205+
rebuild-release: export RELEASE_YAML_DIR = $(if $(filter 1,$(ENABLE_V3)),e2e/kots-release-install-v3,e2e/kots-release-install)
206206
rebuild-release: check-env-EC_VERSION check-env-APP_VERSION
207207
UPLOAD_BINARIES=0 \
208208
SKIP_RELEASE=1 \

‎api/integration/kubernetes/install/app_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -288,9 +288,9 @@ func TestKubernetesPatchAppConfigValues(t *testing.T) {
288288
assert.Equal(t, http.StatusBadRequest, apiError.StatusCode)
289289
assert.Len(t, apiError.Errors, 2)
290290
assert.Equal(t, apiError.Errors[0].Field, "required-item")
291-
assert.Equal(t, apiError.Errors[0].Message, "item is required")
291+
assert.Equal(t, apiError.Errors[0].Message, "Required Item is required")
292292
assert.Equal(t, apiError.Errors[1].Field, "required-password")
293-
assert.Equal(t, apiError.Errors[1].Message, "item is required")
293+
assert.Equal(t, apiError.Errors[1].Message, "Required Password is required")
294294
})
295295
}
296296

@@ -478,7 +478,7 @@ func TestInstallController_PatchAppConfigValuesWithAPIClient(t *testing.T) {
478478
assert.Equal(t, http.StatusBadRequest, apiErr.StatusCode, "Error should have BadRequest status code")
479479
assert.Len(t, apiErr.Errors, 1, "Should have one validation error")
480480
assert.Equal(t, "required-item", apiErr.Errors[0].Field, "Error should be for required-item field")
481-
assert.Equal(t, "item is required", apiErr.Errors[0].Message, "Error should indicate item is required")
481+
assert.Equal(t, "Required Item is required", apiErr.Errors[0].Message, "Error should indicate item is required")
482482
})
483483

484484
// Test PatchKubernetesAppConfigValues with invalid state transition

‎api/integration/linux/install/app_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -290,9 +290,9 @@ func TestLinuxPatchAppConfigValues(t *testing.T) {
290290
assert.Equal(t, http.StatusBadRequest, apiError.StatusCode)
291291
assert.Len(t, apiError.Errors, 2)
292292
assert.Equal(t, apiError.Errors[0].Field, "required-item")
293-
assert.Equal(t, apiError.Errors[0].Message, "item is required")
293+
assert.Equal(t, apiError.Errors[0].Message, "Required Item is required")
294294
assert.Equal(t, apiError.Errors[1].Field, "required-password")
295-
assert.Equal(t, apiError.Errors[1].Message, "item is required")
295+
assert.Equal(t, apiError.Errors[1].Message, "Required Password is required")
296296
})
297297
}
298298

@@ -480,7 +480,7 @@ func TestInstallController_PatchAppConfigValuesWithAPIClient(t *testing.T) {
480480
assert.Equal(t, http.StatusBadRequest, apiErr.StatusCode, "Error should have BadRequest status code")
481481
assert.Len(t, apiErr.Errors, 1, "Should have one validation error")
482482
assert.Equal(t, "required-item", apiErr.Errors[0].Field, "Error should be for required-item field")
483-
assert.Equal(t, "item is required", apiErr.Errors[0].Message, "Error should indicate item is required")
483+
assert.Equal(t, "Required Item is required", apiErr.Errors[0].Message, "Error should indicate item is required")
484484
})
485485

486486
// Test PatchLinuxAppConfigValues with invalid state transition

‎api/internal/managers/app/config/config.go‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,6 @@ const (
2020
)
2121

2222
var (
23-
// ErrConfigItemRequired is returned when a required item is not set
24-
ErrConfigItemRequired = errors.New("item is required")
2523
// ErrValueNotBase64Encoded is returned when a file item value is not base64 encoded
2624
ErrValueNotBase64Encoded = errors.New("value must be base64 encoded for file items")
2725
)
@@ -93,7 +91,8 @@ func (m *appConfigManager) ValidateConfigValues(configValues types.AppConfigValu
9391
configValue := getConfigValueFromItem(item, configValues)
9492
// check required items
9593
if isRequiredItem(item) && isUnsetItem(configValue) {
96-
ve = types.AppendFieldError(ve, item.Name, ErrConfigItemRequired)
94+
fieldError := createRequiredFieldError(item)
95+
ve = types.AppendFieldError(ve, item.Name, fieldError)
9796
}
9897
// check value is base64 encoded for file items
9998
if isFileType(item) && !isValueBase64Encoded(configValue) {
@@ -306,6 +305,15 @@ func isFileType(item kotsv1beta1.ConfigItem) bool {
306305
return item.Type == "file"
307306
}
308307

308+
// createRequiredFieldError creates a user-friendly error message for required fields
309+
func createRequiredFieldError(item kotsv1beta1.ConfigItem) error {
310+
fieldName := item.Title
311+
if fieldName == "" {
312+
fieldName = item.Name
313+
}
314+
return fmt.Errorf("%s is required", fieldName)
315+
}
316+
309317
// isValueBase64Encoded checks if the value of a ConfigValue is base64 encoded, this is used for file items
310318
func isValueBase64Encoded(configValue kotsv1beta1.ConfigValue) bool {
311319
if configValue.Value == "" {

‎web/src/components/common/Button.tsx‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ interface ButtonProps {
1313
dataTestId?: string;
1414
}
1515

16-
const Button: React.FC<ButtonProps> = ({
16+
const Button = React.forwardRef<HTMLButtonElement, ButtonProps>(({
1717
children,
1818
onClick,
1919
type = 'button',
@@ -23,7 +23,7 @@ const Button: React.FC<ButtonProps> = ({
2323
className = '',
2424
icon,
2525
dataTestId
26-
}) => {
26+
}, ref) => {
2727
const { settings } = useSettings();
2828
const themeColor = settings.themeColor;
2929

@@ -46,6 +46,7 @@ const Button: React.FC<ButtonProps> = ({
4646

4747
return (
4848
<button
49+
ref={ref}
4950
type={type}
5051
onClick={onClick}
5152
disabled={disabled}
@@ -61,6 +62,8 @@ const Button: React.FC<ButtonProps> = ({
6162
{children}
6263
</button>
6364
);
64-
};
65+
});
66+
67+
Button.displayName = 'Button';
6568

6669
export default Button;

‎web/src/components/common/Checkbox.tsx‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ interface CheckboxProps {
1616
dataTestId?: string;
1717
}
1818

19-
const Checkbox: React.FC<CheckboxProps> = ({
19+
const Checkbox = React.forwardRef<HTMLInputElement, CheckboxProps>(({
2020
id,
2121
label,
2222
helpText,
@@ -28,14 +28,15 @@ const Checkbox: React.FC<CheckboxProps> = ({
2828
className = '',
2929
labelClassName = '',
3030
dataTestId,
31-
}) => {
31+
}, ref) => {
3232
const { settings } = useSettings();
3333
const themeColor = settings.themeColor;
3434

3535
return (
3636
<div className="mb-4">
3737
<div className="flex items-center space-x-3">
3838
<input
39+
ref={ref}
3940
id={id}
4041
type="checkbox"
4142
checked={checked}
@@ -58,6 +59,8 @@ const Checkbox: React.FC<CheckboxProps> = ({
5859
<HelpText helpText={helpText} error={error} />
5960
</div>
6061
);
61-
};
62+
});
63+
64+
Checkbox.displayName = 'Checkbox';
6265

6366
export default Checkbox;

‎web/src/components/common/Input.tsx‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ interface InputProps {
2424
dataTestId?: string;
2525
}
2626

27-
const Input: React.FC<InputProps> = ({
27+
const Input = React.forwardRef<HTMLInputElement, InputProps>(({
2828
id,
2929
label,
3030
helpText,
@@ -43,7 +43,7 @@ const Input: React.FC<InputProps> = ({
4343
className = '',
4444
labelClassName = '',
4545
dataTestId,
46-
}) => {
46+
}, ref) => {
4747
const { settings } = useSettings();
4848
const [showPassword, setShowPassword] = useState(false);
4949
const themeColor = settings.themeColor;
@@ -68,6 +68,7 @@ const Input: React.FC<InputProps> = ({
6868
</div>
6969
)}
7070
<input
71+
ref={ref}
7172
id={id}
7273
type={inputType}
7374
value={value}
@@ -106,6 +107,8 @@ const Input: React.FC<InputProps> = ({
106107
<HelpText helpText={helpText} defaultValue={defaultValue} error={error} />
107108
</div>
108109
);
109-
};
110+
});
111+
112+
Input.displayName = 'Input';
110113

111114
export default Input;

‎web/src/components/common/Radio.tsx‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ interface RadioProps {
1717
labelClassName?: string;
1818
}
1919

20-
const Radio: React.FC<RadioProps> = ({
20+
const Radio = React.forwardRef<HTMLInputElement, RadioProps>(({
2121
id,
2222
label,
2323
helpText,
@@ -29,7 +29,7 @@ const Radio: React.FC<RadioProps> = ({
2929
disabled = false,
3030
className = '',
3131
labelClassName = '',
32-
}) => {
32+
}, ref) => {
3333
const { settings } = useSettings();
3434
const themeColor = settings.themeColor;
3535

@@ -40,9 +40,10 @@ const Radio: React.FC<RadioProps> = ({
4040
{required && <span className="text-red-500 ml-1">*</span>}
4141
</label>
4242
<div className="space-y-2">
43-
{options.map(option => (
43+
{options.map((option, index) => (
4444
<div key={option.name} className="flex items-center">
4545
<input
46+
ref={index === 0 ? ref : undefined}
4647
type="radio"
4748
id={option.name}
4849
name={id}
@@ -68,6 +69,8 @@ const Radio: React.FC<RadioProps> = ({
6869
<HelpText helpText={helpText} error={error} />
6970
</div>
7071
);
71-
};
72+
});
73+
74+
Radio.displayName = 'Radio';
7275

7376
export default Radio;

‎web/src/components/common/Textarea.tsx‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { ChangeEvent, CSSProperties } from 'react';
1+
import React, { ChangeEvent, CSSProperties } from 'react';
22
import { useSettings } from '../../contexts/SettingsContext';
33
import HelpText from './HelpText';
44

@@ -19,7 +19,7 @@ interface TextareaProps {
1919
dataTestId?: string;
2020
}
2121

22-
const Textarea = ({
22+
const Textarea = React.forwardRef<HTMLTextAreaElement, TextareaProps>(({
2323
id,
2424
label,
2525
helpText,
@@ -34,7 +34,7 @@ const Textarea = ({
3434
className = '',
3535
labelClassName = '',
3636
dataTestId,
37-
}: TextareaProps) => {
37+
}, ref) => {
3838
const { settings } = useSettings();
3939
const themeColor = settings.themeColor;
4040

@@ -45,6 +45,7 @@ const Textarea = ({
4545
{required && <span className="text-red-500 ml-1">*</span>}
4646
</label>
4747
<textarea
48+
ref={ref}
4849
id={id}
4950
value={value}
5051
onChange={onChange}
@@ -67,6 +68,8 @@ const Textarea = ({
6768
<HelpText helpText={helpText} defaultValue={defaultValue} error={error} />
6869
</div>
6970
);
70-
};
71+
});
72+
73+
Textarea.displayName = 'Textarea';
7174

7275
export default Textarea;

‎web/src/components/common/file/FileInput.tsx‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ interface FileInputState {
2828
internalError: string | null;
2929
}
3030

31-
const FileInput: React.FC<FileInputProps> = ({
31+
const FileInput = React.forwardRef<HTMLButtonElement, FileInputProps>(({
3232
id,
3333
label,
3434
value,
@@ -44,7 +44,7 @@ const FileInput: React.FC<FileInputProps> = ({
4444
className = '',
4545
labelClassName = '',
4646
dataTestId,
47-
}) => {
47+
}, ref) => {
4848
const fileInputRef = useRef<HTMLInputElement>(null);
4949

5050
const [state, setState] = useState<FileInputState>({
@@ -206,6 +206,7 @@ const FileInput: React.FC<FileInputProps> = ({
206206

207207
<div className="flex items-center">
208208
<Button
209+
ref={ref}
209210
variant="outline"
210211
onClick={handleClick}
211212
disabled={disabled || state.isProcessing}
@@ -236,6 +237,8 @@ const FileInput: React.FC<FileInputProps> = ({
236237
<HelpText dataTestId={dataTestId || id} helpText={helpText} defaultValue={defaultValue ? "File provided" : undefined} error={displayError || undefined} />
237238
</div>
238239
);
239-
};
240+
});
241+
242+
FileInput.displayName = 'FileInput';
240243

241244
export default FileInput;

0 commit comments

Comments
 (0)