adobe / react-spectrum

A collection of libraries and tools that help you build adaptive, accessible, and robust user experiences.
https://react-spectrum.adobe.com
Apache License 2.0
12.59k stars 1.09k forks source link

Input field inside GridList #4674

Open tflanagan opened 1 year ago

tflanagan commented 1 year ago

🐛 Bug Report

When using ListBox, if you have an <input type="text" /> element, when typing in the input field, space characters are being ignored.

🤔 Expected Behavior

Typing in the input field would work as normal without keyboard presses being intercepted.

😯 Current Behavior

When pressing the space bar with the <input> focused, ListBox assumes I am selecting the option row as opposed to typing into the input field.

💁 Possible Solution

1) Detect current target of the event vs target of the state of selection manager and ensure they match, or 2) Detect we are inside a user input field and ignore selection management

🔦 Context

DND Sortable list with user input

💻 Code Sample

https://codesandbox.io/s/great-sutherland-cqksy8?file=/src/App.js

Focus any of the <input type="text" /> fields, type some random characters, then try typing a space. The space will be ignored and if you look at the dom element, you'll see the data-pressed=true prop has been set.

devongovett commented 1 year ago

Interactive elements such as textfields are not allowed inside a listbox. The ARIA spec only allows options, and options are marked as "Children Presentational" which means that any child elements cannot be accessed by screen readers. You should use a GridList instead, which has been specifically designed to allow interactive children.

However, I think in both cases text fields won't really work correctly because the arrow keys are used by the row for navigation. Therefore when you get into a textfield you'd be in a keyboard trap. For this we need to implement edit mode, so the user could temporarily disable keyboard navigation by pressing Enter, and when they are done editing press Escape to return back to the row. See #2328.

tflanagan commented 1 year ago

That makes sense.

Thanks for the quick and detailed response.

tflanagan commented 1 year ago

I've updated the sandbox to use a <GridList />, but as you suspected, I'm experiencing the same issue:

https://codesandbox.io/s/great-sutherland-cqksy8?file=/src/App.js

piecyk commented 1 year ago

@tflanagan did you solve the issue? Any hints? struggling with exactly same thing, having inputs in GridList

tflanagan commented 1 year ago

No, I'm still having this issue with the nightly builds. Yes, with GridList too.

I have a monkey patch I'm using to add various things, but nothing has solved it 100%.

piecyk commented 1 year ago

@tflanagan also trying the prevent keyboard events but it's bit tricky, can't pin point what is handling pressing space 🤔

piecyk commented 1 year ago

Ok, it was useTypeSelect 😄.

fyi for now will just stop propagation of keydown events when input is focused.

const preventKeyDownRef = useRef(false)
useEffect(() => {
  const handler = (e: KeyboardEvent) => {
    if (preventKeyDownRef.current) {
      e.stopPropagation()
    }
  }
  window.addEventListener('keydown', handler, true)
  return () => {
    window.removeEventListener('keydown', handler, true)
  }
}, [])

const prevent = (should: boolean) => (preventKeyDownRef.current = should)
const preventProps = {
  onFocus: () => prevent(true),
  onBlur: () => prevent(false),
}
tflanagan commented 1 year ago

@tflanagan also trying the prevent keyboard events but it's bit tricky, can't pin point what is handling pressing space 🤔

That's were I ended up.

Ok, it was useTypeSelect 😄.

fyi for now will just stop propagation of keydown events when input is focused.

Oh, nice!

I'm unsure if this issue should be reopened or if you should add that bit to #2328

tflanagan commented 9 months ago

I'm reopening this so I don't lose track of this issue.

janvorwerk commented 6 months ago

Ok, it was useTypeSelect 😄.

fyi for now will just stop propagation of keydown events when input is focused.

Thank you so much for your inspiration @piecyk! 🙏

I tweaked your solution a bit to have an "edit mode" as suggested by #2328

export function useCellEditMode() {
  const isFocusedRef = useRef(false);
  const isEditModeRef = useRef(false);

  useEffect(() => {
    const handler = (e: KeyboardEvent) => {
      if (isFocusedRef.current) {
        if (e.code === "Enter") {
          isEditModeRef.current = true;
        } else if (e.code === "Escape" || e.code === "Tab") {
          isEditModeRef.current = false;
        }
        if (isEditModeRef.current) {
          e.stopPropagation();
        }
      }
    };
    // capture all events on 'window' because we are in the capture phase
    window.addEventListener("keydown", handler, true);
    return () => {
      window.removeEventListener("keydown", handler, true);
    };
  }, []);

  const setFocus = (should: boolean) => (isFocusedRef.current = should);

  const preventProps = {
    onFocus: () => setFocus(true),
    onBlur: () => setFocus(false),
    onClick: () => {
      // We did not focus through keyboard nav, so we can
      // enter the edit mode right away.
      isEditModeRef.current = true;
    },
  };
  return preventProps;
}
tflanagan commented 6 months ago

@janvorwerk That worked beautifully! Using this with any inside a gridlist solves the issue