Fixes 2 issues with Dialog closing

[t3chguy]

• Upload Confirmation dialog would just change focus on ESC and not close
• Keywords Dialog in UserSettings would also close UserSettings because event bubbled up

element-hq/element-web#1801 doesn't exist in /app/ yet does in /develop/ because of this

[matrixbot]

Can one of the admins verify this patch?

[ara4n]

unsure I understand the fix here; isn't this going to intercept and deny any keyboard activity in the dialog?

[t3chguy]

@ara4n and that is why thats my last commit today
/me goes to sleep

[ara4n]

@lukebarnard1 i assume the onKeyDown() bit is yours - i don't follow the comment here; can you spell out precisely which bug & scenario switching from onKeyPress() to onKeyDown() fixes?

[lukebarnard1]

So @t3chguy did a PR that after closing the dialog would refocus the thing that had focus prior to the dialog appearing #822. But because the shift of focus was happening onKeyDown and because the focused element was then getting the onKeyPressed/Up following that, another problem occured: element-hq/element-web#3714 (comment):

I've realised it's because the priorActiveElement is the button that opened the dialog. If this is focused and the enter key is released(?), this triggers a keyPress which fires once the dialog has closed and the button has been focused 😬 the BaseDialog only calls stopPropagation _onKeyDown.

Resulting in every dialog affected appearing twice.

[t3chguy]

and now because Escape is not caught onKeyDown other things are free to catch it and act upon it

[lukebarnard1]

It's a shame that BaseDialog has to worry about what could happen if onKey[Whatever] bubbles up. It would be much nicer if it could just absorb all of them whilst it's mounted without having to define a handler for each conceivable event.

[t3chguy]

@lukebarnard1 any nice ways of doing that?

[lukebarnard1]

@t3chguy I talked it through with @dbkr and we can't think of a nicer way to do it other than absorbing all the key events (up, down, pressed) in the dialog. To prevent the inputs on the dialog from being broken, you should only call stopPropagation (not preventDefault).

[t3chguy]

@lukebarnard1 ptal