Opened 10 years ago

Last modified 23 months ago

#3980 assigned enhancement

[patch] Keyboard shortcut icon for Apple keyboards

Reported by: VinDuv Owned by: nobody
Priority: normal Milestone: R1
Component: Kits/Interface Kit Version: R1/pre-alpha1
Keywords: Cc: planche2k@…
Blocked By: Blocking:
Has a Patch: no Platform: All

Description

Depending on the keymap chosen in the Keymaps preflet, the keyboard shortcut icon in the menus shows "ALT" or "CTL". This is not appropriate when using an Apple keyboard, which has a Command key.
The attached patch adds a "CMD" icon to the shortcut icons, and use it when an Apple keymap is used. It also disables the "Switch Shortcut Keys" in the Keymaps preflet, which does not makes much sense in that case.

Attachments (2)

cmd_shortcut.diff (5.4 KB) - added by VinDuv 10 years ago.
cmd_shortcut2.diff (5.7 KB) - added by VinDuv 10 years ago.
Patch v2

Download all attachments as: .zip

Change History (11)

Changed 10 years ago by VinDuv

Attachment: cmd_shortcut.diff added

comment:1 Changed 10 years ago by andreasf

Cc: planche2k@… added

Your patch introduces a potential NULL dereference of keys in src/kits/interface/Menu.cpp.

I'm less certain why in presence of the command key the alt and ctrl alternatives - present on Apple keyboards, too - should be disallowed.

But the shortcut icon is a neat idea, I'll have to try it out sometime. :)

comment:2 in reply to:  1 Changed 10 years ago by VinDuv

Replying to andreasf:

Your patch introduces a potential NULL dereference of keys in src/kits/interface/Menu.cpp.

Whoops, fixed ;)

I'm less certain why in presence of the command key the alt and ctrl alternatives - present on Apple keyboards, too - should be disallowed.

I wasn't sure if it was really useful... Mac users are generally accustomed to using the Command key for keyboard shortcuts, so allow switching to Control is probably not needed.
Anyway, I reactivated the button in this new patch and fixed the mode detection so it shows "Switch Shortcut Keys To Windows/Linux Mode" or "Switch Shortcut Keys To Haiku/Mac OS Mode" instead of just "Switch Shortcut Keys" for Apple keymaps.

Changed 10 years ago by VinDuv

Attachment: cmd_shortcut2.diff added

Patch v2

comment:3 Changed 10 years ago by axeld

Just a few comments: the B_COMMAND_IS_* constants should not be public or used at all, since we already have modifier constants that can do the job.

I don't like the "/Mac OS" addition in Keymaps, and would just remove it again - the check now looks wrong, too, as it will detect Windows/Haiku mode now incorrectly.

comment:4 Changed 9 years ago by nielx

Has a Patch: unset

comment:5 Changed 9 years ago by nielx

Has a Patch: set

comment:6 Changed 7 years ago by anevilyak

Has a Patch: unset

comment:7 Changed 4 years ago by waddlesplash

Milestone: R1Unscheduled

comment:8 Changed 4 years ago by waddlesplash

Milestone: UnscheduledR1

Reverting earlier milestone change

comment:9 Changed 23 months ago by axeld

Owner: changed from axeld to nobody
Status: newassigned
Note: See TracTickets for help on using tickets.