Skip to content
Open
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 1 addition & 14 deletions packages/components/scripts/post-build/components.ts
Original file line number Diff line number Diff line change
Expand Up @@ -371,20 +371,7 @@ export const getComponents = (): Component[] => [
vue: [{ from: ', index', to: '' }],
stencil: [{ from: 'HTMLElement', to: 'HTMLInputElement' }],
react: [{ from: /HTMLAttributes/g, to: 'InputHTMLAttributes' }],
angular: [
{ from: '<HTMLElement>', to: '<HTMLInputElement>' },
{
from: 'writeValue(value: any) {',
to:
'writeValue(value: any) {\n' +
'if (!value && value !== "" && (this.type() === "date" ||\n' +
' this.type() === "time" ||\n' +
' this.type() === "week" ||\n' +
' this.type() === "month" ||\n' +
' this.type() === "datetime-local"\n' +
' )) return;'
}
]
angular: [{ from: '<HTMLElement>', to: '<HTMLInputElement>' }]
},
config: {
vue: {
Expand Down
2 changes: 1 addition & 1 deletion packages/components/scripts/post-build/react.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ export default DB${upperComponentName};`
/* We need to overwrite the internal state._value property just for react to have controlled components.
* It works for Angular & Vue, so we overwrite it only for React. */
{
from: 'props.value ?? _value',
from: 'props.value ?? _value ?? ""',
to: 'props.value'
}
];
Expand Down
22 changes: 16 additions & 6 deletions packages/components/src/components/input/input.lite.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,13 @@ export default function DBInput(props: DBInputProps) {
});

useTarget({
angular: () => handleFrameworkEventAngular(state, event),
angular: () =>
handleFrameworkEventAngular(
state,
event,
'value',
state._value
),
vue: () => handleFrameworkEventVue(() => {}, event)
});
state.handleValidation();
Expand All @@ -161,7 +167,13 @@ export default function DBInput(props: DBInputProps) {
});

useTarget({
angular: () => handleFrameworkEventAngular(state, event),
angular: () =>
handleFrameworkEventAngular(
state,
event,
'value',
state._value
),
vue: () => handleFrameworkEventVue(() => {}, event)
});
state.handleValidation();
Expand Down Expand Up @@ -235,9 +247,7 @@ export default function DBInput(props: DBInputProps) {
}, [state._id]);

onUpdate(() => {
if (props.value !== undefined) {
state._value = props.value;
}
state._value = props.value;
}, [props.value]);

onUpdate(() => {
Expand Down Expand Up @@ -305,7 +315,7 @@ export default function DBInput(props: DBInputProps) {
disabled={getBoolean(props.disabled, 'disabled')}
required={getBoolean(props.required, 'required')}
step={getStep(props.step)}
value={props.value ?? state._value}
value={props.value ?? state._value ?? ''}
maxLength={getNumber(props.maxLength, props.maxlength)}
minLength={getNumber(props.minLength, props.minlength)}
max={getInputValue(props.max, props.type)}
Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/components/select/select.lite.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ export default function DBSelect(props: DBSelectProps) {
id={state._id}
name={props.name}
size={props.size}
value={props.value ?? state._value}
value={props.value ?? state._value ?? ''}
autocomplete={props.autocomplete}
multiple={props.multiple}
onInput={(event: ChangeEvent<HTMLSelectElement>) =>
Expand Down
6 changes: 2 additions & 4 deletions packages/components/src/components/textarea/textarea.lite.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -206,9 +206,7 @@ export default function DBTextarea(props: DBTextareaProps) {
}, [state._id]);

onUpdate(() => {
if (props.value !== undefined) {
state._value = props.value;
}
state._value = props.value;
}, [props.value]);

onUpdate(() => {
Expand Down Expand Up @@ -288,7 +286,7 @@ export default function DBTextarea(props: DBTextareaProps) {
onFocus={(event: InteractionEvent<HTMLTextAreaElement>) =>
state.handleFocus(event)
}
value={props.value ?? state._value}
value={props.value ?? state._value ?? ''}
aria-describedby={props.ariaDescribedBy ?? state._descByIds}
placeholder={props.placeholder ?? DEFAULT_PLACEHOLDER}
rows={getNumber(props.rows, DEFAULT_ROWS)}
Expand Down
123 changes: 123 additions & 0 deletions packages/components/src/utils/form-components.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
import { describe, expect, it, vi } from 'vitest';
import {
handleFrameworkEventAngular,
handleFrameworkEventVue
} from './form-components';

const createNumberEvent = (
value: string,
badInput: boolean,
inputType?: string
) => ({
inputType,
target: {
type: 'number',
value,
validity: { badInput }
}
});

const createTextEvent = (value: string) => ({
target: {
type: 'text',
value,
validity: { badInput: false }
}
});

describe('handleFrameworkEventAngular', () => {
it('calls propagateChange and writeValue for valid number value', () => {
const component = {
propagateChange: vi.fn(),
writeValue: vi.fn()
};
const event = createNumberEvent('1.5', false);
handleFrameworkEventAngular(component, event);
expect(component.propagateChange).toHaveBeenCalledWith('1.5');
expect(component.writeValue).toHaveBeenCalledWith('1.5');
});

it('calls propagateChange but skips writeValue when number input has badInput (intermediate state like "1.")', () => {
const component = {
propagateChange: vi.fn(),
writeValue: vi.fn()
};
const event = {
type: 'input',
data: '.',
inputType: 'insertText',
target: { type: 'number', value: '1.' }
};
handleFrameworkEventAngular(component, event, 'value', '1');
expect(component.propagateChange).not.toHaveBeenCalled();
expect(component.writeValue).not.toHaveBeenCalled();
});
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

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

The test description and implementation don't match the actual code in form-components.ts. The test claims to check for badInput (intermediate state like "1."), but the actual implementation in form-components.ts checks event.data === '.' instead of validity.badInput. The test event object doesn't include event.type = 'input' which is required by the implementation logic at line 23. This test would pass but doesn't actually validate the real implementation behavior.

Copilot uses AI. Check for mistakes.

it('calls propagateChange but skips writeValue when number input value is empty after insertText (e.g. comma "," in browsers where badInput stays false)', () => {
const component = {
propagateChange: vi.fn(),
writeValue: vi.fn()
};
const event = {
type: 'input',
data: ',',
inputType: 'insertText',
target: { type: 'number', value: '' }
};
handleFrameworkEventAngular(component, event, 'value', '1');
expect(component.propagateChange).not.toHaveBeenCalled();
expect(component.writeValue).not.toHaveBeenCalled();
});
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

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

The test description and implementation don't match the actual code in form-components.ts. The test claims to check for the comma scenario, but the actual implementation checks event.data === ',' instead of checking for badInput === false with empty value. The test event object doesn't include event.type = 'input' which is required by the implementation logic at line 23. This test would pass but doesn't actually validate the real implementation behavior.

Copilot uses AI. Check for mistakes.

it('calls propagateChange and writeValue when number input is cleared via backspace (deleteContentBackward)', () => {
const component = {
propagateChange: vi.fn(),
writeValue: vi.fn()
};
const event = createNumberEvent('', false, 'deleteContentBackward');
handleFrameworkEventAngular(component, event);
expect(component.propagateChange).toHaveBeenCalledWith('');
expect(component.writeValue).toHaveBeenCalledWith('');
});
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

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

The test event is missing event.type = 'input' which is required for the implementation to check at line 23 of form-components.ts. Without this property, the implementation would skip the number input handling entirely and proceed to call both propagateChange and writeValue, making this test pass for the wrong reason.

Copilot uses AI. Check for mistakes.

it('calls propagateChange and writeValue for text input type', () => {
const component = {
propagateChange: vi.fn(),
writeValue: vi.fn()
};
const event = createTextEvent('hello');
handleFrameworkEventAngular(component, event);
expect(component.propagateChange).toHaveBeenCalledWith('hello');
expect(component.writeValue).toHaveBeenCalledWith('hello');
});
});
Comment on lines +31 to +149
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

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

The tests don't cover the date/time input handling logic (lines 13-20 in form-components.ts). Given that this logic was previously implemented as an Angular-specific build-time injection and is now moved to runtime, it should have test coverage to ensure it works correctly for date, time, week, month, and datetime-local input types.

Copilot uses AI. Check for mistakes.

describe('handleFrameworkEventVue', () => {
it('emits update:value for valid number value', () => {
const emit = vi.fn();
const event = createNumberEvent('1.5', false);
handleFrameworkEventVue(emit, event);
expect(emit).toHaveBeenCalledWith('update:value', '1.5');
});

it('emits update:value with empty string when number input has badInput (intermediate state like "1.")', () => {
const emit = vi.fn();
const event = createNumberEvent('', true);
handleFrameworkEventVue(emit, event);
expect(emit).toHaveBeenCalledWith('update:value', '');
});

it('emits update:value when number input is cleared (empty, no badInput)', () => {
const emit = vi.fn();
const event = createNumberEvent('', false);
handleFrameworkEventVue(emit, event);
expect(emit).toHaveBeenCalledWith('update:value', '');
});

it('emits update:value for text input type', () => {
const emit = vi.fn();
const event = createTextEvent('hello');
handleFrameworkEventVue(emit, event);
expect(emit).toHaveBeenCalledWith('update:value', 'hello');
});
});
36 changes: 33 additions & 3 deletions packages/components/src/utils/form-components.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,40 @@ import { delay } from './index';
export const handleFrameworkEventAngular = (
component: any,
event: any,
modelValue: string = 'value'
modelValue: string = 'value',
lastValue?: any
): void => {
component.propagateChange(event.target[modelValue]);
component.writeValue(event.target[modelValue]);
const value = event.target[modelValue];
const type = event.target?.type;

if (
!value &&
value !== '' &&
['date', 'time', 'week', 'month', 'datetime-local'].includes(type)
) {
// If value is empty and type date we skip `writingValue` function
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

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

The date/time handling logic has a flaw: when both propagateChange and writeValue are skipped (early return), Angular forms won't be notified of the clearing action. The condition !value && value !== '' is checking if value is falsy but not an empty string. However, for date/time inputs, the browser typically returns an empty string '' when cleared, not null or undefined. This means the condition will be false when the input is cleared (because value === '' is true), and the early return won't trigger, which is actually correct behavior. But if the value is undefined or null, the condition will be true and the function will return early without propagating the change, which could cause Angular forms to not update properly.

Suggested change
!value &&
value !== '' &&
['date', 'time', 'week', 'month', 'datetime-local'].includes(type)
) {
// If value is empty and type date we skip `writingValue` function
value === '' &&
['date', 'time', 'week', 'month', 'datetime-local'].includes(type)
) {
// If value is empty and type date/time, propagate the change but skip `writeValue`
component.propagateChange(value);

Copilot uses AI. Check for mistakes.
return;
}

if (type === 'number') {
if (event.type === 'input') {
if (
['.', ','].includes(event.data) ||
(lastValue.toString().includes('.') &&
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

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

Potential runtime error: When lastValue is undefined (not provided), calling lastValue.toString() will throw a TypeError. The implementation should check if lastValue exists before calling .toString() on it, or handle the optional parameter properly.

Suggested change
(lastValue.toString().includes('.') &&
(lastValue != null &&
lastValue.toString().includes('.') &&

Copilot uses AI. Check for mistakes.
event.inputType === 'deleteContentBackward')
) {
// Skip `writingValue` function if number type and input event
// and `.` or `,` was typed
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

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

The logic only handles '.' and ',' decimal separators but doesn't account for other valid intermediate states in number inputs, such as typing 'e' or 'E' for scientific notation (e.g., "1e"), typing '+' or '-' for signs, or typing multiple decimals. These will also result in event.target.value === '' but won't be caught by event.data === '.' || event.data === ',', potentially causing the same clearing issue this fix is trying to address.

Suggested change
['.', ','].includes(event.data) ||
(lastValue.toString().includes('.') &&
event.inputType === 'deleteContentBackward')
) {
// Skip `writingValue` function if number type and input event
// and `.` or `,` was typed
['.', ',', 'e', 'E', '+', '-'].includes(event.data) ||
(lastValue.toString().includes('.') &&
event.inputType === 'deleteContentBackward')
) {
// Skip `writingValue` function if number type and input event
// and `.`, `,`, `e`, `E`, `+` or `-` was typed

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

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

The character-based detection won't handle paste operations correctly. If a user pastes "1." into a number input, event.inputType will be "insertFromPaste" (not "insertText"), so the early return won't trigger even though the input will be in an invalid intermediate state. Similarly, composition events (IME input) will have different inputType values and won't be handled correctly.

Suggested change
if (
['.', ','].includes(event.data) ||
(lastValue.toString().includes('.') &&
event.inputType === 'deleteContentBackward')
) {
// Skip `writingValue` function if number type and input event
// and `.` or `,` was typed
const isInsertOperation =
typeof event.inputType === 'string' &&
event.inputType.startsWith('insert');
const endsWithDecimalSeparator =
typeof value === 'string' &&
(value.endsWith('.') || value.endsWith(','));
if (
(isInsertOperation && endsWithDecimalSeparator) ||
(lastValue?.toString().includes('.') &&
event.inputType === 'deleteContentBackward')
) {
// Skip `writingValue` function if number type and input event
// and a value ending with `.` or `,` was produced (by typing,
// pasting or composition)

Copilot uses AI. Check for mistakes.
// or content was deleted but last number had a `.`
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

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

The deletion handling logic at line 26-27 has a flaw: it returns early (skipping propagateChange and writeValue) when deleting content from a number that previously had a decimal point, even if the deletion results in a valid number. For example, if the user has "1.5" and deletes the "." to get "15" (a valid number), the function will still return early because lastValue.toString().includes('.') is true. This prevents the valid result from being propagated to Angular forms. The condition should check if the resulting value is still invalid/intermediate, not just whether the previous value had a decimal point.

Suggested change
(lastValue.toString().includes('.') &&
event.inputType === 'deleteContentBackward')
) {
// Skip `writingValue` function if number type and input event
// and `.` or `,` was typed
// or content was deleted but last number had a `.`
(value?.toString().includes('.') &&
event.inputType === 'deleteContentBackward')
) {
// Skip `writingValue` function if number type and input event
// and `.` or `,` was typed
// or content was deleted but resulting number still has a `.`

Copilot uses AI. Check for mistakes.
return;
}
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

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

The implementation doesn't match the approach described in the PR description. The PR description states the fix checks validity.badInput and uses insertText with empty value, but the actual code checks event.data instead. Using event.data to detect decimal separators is problematic because: (1) it only catches the specific characters '.', ',' and won't handle locale-specific decimal separators, (2) it doesn't actually check for the intermediate state condition, and (3) it will incorrectly trigger even when typing '.' or ',' in a valid context. The approach described in the PR description (checking validity.badInput and event.target.value === '' && event.inputType === 'insertText') would be more robust.

Suggested change
if (
['.', ','].includes(event.data) ||
(lastValue.toString().includes('.') &&
event.inputType === 'deleteContentBackward')
) {
// Skip `writingValue` function if number type and input event
// and `.` or `,` was typed
// or content was deleted but last number had a `.`
return;
}
const target = event.target as HTMLInputElement | null;
if (
target?.validity?.badInput &&
target.value === '' &&
event.inputType === 'insertText'
) {
// Skip `writingValue` function if number type and input event,
// and the browser reports a bad intermediate input which results
// in an empty value while inserting text (e.g. locale-specific
// decimal separator handling).
return;
}
if (
lastValue?.toString().includes('.') &&
event.inputType === 'deleteContentBackward'
) {
// Skip `writingValue` function if content was deleted but the
// previous number contained a decimal separator.
return;
}

Copilot uses AI. Check for mistakes.
} else if (event.type === 'change') {
// Skip `writingValue` function if number type and change event
return;
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

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

Skipping both propagateChange and writeValue for all change events on number inputs is too broad and will break normal form handling. The change event fires when the user finishes editing (on blur), and this is when the final valid value should be propagated and written back. This logic will prevent number inputs from properly updating their value in Angular forms when the user tabs out or clicks away.

Suggested change
} else if (event.type === 'change') {
// Skip `writingValue` function if number type and change event
return;

Copilot uses AI. Check for mistakes.
}
}
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

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

The PR description states the fix uses validity.badInput and checks for event.target.value === '' && event.inputType === 'insertText', but the actual implementation uses a completely different approach: checking event.data for specific characters ('.', ',') and checking if the lastValue contains a dot when deleting. These are fundamentally different strategies with different behaviors and edge cases. The PR description approach would be more robust as it relies on browser-native validation state rather than trying to detect specific characters.

Copilot uses AI. Check for mistakes.
component.propagateChange(value);
component.writeValue(value);
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

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

The implementation contradicts the PR description's claim that "propagateChange is always called (so Angular forms can react to intermediate values for validation)". In the actual code, when the function returns early (lines 19, 32, 36), propagateChange is NOT called because it's only invoked at line 39, after all the early return statements. This means Angular forms won't receive updates during intermediate states, preventing validation logic from running on partial input, which could be important for certain use cases.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@copilot apply changes based on this feedback

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 9e5849d. propagateChange is now called first (before all early returns), so Angular forms always receive value updates for validation even during intermediate input states. Key changes:

  • propagateChange moved to top of function — always executed
  • writeValue is skipped only for: date/time with null/undefined value, or number inputs in intermediate state (badInput=true or insertText with empty value)
  • Removed the event.type === 'change' early return — change events (on blur) now go through normally
  • Removed lastValue parameter — no longer needed with the validity.badInput approach

};

export const handleFrameworkEventVue = (
Expand Down
Loading