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

feat: add function key modifier #3470

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

decodism
Copy link
Contributor

@decodism decodism commented Jul 7, 2024

It's a bit hacky as Carbon doesn't support it.

@lwouis
Copy link
Owner

lwouis commented Jul 7, 2024

Hi @decodism,

Thank you for sharing this work.

Adding the function key was discussed in the past. In end, I concluded that opening that door would create support situation that would not be worth. Have you seen this thread perhaps?

It's pretty cool that you managed to implement that! I need to test it to see how it behaves.

As I mentioned above, I'm not sure it's worth to add this feature. It would probably be of niche use, and open the door to all sorts of exotic bugs and behaviors.

What do you think?

Thank you 🙇

@lwouis
Copy link
Owner

lwouis commented Jul 8, 2024

I played around with the PR. Here's some feedback, in addition to my reserves above about opening that door:

  • I think the symbol F may be a bit confusing. Perhaps some fn in small letters would be clearer. It's a consistent wording on the physical keyboards
  • The app crashed when I tried to use a shortcut with F1. I held fn then the f-keys appeared, and I pressed F1. I see in the debugger that in ControlsTabs.swift, line 194,Shortcut(keyEquivalent: newValue) was nil. newValue was equal to "⌥FF1". It raises the question: how should the fn key interact with f-keys like F1. Should pressing fn then F1 count as F1 only, or as modifier fn + F1 key?

Thank you

@decodism
Copy link
Contributor Author

Hello,

I'm not sure what issues this addition might bring. I've made some changes following your feedback:

  • Replace F with fn, which has the side effect of fixing the crash with F* keys.
  • No longer display the function modifier with F* keys

@lwouis
Copy link
Owner

lwouis commented Jul 11, 2024

Hi @decodism,

Thank you for the updates!

Regarding my concerns:

As you've said, it's not supported by Carbon. This means it's exotic and other apps supporting it will be few. Users probably are not used to it or expecting it to work, or the nuances which go with it. For instance, the cases with fn+F1 I've shared previously, or the fact that Apple own UI in System Settings is broken currently when assigning shortcuts with fn

The fn key is used together with the globe key. I think this also adds lots of confusion to supporting this feature. For instance, my setup is an external Logitech K380 keyboard, and a 2022 macbook pro. Here's how they physically look like:

Logitech K380 2022 macbook pro
IMG_2324 Large IMG_2323 Large

Notice how the System Settings mention the fn key for the external keyboard, and the globe key for the internal keyboard. Even though both keys features the fn marking on it:

Logitech K380 2022 macbook pro
image image

I can set fn using the internal keyboard, with your PR, but I can't set fn with the external keyboard. There must be an implementation difference. Maybe you catch the globe key code instead of fn.

Do you see what I mean by adding complexity and exotic use-case if we merge this?

What do you think?

Thank you

@decodism
Copy link
Contributor Author

Hello,

It doesn't seem to be possible with third-party keyboards.

If you think it might bring more issues, you don't have to merge it. Interested people can always roll their own build.

@lwouis lwouis force-pushed the master branch 2 times, most recently from 7d7d9cf to 8abb9b4 Compare October 10, 2024 09:28
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.

2 participants