Skip to content

Commit

Permalink
refactor(select): prefer click event
Browse files Browse the repository at this point in the history
  • Loading branch information
segunadebayo committed Sep 5, 2024
1 parent 3761217 commit 44628a1
Show file tree
Hide file tree
Showing 7 changed files with 63 additions and 136 deletions.
8 changes: 8 additions & 0 deletions .changeset/few-ducks-yawn.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
"@zag-js/select": patch
---

Refactor opening and selection to be based on click events rather than pointerdown/up cycles.

This improves the usability and accessibility of the select component. It also fixes an issue where controlled multiple
selects open state behaved unexpectedly.
66 changes: 19 additions & 47 deletions .xstate/select.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,10 @@ const fetchMachine = createMachine({
"!multiple": false,
"!multiple": false,
"!multiple": false,
"shouldRestoreFocus": false,
"isOpenControlled": false,
"isOpenControlled": false,
"closeOnSelect && isOpenControlled": false,
"closeOnSelect": false,
"shouldRestoreFocus && isOpenControlled": false,
"shouldRestoreFocus": false,
"isOpenControlled": false,
"hasHighlightedItem && loop && isLastItemHighlighted": false,
"hasHighlightedItem": false,
"hasHighlightedItem && loop && isFirstItemHighlighted": false,
Expand Down Expand Up @@ -81,16 +77,17 @@ const fetchMachine = createMachine({
"CONTROLLED.OPEN": [{
cond: "isTriggerClickEvent",
target: "open",
actions: ["highlightFirstSelectedItem"]
actions: ["setInitialFocus", "highlightFirstSelectedItem"]
}, {
target: "open"
target: "open",
actions: ["setInitialFocus"]
}],
"TRIGGER.CLICK": [{
cond: "isOpenControlled",
actions: ["invokeOnOpen"]
}, {
target: "open",
actions: ["invokeOnOpen", "highlightFirstSelectedItem"]
actions: ["invokeOnOpen", "setInitialFocus", "highlightFirstSelectedItem"]
}],
"TRIGGER.FOCUS": {
target: "focused"
Expand All @@ -100,35 +97,35 @@ const fetchMachine = createMachine({
actions: ["invokeOnOpen"]
}, {
target: "open",
actions: ["invokeOnOpen"]
actions: ["setInitialFocus", "invokeOnOpen"]
}]
}
},
focused: {
tags: ["closed"],
entry: ["focusTriggerEl"],
on: {
"CONTROLLED.OPEN": [{
cond: "isTriggerClickEvent",
target: "open",
actions: ["highlightFirstSelectedItem"]
actions: ["setInitialFocus", "highlightFirstSelectedItem"]
}, {
cond: "isTriggerArrowUpEvent",
target: "open",
actions: ["highlightComputedLastItem"]
actions: ["setInitialFocus", "highlightComputedLastItem"]
}, {
cond: "isTriggerArrowDownEvent || isTriggerEnterEvent",
target: "open",
actions: ["highlightComputedFirstItem"]
actions: ["setInitialFocus", "highlightComputedFirstItem"]
}, {
target: "open"
target: "open",
actions: ["setInitialFocus"]
}],
OPEN: [{
cond: "isOpenControlled",
actions: ["invokeOnOpen"]
}, {
target: "open",
actions: ["invokeOnOpen"]
actions: ["setInitialFocus", "invokeOnOpen"]
}],
"TRIGGER.BLUR": {
target: "idle"
Expand All @@ -138,28 +135,28 @@ const fetchMachine = createMachine({
actions: ["invokeOnOpen"]
}, {
target: "open",
actions: ["invokeOnOpen", "highlightFirstSelectedItem"]
actions: ["setInitialFocus", "invokeOnOpen", "highlightFirstSelectedItem"]
}],
"TRIGGER.ENTER": [{
cond: "isOpenControlled",
actions: ["invokeOnOpen"]
}, {
target: "open",
actions: ["invokeOnOpen", "highlightComputedFirstItem"]
actions: ["setInitialFocus", "invokeOnOpen", "highlightComputedFirstItem"]
}],
"TRIGGER.ARROW_UP": [{
cond: "isOpenControlled",
actions: ["invokeOnOpen"]
}, {
target: "open",
actions: ["invokeOnOpen", "highlightComputedLastItem"]
actions: ["setInitialFocus", "invokeOnOpen", "highlightComputedLastItem"]
}],
"TRIGGER.ARROW_DOWN": [{
cond: "isOpenControlled",
actions: ["invokeOnOpen"]
}, {
target: "open",
actions: ["invokeOnOpen", "highlightComputedFirstItem"]
actions: ["setInitialFocus", "invokeOnOpen", "highlightComputedFirstItem"]
}],
"TRIGGER.ARROW_LEFT": [{
cond: "!multiple && hasSelectedItems",
Expand Down Expand Up @@ -191,24 +188,19 @@ const fetchMachine = createMachine({
},
open: {
tags: ["open"],
entry: ["setInitialFocus"],
exit: ["scrollContentToTop"],
activities: ["trackDismissableElement", "computePlacement", "scrollToHighlightedItem"],
on: {
"CONTROLLED.CLOSE": [{
cond: "shouldRestoreFocus",
"CONTROLLED.CLOSE": {
target: "focused",
actions: ["clearHighlightedItem"]
}, {
target: "idle",
actions: ["clearHighlightedItem"]
}],
actions: ["focusTriggerEl", "clearHighlightedItem"]
},
CLOSE: [{
cond: "isOpenControlled",
actions: ["invokeOnClose"]
}, {
target: "focused",
actions: ["invokeOnClose", "clearHighlightedItem"]
actions: ["invokeOnClose", "focusTriggerEl", "clearHighlightedItem"]
}],
"TRIGGER.CLICK": [{
cond: "isOpenControlled",
Expand All @@ -227,24 +219,6 @@ const fetchMachine = createMachine({
}, {
actions: ["selectHighlightedItem"]
}],
"CONTENT.INTERACT_OUTSIDE": [
// == group 1 ==
{
cond: "shouldRestoreFocus && isOpenControlled",
actions: ["invokeOnClose"]
}, {
cond: "shouldRestoreFocus",
target: "focused",
actions: ["invokeOnClose", "clearHighlightedItem"]
},
// == group 2 ==
{
cond: "isOpenControlled",
actions: ["invokeOnClose"]
}, {
target: "idle",
actions: ["invokeOnClose", "clearHighlightedItem"]
}],
"CONTENT.HOME": {
actions: ["highlightFirstItem"]
},
Expand Down Expand Up @@ -299,10 +273,8 @@ const fetchMachine = createMachine({
"isTriggerArrowDownEvent || isTriggerEnterEvent": ctx => ctx["isTriggerArrowDownEvent || isTriggerEnterEvent"],
"!multiple && hasSelectedItems": ctx => ctx["!multiple && hasSelectedItems"],
"!multiple": ctx => ctx["!multiple"],
"shouldRestoreFocus": ctx => ctx["shouldRestoreFocus"],
"closeOnSelect && isOpenControlled": ctx => ctx["closeOnSelect && isOpenControlled"],
"closeOnSelect": ctx => ctx["closeOnSelect"],
"shouldRestoreFocus && isOpenControlled": ctx => ctx["shouldRestoreFocus && isOpenControlled"],
"hasHighlightedItem && loop && isLastItemHighlighted": ctx => ctx["hasHighlightedItem && loop && isLastItemHighlighted"],
"hasHighlightedItem": ctx => ctx["hasHighlightedItem"],
"hasHighlightedItem && loop && isFirstItemHighlighted": ctx => ctx["hasHighlightedItem && loop && isFirstItemHighlighted"]
Expand Down
11 changes: 0 additions & 11 deletions e2e/select.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,17 +40,6 @@ test.describe("pointer", () => {
await I.seeTriggerHasText("Select option")
})

test("should open and select with pointer cycle", async () => {
const albania = "Albania"

await I.pointerDownTrigger()
await I.hoverItem(albania)
await I.pointerUpItem(albania)

await I.seeItemIsChecked(albania)
await I.seeTriggerHasText(albania)
})

test("should highlight on hover", async () => {
await I.clickTrigger()
await I.hoverItem("Albania")
Expand Down
2 changes: 1 addition & 1 deletion examples/next-ts/pages/compositions
28 changes: 9 additions & 19 deletions packages/machines/select/src/select.connect.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { getEventKey, isLeftClick, type EventKeyMap } from "@zag-js/dom-event"
import { getEventKey, type EventKeyMap } from "@zag-js/dom-event"
import {
ariaAttr,
dataAttr,
Expand Down Expand Up @@ -166,19 +166,11 @@ export function connect<T extends PropTypes, V extends CollectionItem = Collecti
"data-readonly": dataAttr(readOnly),
"data-placement": state.context.currentPlacement,
"data-placeholder-shown": dataAttr(!state.context.hasSelectedItems),
onPointerDown(event) {
if (!isLeftClick(event)) return
onClick(event) {
if (!interactive) return
event.currentTarget.dataset.pointerType = event.pointerType
if (disabled || event.pointerType === "touch") return
if (event.defaultPrevented) return
send({ type: "TRIGGER.CLICK" })
},
onClick(event) {
if (!interactive || event.button) return
if (event.currentTarget.dataset.pointerType === "touch") {
send({ type: "TRIGGER.CLICK" })
}
},
onFocus() {
send("TRIGGER.FOCUS")
},
Expand Down Expand Up @@ -267,23 +259,21 @@ export function connect<T extends PropTypes, V extends CollectionItem = Collecti
if (itemState.value === state.context.highlightedValue) return
send({ type: "ITEM.POINTER_MOVE", value: itemState.value })
},
onPointerUp() {
onClick(event) {
if (event.defaultPrevented) return
if (itemState.disabled) return
send({ type: "ITEM.CLICK", src: "pointerup", value: itemState.value })
},
onPointerLeave(event) {
if (itemState.disabled) return
if (props.persistFocus) return
if (event.pointerType !== "mouse") return
const isArrowKey = ["CONTENT.ARROW_UP", "CONTENT.ARROW_DOWN"].includes(state.event.type)
if (isArrowKey) return

const pointerMoved = state.previousEvent.type.includes("POINTER")
if (!pointerMoved) return

send({ type: "ITEM.POINTER_LEAVE" })
},
onTouchEnd(event) {
// prevent clicking elements behind content
event.preventDefault()
event.stopPropagation()
},
})
},

Expand Down
Loading

0 comments on commit 44628a1

Please sign in to comment.