search

ozhanefemeral / visual-ts

Generate React & TypeScript code visually.
https://visual-ts.vercel.app
11 stars 2 forks source link

Key combinations in SearchDialog component not working on Mac #22

Closed ozhanefemeral closed 2 months ago

ozhanefemeral commented 3 months ago

Is your feature request related to a problem? Please describe.

Combining ⌥ + any numbers from 1 to 9 should be selecting a function from the list and add it to the CodeGeneratorContext. This works flawlessly on Windows - Chrome but not on Mac - Chrome.

image

Describe the solution you'd like

Create a solution that is OS/Browser agnostic and consistent. Refactor all keyboard event handlers within SearchDialog based on that solution. Feel free to use another library

Feel free to share your thoughts or create a PR for this issue 🤞

AhmetSBulbul commented 2 months ago
image

Using option key modifies the value of event.key, so instead of using option we can use control key or another solution is using code value instead of key. For example the code always will be Digit1 but the key can be "1" or "i" or something else, removing the Digit prefix will solve the issue but in searchable lists field is updated with the value instead of executing shortcut.

I'm gonna open PR with using control key solution.

I'm adding the "using event.code instead of key and clear the Digit prefix" solution here as a referance if anyone needs to use altKey for shortcut later.

image image
ozhanefemeral commented 2 months ago

Hey @AhmetSBulbul 👋

Thank you for the heads up.

Isn't CTRL key used for switching tabs on Windows devices? CTRL + 1-9 key combination is reserved for this purpose as far as I know.

Maybe we use CTRL for Mac and Alt on Windows, or use the other solution you offered. That would be great if you can test that behavior on Windows or open a branch/draft PR for me to test

Cheers

AhmetSBulbul commented 2 months ago

Okay, right now it's optional, both of them are working. I updated the PR as a draft. If we don't have more complex use case I'm gonna just create new Component to show the right key according to OS. I don't have a windows setup to test right now but I haven't changed the logic, I just added a "or" statement so it must be working as expected. But if you have a chance to test you can let me know if there is an any problem.

ozhanefemeral commented 2 months ago

Let's keep this branch only for fixing the bug.

That would be great if you can create a new issue for showing the right key enhancement if you have time. I can also create the issue next week. we can implement showing the right key with:

  1. using/creating a custom hook for getting OS details, and call that in KeyCombinationLabel
  2. add a util function, and call that in KeyCombinationLabel
  3. Or other ways?

and here is how you can change User Agent from Chrome Dev Tools: https://developer.chrome.com/docs/devtools/device-mode/override-user-agent

So you can change Firefox/Opera/Chrome and Mac/Windows while working👍

AhmetSBulbul commented 2 months ago

Thanks for the trick. I'm gonna create an issue as soon as possible. If you have a chance to open before me, you can mention me in the comment.

ozhanefemeral commented 2 months ago

Closing this issue as completed thanks to PR by @AhmetSBulbul 💪