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

Do not uppercase single letter key legends #1184

Merged
merged 1 commit into from
Oct 4, 2022

Conversation

precondition
Copy link
Contributor

@precondition precondition commented Sep 5, 2022

Description

It is not necessary and in the cases of OS keyboard layouts like French (France) or French (Switzerland) that have lowercase letters as the shift keysyms, it is wrong.

FR_MICR incorrectly displayed as capital µ which looks visually identical to capital latin M which also happens to be KC_CLN.
Capital M and Capital Mu in French layout

FR-Swiss diaeresis keys incorrectly displayed in capital form even though the actual output is lowercase:
FR-swiss diaeresis highlighted

(Screenshots done with the #1165 PR)

@precondition
Copy link
Contributor Author

Hm, interestingly, the tester page doesn't apply the OS keyboard layout on page load so it falls back to store/modules/keycodes/ansi.js and store/modules/keycodes/iso.js which contain lowercase name fields like { name: 'z', code: 'KC_Z' } unlike the keymap_*.js files which feature uppercase name fields.

@precondition precondition marked this pull request as draft September 5, 2022 18:33
@yanfali
Copy link
Collaborator

yanfali commented Oct 4, 2022

Seems like a good change now we have multi layout awareness. Please rebase to get rid of the cypress error.

@precondition precondition force-pushed the do_not_uppercase_single_letter branch 3 times, most recently from 268c368 to dd07a74 Compare October 4, 2022 15:24
@precondition precondition marked this pull request as ready for review October 4, 2022 15:24
@precondition
Copy link
Contributor Author

Despite the multi-layout awareness, “Handle typing” and “Detects chatter” still require #1186 to pass.

@yanfali
Copy link
Collaborator

yanfali commented Oct 4, 2022

1186 is merged. Hopefully this fixes your issues

@precondition precondition force-pushed the do_not_uppercase_single_letter branch from dd07a74 to c3923b1 Compare October 4, 2022 15:49
@precondition
Copy link
Contributor Author

Yes, the capital P can now be found.

Copy link
Collaborator

@yanfali yanfali left a comment

Choose a reason for hiding this comment

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

Lgtm

@yanfali
Copy link
Collaborator

yanfali commented Oct 4, 2022

Thanks

@yanfali yanfali merged commit 057f349 into qmk:master Oct 4, 2022
@precondition precondition deleted the do_not_uppercase_single_letter branch October 4, 2022 16:10
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