search

fregante / GhostText

👻 Use your text editor to write in your browser. Everything you type in the editor will be instantly updated in the browser (and vice versa).
https://GhostText.fregante.com
MIT License
3.31k stars 117 forks source link

Don't ask for textarea if there is one already selected #175

Closed luisherranz closed 3 years ago

luisherranz commented 3 years ago

First of all, thanks for this wonderful extension! The concept is so simple and yet so powerful.

The new codebase (https://github.com/GhostText/GhostText/issues/172) is working great for me in Chrome + VimR.

But in applications with more than one textarea (like a search), the need to select the correct one with the mouse is annoying. It kind of ruins the purpose of having a keyboard shortcut. In my opinion, it makes sense that if you have already selected a textarea, you don't have to select it again.

If you don't want to make this the default, then please consider adding an option 🙂

fregante commented 3 years ago

Hi, both PRs are currently not passing linting. You can probably fix some errors by running this locally: npx xo --fix

luisherranz commented 3 years ago

You can probably fix some errors by running this locally: npx xo --fix

Done 👍

fregante commented 3 years ago

I found the commit that dropped this feature: https://github.com/luisherranz/GhostText/commit/c87caa95af5936f2afca2c0224ad7e93ea9f3a3d

And before that, there was additional logic: https://github.com/luisherranz/GhostText/blame/cd136f31a55f52d572defcaeec32143cddacab3b/browser/scripts/content.js#L127

I think the problem was: What happens when the field is already activated?

  1. Should the "activate" command be ignored? This is what happens on this PR I think. Not good.
  2. Should we let the user activate another field? I think that's what happened before the commit I mentioned
  3. Should the field be deactivated? This would be a solution to #178, but it becomes awkward if you're trying to activate multiple fields. Example:
    1. Focus textarea
    2. Activate GhostText
    3. The textarea is automatically connected
    4. Activate GhostText to connect another textarea
    5. Oops, the previous textarea was still focused, so now we have to start again

cc @subnut if you have any ideas

subnut commented 3 years ago

I actually didn't even know that there was a keyboard shortcut available :sweat_smile:

If possible, I think such behavior is best suited to a separate keybinding......

....OR, we could make a different function that does something like this -

gt2

and make the keyboard shortcut trigger this new function. (Clicking the icon shall retain the old function, though) So, both keyboard users and mouse users shall be happy(?)

Does @luisherranz agree with this approach?

https://github.com/GhostText/GhostText/blob/261c838a8205e8fbb8cf0f5c2d3520796596431c/browser/scripts/content.js#L223

fregante commented 3 years ago

If possible, I think such behavior is best suited to a separate keybinding......

Are you suggesting that the current situation works best as is?

....OR, we could make a different function that does something like this -

That'd be nice, but correctly locating the fields and attaching an element like that sounds problematic. I'd like to keep GhostText as lean as possible because I don't really have time to maintain it — as you may have noticed 😅

subnut commented 3 years ago

Are you suggesting that the current situation works best as is?

As I said, I didn't even know that the keybinding existed, so I have zero experience about working with it... so I don't really want to suggest anything..

Maybe other users who use the keyboard shortcuts should comment? I think that taking the comments of other users is necessary before changing anything...

fregante commented 3 years ago

As I said, I didn't even know that the keybinding existed, so I have zero experience about working with it... so I don't really want to suggest anything..

This is unrelated to the keyboard shortcut, this also applies to just clicking the button. Before v21, I think GhostText used to activate with just one click if the field was selected, or at least that's what the intention was.

luisherranz commented 3 years ago

Should the "activate" command be ignored?

I'm happy with that to be honest. The times I used the shortcut again in a field that was already activated were by mistake and ignoring it was the expected behavior.

Maybe it would be enough to give the user that information in the alert. Something like: "This field is already activated, please choose another one".