Mattermost-server: Desktop: Ctrl+M shortcut minimizes the Windows app and sends a message

[esethna]

If you're interested please comment here and come join our "Contributors" community channel on our daily build server, where you can discuss questions with community members and the Mattermost core team. For technical advice or questions, please join our "Developers" community channel.

New contributors please see our Developer's Guide.

---

Notes: Jira ticket

This can be made into a Help Wanted ticket

Original report: https://github.com/mattermost/mattermost-server/issues/9306

Steps to repro:
Press Ctrl+M after composing a message.

Observed behaviour:

On the Windows app it sends the message right away, minimizes the window, then when you restore the window it looks like it is sending the message.

Expected behavior

Windows: CTRL-M does nothing

Mac: No change to current behavior. ie CMD-M minimizes (Mac OS standard) but does not send message

[ejachang]

Hi - would I be able to try taking this one?

[amyblais]

Thank you @ejachang!

[ejachang]

Hi @amyblais I'm so sorry, but I realized I misread the ticket and I actually can't work/test this because I don't have access to a Windows system. Would I actually be able to take #8092 if it is still up for grabs per the label?

[amyblais]

Hi @ejachang, no worries! Yes, https://github.com/mattermost/mattermost-server/issues/8092 seems to be up for grabs, I'll remove the label on that one.

[dnguneratne]

I'd like to work on this if it's ok.

[jasonblais]

Sure thing, thanks @dnguneratne!

[hanzei]

@dnguneratne How is your work on this ticket going? Do you have any questions?

[hanzei]

Making this available for the public again. @dnguneratne feel free to pick it up again, if you have the time.

[steevsachs]

@hanzei New contributor, mind if I take a stab at this one? I do have access to a Windows development machine.

[hanzei]

Surely @steevsachs! :+1: Thanks for working on this. Let us know, if you have questions.

[steevsachs]

Will do, thank you for the quick response!

[steevsachs]

@hanzei

Ctrl+M sends

Personally, I think we should leave this as is. The issue occurs because on Windows, Ctrl+M produces a keypress event with e.key === 'Enter'.

in utils/utils.jsx, e.key is preferred over e.keyCode when possible, which causes Ctrl+M to behave identically to Enter.

We can prevent this behavior by not preferring key over keyCode on Windows user agents, either in all cases or specifically for key === 'Enter. However, that risks reduced support for some keyboards on Windows devices.

Ctrl+M minimizes

This is being caused by the lack of a specified accelerator at https://github.com/mattermost/desktop/blob/master/src/main/menus/app.js#L193

We could

1. agree on a default minimize shortcut cross platform (ie CmdOrCtrl+Shift+M) or
2. specifically set this to a different shortcut on Windows.
3. remove the shortcut by setting accelerator to an empty string

Any opinions between those options, or what the shortcut should be?

[hanzei]

@deanwhillier What are you thoughts on the above?

[deanwhillier]

@steevsachs, thanks for looking into this! I'm just checking into a couple things and will get back to you shortly!

[deanwhillier]

@steevsachs, ya there's unfortunately no simple/clear solution for this that I can find. I almost thought I had a possible solution, but then the behaviour changed on me for no apparent reason that I could determine. 🤷‍♂️

I would say that, as you said, we leave ctrl+m as is for sending. i.e. leave it to act the same as the enter key. We can't reliably disable it without potentially affecting some other keyboard shortcut behaviour already supported, like ignoring ctrl+enter as we do for shift+enter or alt+enter.

As far as minimizing is concerned, though not a simple ctrl+m, Windows has it's own methods to minimize a window (alt-space-n & win-down-arrow). I looked at how a few other applications handle minimizing in Windows but none of them had a custom, dedicated menu option to minimize the window. So I would say that we force electron to not use a keyboard shortcut for the menu item in Windows and leave the shortcut available for Mac (expected behaviour). I'd almost say remove the whole menu item in Windows, except that it's already there in the production version, so we should probably leave it.

Summary:

1. Don't try to prevent ctrl-m from submitting a post
2. Remove the ctrl-m shortcut from the minimize menu option in Windows and leave it as-is in Mac (will need some conditionals)

Sound good?

[steevsachs]

Sounds great to me! Thank you for documenting that thought process.

I am traveling today but expect to be able to make a PR tomorrow.