Refined-github: Escape key canceling comments is conflicting with closing the emoji menu

[ewjoachim]

I'm talking about this code :
https://github.com/sindresorhus/refined-github/blob/master/source/features/add-keyboard-shortcuts-to-comment-fields.js#L42-L50

The problem is that when you type a : after a , GitHub opens the emoji menus, and the most obvious way to close it is with the esc key, but the esc key will close the comment. In some cases (but not all), you get a confirmation window, so you actually don't lose what you've typed. In some cases you don't. This makes this feature actually really painful.

This problem happens a lot when you write comments using languages that require a before :, like French.

[fregante]

In the esc handler there’s a line specifically to avoid this, select.exists. Probably the selector needs to be updated.

Edit: I was confusing it with the tab handler. The line is there, I needs to be copied for ESC as well.

[levrik]

I just hit this issue. Wrote some internal review on a PR and lost everything because I tried to close this emoji view. Started to rewrite everything, almost finished it and again.. this time I wanted to write an @ but this brought up the mentioning menu... and I automatically pressed escape and lost everything again.
Sadly I've disabled the extension and won't enable again until this is fixed. This issue costed me around 45 minutes today... and a lot of frustration.

[fregante]

I understand the issue but I don't know how to reproduce this.

There's an explicit check to avoid closing the comment if it's not empty and a separate confirmation in add-confirmation-to-comment-cancellation

Can you tell me where this happens?

[levrik]

@bfred-it It happens on pull requests when you add a comment to some code line.

[fregante]

I get the confirmation:

[ewjoachim]

The problem is not not that you get or don't get the confirmation modal, but that the esc key should close the emoji/people selector menu, not try to close the comment.

If I find a case where the confirmation screen is not asked, I'll add info, but for me, the main part is that "esc" should just close the menu.

[levrik]

@bfred-it

[OvermindDL1]

I thought this issue was just me screwing up somehow!

_/me honestly wishes that the : menu didn't pop up at all, if they want an emoji then will actually put in a unicode emoji..._

And chance for an option to disable the popup menu too?

[ewjoachim]

Hm, I like the emoji menu (even if I admit it's annoying more often than not, but half of the time I'm using an emoji, I don't know its name). And as far as I can tell, refined github is about improving the default UI, more than adding more UI choices to the user, but my opinion on the matter is really limited.

@levrik ah, yes, that's it :) I bet the difference between your gif and @bfred-it 's is "comment in the "Files Changed" page" vs "comment in the Commits page"

[ewjoachim]

@OvermindDL1 (you can probably add a custom display: none on the emoji menu https://github.com/sindresorhus/refined-github/pull/1193)

[OvermindDL1]

@ewjoachim Would that also fix the tab/enter-completion because that has a habit of breaking code I'm typing in when :'s are around (which is the main reason I want it dead)...

[fregante]

@levrik thanks for the gif. Do you see any errors in the console? Regardless of this issue, you should get a confirmation. Did you disable any features?

[levrik]

@bfred-it I have the following things disabled:

monospace-textareas
reactions-avatars

[fregante]

What happens if you type something and then click the "Cancel" button? If you don't get the "confirm?" question, can you open an issue with more details (os/browser etc)?

@ewjoachim @levrik @OvermindDL1 are you all Windows/Firefox users?

[OvermindDL1]

@bfred-it I use both Firefox and Chrome routinely, always on linux though, same issue with both, hitting esc closes both the : annoying popup menu as well as also killing the comment box entirely, there is no confirm question.

[levrik]

@bfred-it I've opened #1310