Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(chat): Edit last message hotkey on macOS #13658

Merged
merged 5 commits into from
Nov 1, 2024
Merged

fix(chat): Edit last message hotkey on macOS #13658

merged 5 commits into from
Nov 1, 2024

Conversation

ShGKme
Copy link
Contributor

@ShGKme ShGKme commented Oct 29, 2024

☑️ Resolves

🖌️ UI Checklist

🖼️ Screenshots / Screencasts

🏚️ Before 🏡 After
image image
image image

🏁 Checklist

  • 🌏 Tested with different browsers / clients:
    • Chromium (Chrome / Edge / Opera / Brave)
    • Firefox
    • Safari
    • Talk Desktop
    • Not risky to browser differences / client
  • 🖌️ Design was reviewed, approved or inspired by the design team
  • ⛑️ Tests are included or not possible
  • 📗 User documentation in https://github.com/nextcloud/documentation/tree/master/user_manual/talk has been updated or is not required

@ShGKme ShGKme added 3. to review bug feature: chat 💬 Chat and system messages labels Oct 29, 2024
@ShGKme ShGKme added this to the 🖤 Next Major (31) milestone Oct 29, 2024
@ShGKme ShGKme self-assigned this Oct 29, 2024
@ShGKme
Copy link
Contributor Author

ShGKme commented Oct 29, 2024

/backport to stable30

@@ -82,6 +82,7 @@
dir="auto"
@keydown.esc="handleInputEsc"
@keydown.ctrl.up="handleEditLastMessage"
@keydown.meta.up="handleEditLastMessage"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

meta is a 'Windows' key also. maybe replace it with useHotKey()?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

meta is a 'Windows' key also

It is not a problem, it is ignored on Windows and, afaik, all (most?) Linux DEs (Checked on GNOME, KDE, xfce, Cinnamon)

maybe replace it with useHotKey()?

I'd avoid having a global listener here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update: xfce handles meta key with default settings.

But this doesn't lead to an issue:

  • If a user has desktop environment to handle this hotkey, e.g., for window management - it has no affect in talk
  • If a user doesn't have it in DE, and presses this hotkey while the focus is in the message input - it enables the last message editing
    • In the best case - a user get used to macOS is happy
    • In the worst case - some other (bot not desktop environment level) hotkey lead to a harmless action in talk - edit mode

* Per-OS flags
*/

export const isMac = os.name === 'Mac OS'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we update in the meantime?
https://docs.uaparser.dev/intro/whats-new.html#version-2-0

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then let's follow-up, if you don't want to mix PRs

Signed-off-by: Grigorii K. Shartsev <[email protected]>
Copy link
Contributor

@DorraJaouad DorraJaouad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested on Windows (at least it's confirmed that it remains working for Win :') )

@ShGKme ShGKme merged commit 5c10a88 into main Nov 1, 2024
46 checks passed
@ShGKme ShGKme deleted the fix/macos-hotkeys branch November 1, 2024 16:21
@ShGKme
Copy link
Contributor Author

ShGKme commented Nov 1, 2024

/backport to stable30

Copy link

backportbot bot commented Nov 1, 2024

The backport to stable30 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable30
git pull origin stable30

# Create the new backport branch
git checkout -b backport/13658/stable30

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts, resolve them
git cherry-pick cdc937ee 76d88fd8 64622149 fa2a3498 a8e04e76

# Push the cherry pick commit to the remote repository and open a pull request
git push origin backport/13658/stable30

Error: Failed to create pull request: Validation Failed: {"resource":"PullRequest","code":"custom","message":"A pull request already exists for nextcloud:backport/13658/stable30."}


Learn more about backports at https://docs.nextcloud.com/server/stable/go.php?to=developer-backports.

@ShGKme
Copy link
Contributor Author

ShGKme commented Nov 6, 2024

/backport to stable30.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants