Skip to content
Draft
Show file tree
Hide file tree
Changes from 2 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
2 changes: 1 addition & 1 deletion packages/components/playwright.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,6 @@ export default defineConfig({
url: TEST_URL,
reuseExistingServer: false,
/* The builtin Stencil server sometimes fails to serve some assets which leads to intermittent test failures. Use a more stable server (without watcher) for CI: */
...(process.env.CI ? { command: `stencil build --dev && mv dist/kolibri dist/build && npx serve dist -p ${TEST_PORT} -L` } : {}),
command: `stencil build --dev && mv dist/kolibri dist/build && npx serve dist -p ${TEST_PORT} -L`,
},
});
19 changes: 17 additions & 2 deletions packages/components/src/components/single-select/shadow.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,7 @@ export class KolSingleSelect implements SingleSelectAPI, FocusableElement {
_variant="ghost"
_disabled={isDisabled}
class="kol-single-select__delete"
data-testid="single-select-delete"
hidden={isDisabled}
_on={{
onClick: (event: MouseEvent) => {
Expand Down Expand Up @@ -440,8 +441,6 @@ export class KolSingleSelect implements SingleSelectAPI, FocusableElement {
);
}



@Listen('keydown')
public handleKeyDown(event: KeyboardEvent) {
const handleEvent = (isOpen?: boolean, callback?: () => void): void => {
Expand Down Expand Up @@ -712,6 +711,15 @@ export class KolSingleSelect implements SingleSelectAPI, FocusableElement {
public validateOptions(value?: OptionsPropType): void {
this.controller.validateOptions(value);
this._filteredOptions = value;
// Check if current value is still valid in the new options
if (this._value !== null && Array.isArray(value)) {
const valueExists = value.some((option) => option.value === this._value);
if (!valueExists) {
this._value = null;
this._inputValue = '';
this.controller.setFormAssociatedValue(null);
}
}
Comment thread
deleonio marked this conversation as resolved.
Outdated
if (this._isOpen) {
this.setFilteredOptionsByQuery(this._inputValue);
} else {
Expand Down Expand Up @@ -794,6 +802,13 @@ export class KolSingleSelect implements SingleSelectAPI, FocusableElement {
if (!Array.isArray(this._options)) {
return;
}

// Normalize undefined to null
if (value === undefined) {
value = null;
this._value = null;
Comment on lines +809 to +810
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The normalization of undefined to null causes this._value to be modified, which will re-trigger the @Watch('_value') decorator. The current implementation continues to execute the rest of the function with the new null value, which is redundant because the function will be called again by the watcher.

To make the flow clearer and avoid redundant execution, you can simply set this._value = null and then return from the if block.

Suggested change
value = null;
this._value = null;
this._value = null;
return;

}

if (this.isSelectionCleared && value === null) {
this._inputValue = '';
this._filteredOptions = [...this.state._options];
Expand Down
6 changes: 4 additions & 2 deletions packages/components/src/components/toolbar/shadow.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import type { OrientationPropType } from '../../schema/props/orientation';
import { validateOrientation } from '../../schema/props/orientation';
import { delegateClick, setClick } from '../../utils/element-click';
import { delegateFocus, setFocus } from '../../utils/element-focus';
import { delegateFocus } from '../../utils/element-focus';

@Component({
tag: 'kol-toolbar',
Expand Down Expand Up @@ -36,7 +36,9 @@
public async focus(): Promise<void> {
const firstEnabledItem = this.indexToElement.get(this.currentIndex);
if (firstEnabledItem) {
return delegateFocus(this.host!, () => setFocus(firstEnabledItem));
return delegateFocus(this.host!, async () => {
await (firstEnabledItem as any).focus?.();

Check failure on line 40 in packages/components/src/components/toolbar/shadow.tsx

View workflow job for this annotation

GitHub Actions / build-and-check

Unexpected any. Specify a different type

Check failure on line 40 in packages/components/src/components/toolbar/shadow.tsx

View workflow job for this annotation

GitHub Actions / build-and-check

Unsafe call of an `any` typed value
Comment thread
deleonio marked this conversation as resolved.
Outdated
});
}
}

Expand Down
Loading