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:
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:
- Go to library panel, press arrow keys or use mouse to select any picture other than the first one.
- Go to tag panel.
- Double click on a tag to rename.
- Press left/right arrow key to move pointer.
- See error
Expected behavior
Here is a gif illustrating the expected behavior:
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