Skip to content

Commit

Permalink
listbox: fix over-aggressive focus when triggered by click
Browse files Browse the repository at this point in the history
  • Loading branch information
Chance Strickland committed Mar 19, 2020
1 parent 180752f commit 28d3fed
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 5 deletions.
2 changes: 1 addition & 1 deletion packages/descendants/src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ export function useDescendantKeyDown<
break;
case "End":
event.preventDefault();
let last = getFirstOption();
let last = getLastOption();
callback(key === "option" ? last : last[key]);
break;
}
Expand Down
1 change: 0 additions & 1 deletion packages/listbox/src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -523,7 +523,6 @@ export const ListboxButton = forwardRefWithAs<ListboxButtonProps, "span">(
function handleMouseDown(event: React.MouseEvent) {
if (!isRightClick(event.nativeEvent)) {
mouseEventStartedRef.current = true;
event.persist();
event.preventDefault();
send({
type: ListboxEvents.ButtonMouseDown,
Expand Down
12 changes: 9 additions & 3 deletions packages/listbox/src/machine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ function selectOption(data: ListboxStateData, event: any) {
}

function submitForm(data: ListboxStateData, event: any) {
if (event.type !== ListboxEvents.KeyDownEnter || event.disabled) {
if (event.type !== ListboxEvents.KeyDownEnter) {
return;
}

Expand Down Expand Up @@ -297,6 +297,13 @@ let openEvents = {
},
[ListboxEvents.ButtonMouseDown]: {
target: ListboxStates.Idle,
// When the user triggers a mouseDown event on the button, we call
// event.preventDefault() because the browser will naturally call mouseUp
// and click, which will reopen the button (which we don't want). As such,
// the click won't blur the open list or re-focus the trigger, so we call
// `focusButton` to do that manually. We could work around this with
// deferred transitions with xstate, but @xstate/fsm currently doesn't
// support that feature and this works good enough for the moment.
actions: [focusButton],
},
[ListboxEvents.KeyDownEscape]: {
Expand Down Expand Up @@ -339,7 +346,7 @@ export const createMachineDefinition = ({
...commonEvents,
[ListboxEvents.ButtonMouseDown]: {
target: ListboxStates.Navigating,
actions: [navigateFromCurrentValue, focusButton],
actions: [navigateFromCurrentValue],
cond: listboxIsNotDisabled,
},
[ListboxEvents.KeyDownSpace]: {
Expand Down Expand Up @@ -479,7 +486,6 @@ export const createMachineDefinition = ({
},
},
[ListboxStates.NavigatingWithKeys]: {
// entry: focusList,
on: {
...commonEvents,
...openEvents,
Expand Down

0 comments on commit 28d3fed

Please sign in to comment.