feat(evo-marko): evo-combobox#661
Open
LuLaValva wants to merge 2 commits into
Open
Conversation
🦋 Changeset detectedLatest commit: 46b94a9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a new evo-combobox tag to @evo-web/marko, including styling, Storybook docs/examples, and SSR/browser tests.
Changes:
- Introduces
evo-comboboxMarko implementation, styles, README, and Storybook stories/examples (including async filtering via JSON data). - Adds SSR snapshot coverage and browser interaction tests for core combobox behaviors.
- Updates TypeScript config to support JSON module imports and includes JSON in compilation scope.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/evo-marko/tsconfig.json | Enables JSON module support for Storybook/examples and includes JSON files in TS scope. |
| packages/evo-marko/src/tags/evo-combobox/index.marko | Implements the combobox UI, filtering, keyboard/mouse interactions, and markup/ARIA. |
| packages/evo-marko/src/tags/evo-combobox/style.ts | Adds Skin CSS imports required by the new combobox and related UI elements. |
| packages/evo-marko/src/tags/evo-combobox/combobox.stories.ts | Adds Storybook stories and controls for the new component. |
| packages/evo-marko/src/tags/evo-combobox/README.md | Documents usage, props, and attribute tags for evo-combobox. |
| packages/evo-marko/src/tags/evo-combobox/examples/*.marko | Adds examples (default, controllable, async filtering) used by Storybook. |
| packages/evo-marko/src/tags/evo-combobox/examples/data.json | Adds example dataset used by async filtering example. |
| packages/evo-marko/src/tags/evo-combobox/test/test.server.ts | Adds SSR snapshot tests for multiple variants. |
| packages/evo-marko/src/tags/evo-combobox/test/test.browser.ts | Adds browser interaction tests (focus, expand/collapse, keyboard, click, filtering). |
| packages/evo-marko/src/tags/evo-combobox/test/snapshots/test.server.ts.snap | Adds SSR snapshots. |
| CLAUDE.md | Adds Marko 6 guidance notes for extractor and event handler typing patterns. |
| .changeset/bright-socks-care.md | Publishes a patch changeset for @evo-web/marko. |
Comment on lines
+150
to
+157
| } else if (e.key === "Enter") { | ||
| if (open && activeIndex >= 0) { | ||
| e.preventDefault(); | ||
| value = displayValue ?? visibleOptions[activeIndex].text; | ||
| displayValue = null; | ||
| } | ||
| open = false; | ||
| } else if (e.key === "Escape") { |
| optionClass, | ||
| ] | ||
| tabindex="-1" | ||
| aria-selected=((optionValue || text) === value) && "true" |
Comment on lines
+204
to
+209
| onClick() { | ||
| value = text; | ||
| displayValue = null; | ||
| activeIndex = i; | ||
| open = false; | ||
| }> |
| fluid?: boolean; | ||
| strategy?: "fixed" | "absolute"; | ||
| open?: boolean; | ||
| openChange?: (open: boolean) => void; |
Comment on lines
+91
to
+105
| <input | ||
| ...htmlInput | ||
| id=inputId | ||
| type="text" | ||
| role="combobox" | ||
| disabled=disabled | ||
| value:=value | ||
| placeholder=showPlaceholder && placeholder | ||
| autocomplete="off" | ||
| aria-autocomplete=(autocomplete === "list" ? "list" : "none") | ||
| aria-haspopup="listbox" | ||
| aria-expanded=open ? "true" : "false" | ||
| aria-activedescendant=((open && activeIndex >= 0) | ||
| ? `${inputId}-option-${activeIndex}` | ||
| : undefined) |
| optionClass, | ||
| ] | ||
| tabindex="-1" | ||
| aria-selected=((optionValue || text) === value) && "true" |
| activeIndex = -1; | ||
| if (input.open !== false) open = true; | ||
| } | ||
| onKeyDown(e: KeyboardEvent, target: HTMLInputElement) { |
| "verbatimModuleSyntax": true, | ||
| "resolveJsonModule": true | ||
| }, | ||
| "exclude": ["**/*test*/**/*", "**/*.stories.ts"], |
| } else if (e.key === "Enter") { | ||
| if (open && activeIndex >= 0) { | ||
| e.preventDefault(); | ||
| value = displayValue ?? visibleOptions[activeIndex].text; |
| e.preventDefault(); | ||
| } | ||
| onClick() { | ||
| value = text; |
| onBlur, | ||
| onFocus, | ||
| onKeyDown, | ||
| placeholder, |
Member
There was a problem hiding this comment.
Do we need to remove open and openChange as we did in the other component
| <let/value:=input.value> | ||
| <let/open:=input.open> | ||
| <let/activeIndex=(-1)> | ||
| <let/displayValue=(null as string | null)> |
Member
There was a problem hiding this comment.
Is this also a valid Marko syntax? I think using as it is usually not typesafe
Suggested change
| <let/displayValue=(null as string | null)> | |
| <let/displayValue: string | null =null> |
| }> | ||
| <if=button> | ||
| <button | ||
| ...button |
Member
There was a problem hiding this comment.
Can't we use the evo-icon-button component here instead or it is not necessary?
| </evo-combobox> | ||
| ``` | ||
|
|
||
| ## Props |
Member
There was a problem hiding this comment.
In evo-react we are moving away from documentation in the README to storybook only and link to storybook on the readme. Reason is that it is hard to maintain this list updated most of the time. In evo-marko we are using the README as the documentation, but we also have the args from storybook so it will duplicate information I think.
| @@ -0,0 +1,3 @@ | |||
| import "@ebay/skin/combobox"; | |||
| import "@ebay/skin/icon-button"; | |||
| import "@ebay/skin/floating-label"; | |||
Member
There was a problem hiding this comment.
This one shouldn't be necessary now that is imported by evo-floating-label component
Suggested change
| import "@ebay/skin/floating-label"; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
<evo-combobox>to@evo-web/markoScreenshots