allusion-app/Allusion

rc6.1 Tag Renaming Keypress Focus Strange Behavior

PudiccaA opened this issue · 1 comments

Describe the bug
Sorry I forgot to report this in previous issue posts. Just a small bug where keypresses lost focus when renaming a tag in the tag tree.

Here is a gif illustrating the problem:
error
As shown in the gif, I pressed left/right arrow key while I was trying to edit "AF_KURO" tag, and the focus lost, the arrow key changed the image selection instead of moving pointer in the text field.

To Reproduce
Steps to reproduce the behavior:

  1. Go to library panel, press arrow keys or use mouse to select any picture other than the first one.
  2. Go to tag panel.
  3. Double click on a tag to rename.
  4. Press left/right arrow key to move pointer.
  5. See error

Expected behavior
Here is a gif illustrating the expected behavior:
expected

Allusion version

  • 1.0.0-rc6.1

Desktop OS

  • Windows 10

Additional context
From the previous issue discussions, I think adding a e.stopPropagation(); to onKeyDown event seems to solve the problem.

const Label = (props: ILabelProps) =>
  props.isEditing ? (
    <input
      className="input"
      autoFocus
      type="text"
      defaultValue={props.text}
      onBlur={(e) => {
        const value = e.currentTarget.value.trim();
        if (value.length > 0) {
          props.setText(value);
        }
        props.onSubmit(e.currentTarget);
      }}
      onKeyDown={(e) => {
        e.stopPropagation();
        const value = e.currentTarget.value.trim();
        if (e.key === 'Enter' && value.length > 0) {
          props.setText(value);
          props.onSubmit(e.currentTarget);
        } else if (e.key === 'Escape') {
          props.onSubmit(e.currentTarget); // cancel with escape
        }
      }}
      onFocus={(e) => e.target.select()}
      // Stop propagation so that the parent Tag element doesn't toggle selection status
      onClick={(e) => e.stopPropagation()}
      // TODO: Visualizing errors...
      // Only show red outline when input field is in focus and text is invalid
    />
  ) : (
    <div className="label-text" data-tooltip={props.tooltip}>
      {props.text}
    </div>
  );

thanks again! Indeed, that does solve it.
It does hint at the bigger problem that the focus management is not very stable. The arrow-key navigation in the gallery is a global event listener, which it doesn't need to be. We tried making it a local event listener to the gallery but then had issues in certain situations where we couldn't get the gallery to be focused so the hotkeys didn't work when you would expect them to (e.g. during or after opening a popover).

Anyways, besides small issues like the one you pointed out it works pretty well, so I think we'll just fix it with another stopPropagation instead of rewriting the focus handling 👍