Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix suggestions when using select all #1422

Closed
wants to merge 3 commits into from

Conversation

Massi-X
Copy link
Contributor

@Massi-X Massi-X commented Dec 3, 2024

Hi @yairEO this should be my last PR, take your time to review and/or improve them and ask me if you need any clarifications!

To trigger this bug (mode is select with whitelist):

  1. Select a value
  2. Close the dropdown
  3. Tab into the input
  4. Select all (ctrl+A/cmd+A)
  5. Write something (the beginning of another value)
  6. No suggestion shows up

I think this happens because the focus is not on the input but rather on the tagText element, fixed the behavior by adding another case to onWindowKeyDown handler that calls the onInput handler.

1.mov

Tab into the input, select all and start to write something, no suggestion shows up
break;

// remove tag if has focus
case 'Backspace': {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved here because it needs to continue into default (to call on Input if relevant)

@yairEO
Copy link
Owner

yairEO commented Dec 21, 2024

Hi, I've carefully investigated this bug and had made a fix which is very different than yours.
This requires very deep knowledge in Tagify's code which only I possess :)

@yairEO yairEO closed this Dec 21, 2024
yairEO added a commit that referenced this pull request Dec 21, 2024
@Massi-X
Copy link
Contributor Author

Massi-X commented Dec 22, 2024

No problem, you're great thanks!

@yairEO
Copy link
Owner

yairEO commented Dec 22, 2024

@Massi-X - How do you keep your Tagify package updated (NPM) if you are manually patching it?

@Massi-X
Copy link
Contributor Author

Massi-X commented Dec 22, 2024

It's embedded, I'm not using NPM!

@yairEO
Copy link
Owner

yairEO commented Dec 22, 2024

OK but still, how to you manage to merge the my updates with your changes... it seems like a very delicate work which can get frustrating

@Massi-X
Copy link
Contributor Author

Massi-X commented Dec 22, 2024

Yep indeed I'm waiting for all the fixes to be implemented 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants