Fix submit button being focused on an implicit form submission (useButton)

[sookmax]

Closes https://github.com/adobe/react-spectrum/issues/5940

I'm not sure this is the direction @snowystinger wanted in https://github.com/adobe/react-spectrum/issues/5940#issuecomment-1963071411, but I found it rather difficult to implement the fix inside the usePress() hook. I ended up placing the fix in useButton() instead.

I think it's a reasonable fix, but I might be missing something so any comments, suggestions, and opinions are welcome!

P.S. I wanted to add a unit test on this implementation, but realized jsdom didn't support HTMLFormElement.prototype.requestSubmit, which is needed for the implicit form submission test.

✅ Pull Request Checklist:

• [x] Included link to corresponding React Spectrum GitHub Issue.
• [x] Added/updated unit tests and storybook for this change (for new code or code which already has tests).
• [x] Filled out test instructions.
• [ ] Updated documentation (if it already exists for this component).
• [ ] Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

Story

http://localhost:9003/?path=/story/usebutton--submit-type-button&providerSwitcher-express=false&strict=true

https://github.com/adobe/react-spectrum/assets/71210554/f2d0c67e-2749-44a5-aebc-199de8a3b2b2

🧢 Your Project:

[sookmax]

@snowystinger Thanks for the detailed comment. I like the workaround you suggested!

I also like the idea of useForm(), and if I'm not mistaken it sounds like something that can also be implemented in the client code right?

So, you're talking about something like the following?

<form
  onKeyUp={e => {
    if (e.key === 'Enter' && e.target !== submitButtonRef.current) {
      (e.target as HTMLElement).focus();
    }
}}>
  <input ref={setInputRef} type="text" />
  <button {...buttonProps} ref={ref}>
    Submit
  </button>
</form>

Hm, a quick test on the story shows this wouldn't work because by the time onKeyUp() on the form catches the event, it seems like e.target has already been assigned to the submit button even though the 'Enter' was pressed while the input was being focused (weird). Also it looks like attaching an event listener to a non-interactive element like <form> is considered bad practice? I'm seeing jsx-a11y/no-noninteractive-element-interactions lint warning as soon as I added onKeyUp to the form.

Ideally, virtual clicks from VO or input labels could be distinguished from implicit form submission. There may be others that I'm unaware of.

I think you probably already know about this, but I've found clicking the button with a trackpad (on Macbook) is considered as 'virtual' (state.pointerType) while clicking the button with a normal mouse would produce state.pointerType of null.

[snowystinger]

That warning should be a moot point if we pass it as we usually do with hooks. I was thinking more along these lines: (though I'm still not sure this is actually a better solution)

let lastFocused = useRef();
<form
  onSubmit={() => lastFocused.current?.focus();}
  onKeyUpCapture={e => {
    // could also listen to the onFocus/onBlur to track the lastFocused instead of the key events
    if (e.key === 'Enter' && e.target !== submitButtonRef.current) {
      lastFocused.current = e.target
    }
}}>

I think you probably already know about this, but I've found clicking the button with a trackpad (on Macbook) is considered as 'virtual' (state.pointerType) while clicking the button with a normal mouse would produce state.pointerType of null.

Huh, I was not seeing this, my trackpad returns mouse. I'm not sure what settings would change that

[sookmax]

Hm, seems like using onKeyCapture to capture e.target that is not the submit button also doesn't work when I did a quick test on the story. By the time the event handler is called, e.target is, again, already the submit button even though the 'Enter' was pressed on the input.

Huh, I was not seeing this, my trackpad returns mouse. I'm not sure what settings would change that

Haha sorry about that. I always turn on 'tap to click' setting whenever I use a Macbook:

And that 'tap' as a click is recognized as a 'virtual' state.pointerType.

[yihuiliao]

I just wanted to let you know that the team will be slow to review as we address some other priorities at the moment. We appreciate your understanding and apologize for the wait.

[sookmax]

@yihuiliao haha it's okay! Thanks for letting me know 😄.

[LFDanLu]

Fixed by https://github.com/adobe/react-spectrum/pull/7542