zulip-terminal icon indicating copy to clipboard operation
zulip-terminal copied to clipboard

Render special keys as shown on the keyboard

Open Parth576 opened this issue 1 year ago • 4 comments

What does this PR do?

This PR aims to display the keybindings in zulip-terminal as they are shown on the keyboard, or similar to how zulip-web displays keybindings. Related documentation issue: #945. Since zulip-terminal uses keys_for_commands and primary_key_for_command to display keybindings, I tried creating different methods for displaying keybindings, these functions capitalizes the first letter of special keys, characters and uses the unicode characters for the direction keys.

Fixes #1327

Relevant CZO discussion

Tested?

  • [x] Manually
  • [x] Existing tests (adapted, if necessary)
  • [x] New tests added (for any new behavior)
  • [x] Passed linting & tests (each commit)

Commit flow

  • 1st Commit: adds both the display methods in the keys.py file
  • 2nd Commit: use the new keybinding methods for random tips keys
  • 3rd Commit: Use the new display keybinding methods in all files: helper/model/ui_tools

Notes & Questions

  • Adding a '+' in between keys?
  • Sizing issues with Uppercase keys (Shift + letter)
  • Fallback for unicode characters (direction keys)

Interactions

Visual changes image image

Parth576 avatar Mar 24 '23 11:03 Parth576

Hello @zulip/server-hotkeys members, this pull request was labeled with the "area: hotkeys" label, so you may want to check it out!

zulipbot avatar Apr 04 '23 19:04 zulipbot

Hey @neiljp, sorry for the late updates to the PR. Are the urwid_key_to_display_key mapping and display_key functions more in line now with what you were initially thinking?

Also added an extra commit for the arrow key symbols as you had suggested earlier.

Parth576 avatar Apr 12 '23 11:04 Parth576

@zulipbot remove "PR awaiting update" @zulipbot add "PR needs review"

Parth576 avatar Apr 13 '23 21:04 Parth576

Heads up @Parth576, we just merged some commits that conflict with the changes you made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the upstream/main branch and resolve your pull request's merge conflicts accordingly.

zulipbot avatar Dec 16 '23 01:12 zulipbot

@Parth576 Thanks for working on this :+1:

#1476 was just merged as a followup which incorporated and attributed your initial work, so I'm closing this PR now.

neiljp avatar Apr 11 '24 07:04 neiljp