-
Notifications
You must be signed in to change notification settings - Fork 76
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(combobox): clear custom input value on blur #8070
fix(combobox): clear custom input value on blur #8070
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚫🚫🚫🚫🚫🚫🚫🚫🚫🚫🚫🚫🚫🚫🚫🚫🚫🚫🚫🚫🚫🚫🚫🚫🚫🚫🚫🚫
🚫🧊🚫🚫🚫🧊🚫🚫🧊🧊🚫🚫🧊🧊🧊🚫🚫🧊🧊🧊🚫🧊🧊🧊🧊🚫🧊🚫
🚫🧊🧊🚫🚫🧊🚫🧊🚫🚫🧊🚫🚫🧊🚫🚫🧊🚫🚫🚫🚫🧊🚫🚫🚫🚫🧊🚫
🚫🧊🚫🧊🚫🧊🚫🧊🚫🚫🧊🚫🚫🧊🚫🚫🧊🚫🚫🚫🚫🧊🧊🧊🚫🚫🧊🚫
🚫🧊🚫🚫🧊🧊🚫🧊🚫🚫🧊🚫🚫🧊🚫🚫🧊🚫🚫🚫🚫🧊🚫🚫🚫🚫🚫🚫
🚫🧊🚫🚫🚫🧊🚫🚫🧊🧊🚫🚫🧊🧊🧊🚫🚫🧊🧊🧊🚫🧊🧊🧊🧊🚫🧊🚫
🚫🚫🚫🚫🚫🚫🚫🚫🚫🚫🚫🚫🚫🚫🚫🚫🚫🚫🚫🚫🚫🚫🚫🚫🚫🚫🚫🚫
await page.waitForChanges(); | ||
expect(await page.evaluate(() => document.activeElement.id)).not.toBe("myCombobox"); | ||
expect(await inputEl.getProperty("value")).toBe(""); | ||
expect(await combobox.getProperty("value")).toBe(allowCustomValues ? "asdf" : ""); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick: can you use a word vs asdf
? I think the spelling linter will warn otherwise.
Applies to the test below.
@@ -160,8 +160,7 @@ export class ComboboxItem implements ConditionalSlotComponent, InteractiveCompon | |||
this.selected = !this.selected; | |||
} | |||
|
|||
itemClickHandler = (event: MouseEvent): void => { | |||
event.preventDefault(); | |||
itemClickHandler = (): void => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If possible, can you make this private? Up to you since we'll do a linting pass to fix these across the board.
expect(await inputEl.getProperty("value")).toBe(""); | ||
expect(await combobox.getProperty("value")).toBe(allowCustomValues ? "asdf" : ""); | ||
} | ||
selectionModes.forEach((mode) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick: for readability, can you add a newline above this to make it easier to scan?
🤖 I have created a release *beep* *boop* --- <details><summary>@esri/calcite-design-tokens: 1.1.0</summary> ## [1.1.0](https://github.com/Esri/calcite-design-system/compare/@esri/calcite-design-tokens@1.0.0...@esri/calcite-design-tokens@1.1.0) (2023-10-30) ### Features * Add icon tokens ([#8008](#8008)) ([3623df1](3623df1)) * Add js platform formats to calcite-design-tokens ([#8044](#8044)) ([0e1fefb](0e1fefb)) * Add xxs breakpoint to tokens ([#7992](#7992)) ([05512b6](05512b6)) * Reorganize breakpoints ([#7994](#7994)) ([c64a059](c64a059)) </details> <details><summary>@esri/calcite-components: 1.10.0</summary> ## [1.10.0](https://github.com/Esri/calcite-design-system/compare/@esri/calcite-components@1.9.2...@esri/calcite-components@1.10.0) (2023-10-30) ### Features * **block:** Ensure chevron is always displayed ([#8014](#8014)) ([95fecb2](95fecb2)) * **navigation-logo:** Adds `icon` and `iconFlipRtl` properties ([#8054](#8054)) ([049056d](049056d)) * **stepper,stepper-item:** Adds support for built-in translations ([#8002](#8002)) ([bb91624](bb91624)) ### Bug Fixes * **button, fab, inline-editable, split-button:** Prevent redundant opacity when button is both loading and disabled ([#8015](#8015)) ([3a1d3fd](3a1d3fd)) * **combobox:** Clear custom input value on blur ([#8070](#8070)) ([327ff06](327ff06)) * **combobox:** Ensure icon scales are consistent ([#8075](#8075)) ([babba3b](babba3b)) * **filter:** Corrects the accessible label ([#8069](#8069)) ([c203084](c203084)) * Floating components will now get an initial position even if they are not opened ([#8001](#8001)) ([78b680d](78b680d)) * **icon:** Use pixel sizes for icons ([#8009](#8009)) ([49085d5](49085d5)) * **input-date-picker, input-time-picker:** Adjust chevron scale accordingly ([#8012](#8012)) ([f894f80](f894f80)) * **input-time-zone:** Fix city translations ([#8058](#8058)) ([7df7e1f](7df7e1f)) * **list-item:** Restore tabbability when an item's disabled prop is toggled ([#8042](#8042)) ([c970603](c970603)) * **pagination:** Prevents console error when page-size is set to zero ([#8017](#8017)) ([d09d485](d09d485)) * **segmented-control-item:** Fix text color contrast ([#8036](#8036)) ([ede8c43](ede8c43)) * **stepper:** Selects next enabled stepper-item when first one is disabled ([#8004](#8004)) ([e0ed54e](e0ed54e)) ### Dependencies * The following workspace dependencies were updated * devDependencies * @esri/calcite-design-tokens bumped from ^1.1.0-next.3 to ^1.1.0 </details> <details><summary>@esri/calcite-components-react: 1.10.0</summary> ## [1.10.0](https://github.com/Esri/calcite-design-system/compare/@esri/calcite-components-react@1.9.2...@esri/calcite-components-react@1.10.0) (2023-10-30) ### Miscellaneous Chores * **@esri/calcite-components-react:** Synchronize undefined versions ### Dependencies * The following workspace dependencies were updated * dependencies * @esri/calcite-components bumped from ^1.10.0-next.11 to ^1.10.0 </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Related Issue: #2162
Summary
Clears input value on
blur
if the value has no matchingcombobox-item
whenallowCustomValue
property is set tofalse