Skip to content

fix(ui): align macos accelerators with platform expectations - #765

Open
fenbyte wants to merge 1 commit into
screwys:mainfrom
fenbyte:main
Open

fix(ui): align macos accelerators with platform expectations#765
fenbyte wants to merge 1 commit into
screwys:mainfrom
fenbyte:main

Conversation

@fenbyte

@fenbyte fenbyte commented Aug 12, 2026

Copy link
Copy Markdown

my first time writing any rust, be nice!

fixes #764

@screwys

screwys commented Aug 12, 2026

Copy link
Copy Markdown
Owner

thanks a lot ٩(˘◡˘)۶
I really didn't want to mechanically translate these fearing these would certainly be too unconventional/fight with MacOS default keybinds. We can use totally different letters here for MacOS, or change some letters globally. We don't want bare letters since that would not work when type-to-search is enabled

});
add_window_action(shell, "mute", &["<Control>m"], {
#[cfg(target_os = "macos")]
let mute_accels = &["<Meta>m"][..];

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Isn't this for minimizing windows?

Put the Music window in the Dock | Command-M

https://support.apple.com/en-asia/guide/music/mus1019/mac

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

lol, youre right...

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

iina uses cmd+/, not sure about that one though...

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I think it is fine to drop our keybinds for these, I just used initial letters without much thought but there is no specific reason to keep them

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

ctrl +/- is used for volume currently though

@fenbyte fenbyte Aug 16, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

what about m? that seems pretty easy to figure out, and spotify uses that

});
add_window_action(shell, "seek-forward", &["<Control>Right"], {
#[cfg(target_os = "macos")]
let seek_forward_accels = &["Right", "<Shift><Meta>Right"][..];

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

did you know that you can navigate between rows and grids with arrow keys :P
so we would want to avoid bare left/right for seekback

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Actually, it makes more sense to reverse these; navigation can take ctrl keybinding since that is more niche

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

i agree

shell.chrome.window.add_action(&fullscreen);

add_window_action(shell, "play-pause", &["<Control>space"], {
add_window_action(shell, "play-pause", &["space"], {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

bare Space already works, so this would duplicate that
or does it not work on MacOS?

@fenbyte fenbyte Aug 13, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

oh, yeah this works already, oops

@screwys

screwys commented Aug 13, 2026

Copy link
Copy Markdown
Owner

I forgot to submit the review LOL

@fenbyte

fenbyte commented Aug 13, 2026

Copy link
Copy Markdown
Author

LOL all good

@screwys

screwys commented Aug 16, 2026

Copy link
Copy Markdown
Owner

what should we do about this, should I handle the navigation keybinds part? I still kind of want to entrust exact Mac keybinds to you lol

@fenbyte

fenbyte commented Aug 16, 2026

Copy link
Copy Markdown
Author

i think seeking should be left and right alone and navigation would require command to be held. maybe volume should be up and down alone too since i don't know of a single mac app that uses cmd+= and cmd+-. this behaviour would match most media and music players i think. obviously if the cursor is in a text ox this shouldn't happen anymore but i'm sure gtk handles that or something

@fenbyte

fenbyte commented Aug 16, 2026

Copy link
Copy Markdown
Author

actually itunes/apple music lets you scroll up and down the list with up and down keys but next/previous track with left and right... honestly i think this one is up to your preference, but i'm gonna refresh my memory on some apps real quick

@fenbyte

fenbyte commented Aug 16, 2026

Copy link
Copy Markdown
Author

ok i think i like the way spotify does things. arrow keys are for navigation and scrolling, cmd+left/right is for previous/next track, shift+cmd+left/right is for seeking, and cmd+up/down are for volume

@screwys

screwys commented Aug 16, 2026

Copy link
Copy Markdown
Owner

that sounds fine, especially for volume

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: seek controls on macos conflict with system

2 participants