Skip to content

Commit 26d1004

Browse files
committed
Fix read-only Input zero value and align Dropdown validation precedence
- Input: treat numeric 0 as a real value when read-only instead of showing the empty read-only marker - PillInput: same read-only empty-value handling for consistency (internal) - Dropdown: FormField validation status now takes precedence over the local validationStatus prop, matching the other form controls - Add e2e tests for Input zero value, Dropdown validation precedence, and ComboBox read-only marker
1 parent fd2fc75 commit 26d1004

8 files changed

Lines changed: 95 additions & 4 deletions

File tree

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
"@salt-ds/core": patch
3+
---
4+
5+
Fixed `Dropdown` validation status precedence to align with the other form controls, so the `FormField` validation status now takes precedence over the `Dropdown`'s own `validationStatus` prop.
6+
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
"@salt-ds/core": patch
3+
---
4+
5+
Fixed `Input` treating a numeric `0` value as empty when read-only, which caused the `emptyReadOnlyMarker` (``) to be shown instead of the real value. Only `null`, `undefined`, `""`, and empty arrays are now treated as empty.
6+
7+

packages/core/src/__tests__/__e2e__/combo-box/ComboBox.cy.tsx

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { CustomFloatingComponentProvider, FLOATING_TEST_ID } from "../common";
77
const {
88
Default,
99
Readonly,
10+
ReadonlyEmpty,
1011
WithDefaultSelected,
1112
Disabled,
1213
DisabledOption,
@@ -903,4 +904,16 @@ describe("Given a ComboBox", () => {
903904
cy.findByRole("combobox").should("not.have.attr", "aria-describedby");
904905
cy.findByRole("combobox").should("not.have.attr", "aria-labelledby");
905906
});
907+
908+
describe("WHEN read only", () => {
909+
it("THEN should show the selected value rather than the empty marker", () => {
910+
cy.mount(<Readonly />);
911+
cy.findByRole("textbox").should("have.value", "California");
912+
});
913+
914+
it("THEN should show the empty marker when there is no selection", () => {
915+
cy.mount(<ReadonlyEmpty />);
916+
cy.findByRole("textbox").should("have.value", "—");
917+
});
918+
});
906919
});

packages/core/src/__tests__/__e2e__/dropdown/Dropdown.cy.tsx

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { Dropdown, Option } from "@salt-ds/core";
1+
import { Dropdown, FormField, FormFieldLabel, Option } from "@salt-ds/core";
22
import * as dropdownStories from "@stories/dropdown/dropdown.stories";
33
import { composeStories } from "@storybook/react-vite";
44
import { type KeyboardEventHandler, useRef, useState } from "react";
@@ -526,4 +526,31 @@ describe("Given a Dropdown", () => {
526526
.should("exist")
527527
.and("have.attr", "role", "listbox");
528528
});
529+
530+
describe("validation status", () => {
531+
it("should use its own validation status when not in a FormField", () => {
532+
cy.mount(
533+
<Dropdown validationStatus="warning">
534+
<Option value={1}>1</Option>
535+
</Dropdown>,
536+
);
537+
cy.findByRole("combobox").should("have.class", "saltDropdown-warning");
538+
});
539+
540+
it("should prioritize the FormField validation status over its own", () => {
541+
cy.mount(
542+
<FormField validationStatus="error">
543+
<FormFieldLabel>Field</FormFieldLabel>
544+
<Dropdown validationStatus="warning">
545+
<Option value={1}>1</Option>
546+
</Dropdown>
547+
</FormField>,
548+
);
549+
cy.findByRole("combobox").should("have.class", "saltDropdown-error");
550+
cy.findByRole("combobox").should(
551+
"not.have.class",
552+
"saltDropdown-warning",
553+
);
554+
});
555+
});
529556
});

packages/core/src/__tests__/__e2e__/input/Input.cy.tsx

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,18 @@ describe("GIVEN an Input", () => {
175175
cy.findByRole("textbox").should("have.value", "#");
176176
});
177177
});
178+
179+
describe("AND the value is zero", () => {
180+
it("THEN should show the zero value rather than the empty marker", () => {
181+
cy.mount(<Input defaultValue={0} readOnly />);
182+
cy.findByRole("textbox").should("have.value", "0");
183+
});
184+
185+
it("THEN should show a controlled zero value rather than the empty marker", () => {
186+
cy.mount(<Input value={0} readOnly />);
187+
cy.findByRole("textbox").should("have.value", "0");
188+
});
189+
});
178190
});
179191

180192
describe("WHEN used in Formfield", () => {

packages/core/src/dropdown/Dropdown.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ export const Dropdown = forwardRef(function Dropdown<Item>(
177177

178178
const disabled = Boolean(disabledProp) || formFieldDisabled;
179179
const readOnly = Boolean(readOnlyProp) || formFieldReadOnly;
180-
const validationStatus = validationStatusProp ?? formFieldValidationStatus;
180+
const validationStatus = formFieldValidationStatus ?? validationStatusProp;
181181
const required = formFieldRequired
182182
? ["required", "asterisk"].includes(formFieldRequired)
183183
: requiredProp;

packages/core/src/input/Input.tsx

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,16 @@ export interface InputProps
7474
bordered?: boolean;
7575
}
7676

77+
function isEmptyReadOnlyValue(
78+
value: InputProps["value"] | InputProps["defaultValue"],
79+
) {
80+
return (
81+
value == null ||
82+
value === "" ||
83+
(Array.isArray(value) && value.length === 0)
84+
);
85+
}
86+
7787
export const Input = forwardRef<HTMLDivElement, InputProps>(
7888
function Input(props, ref) {
7989
const { className, props: finalProps } = useClassNameInjection(
@@ -134,7 +144,10 @@ export const Input = forwardRef<HTMLDivElement, InputProps>(
134144

135145
const [focused, setFocused] = useState(false);
136146

137-
const isEmptyReadOnly = isReadOnly && !defaultValueProp && !valueProp;
147+
const isEmptyReadOnly =
148+
isReadOnly &&
149+
isEmptyReadOnlyValue(defaultValueProp) &&
150+
isEmptyReadOnlyValue(valueProp);
138151
const defaultValue = isEmptyReadOnly
139152
? emptyReadOnlyMarker
140153
: defaultValueProp;

packages/core/src/pill-input/PillInput.tsx

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,16 @@ export interface PillInputProps
8585
bordered?: boolean;
8686
}
8787

88+
function isEmptyReadOnlyValue(
89+
value: PillInputProps["value"] | PillInputProps["defaultValue"],
90+
) {
91+
return (
92+
value == null ||
93+
value === "" ||
94+
(Array.isArray(value) && value.length === 0)
95+
);
96+
}
97+
8898
export const PillInput = forwardRef(function PillInput(
8999
{
90100
"aria-activedescendant": ariaActiveDescendant,
@@ -148,7 +158,10 @@ export const PillInput = forwardRef(function PillInput(
148158
const [focused, setFocused] = useState(false);
149159
const [focusedPillIndex, setFocusedPillIndex] = useState(-1);
150160

151-
const isEmptyReadOnly = isReadOnly && !defaultValueProp && !valueProp;
161+
const isEmptyReadOnly =
162+
isReadOnly &&
163+
isEmptyReadOnlyValue(defaultValueProp) &&
164+
isEmptyReadOnlyValue(valueProp);
152165
const defaultValue = isEmptyReadOnly ? emptyReadOnlyMarker : defaultValueProp;
153166

154167
const {

0 commit comments

Comments
 (0)