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

Remove unused ISO toggle code #1317

Merged
merged 2 commits into from
Oct 28, 2023

Conversation

precondition
Copy link
Contributor

@precondition precondition commented Oct 20, 2023

Description

Remove unused code related to the Settings Panel ISO toggle, rendered obsolete by the addition of host OS keyboard layout support.
Note that this does not remove configuratorSettings.iso because it it still a helpful flag to use for the communication between the keycode updating logic and the key tester updating logic. The key tester must automatically switch between ISO and ANSI depending on what's the standard physical geometry for a given national layout. I initially wanted to remove configuratorSettings.iso as well but I forgot about that auto-layout-change behaviour from the key tester, hence the misleading branch name.

@precondition precondition changed the title Remove remnants of obsolete configuratorSettings.iso Remove unused ISO toggle code Oct 20, 2023
@yanfali
Copy link
Collaborator

yanfali commented Oct 28, 2023

It's good to have screenshots to accompany UI changes. I'll try and review this today.

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. @noroadsleft any issues?

@precondition
Copy link
Contributor Author

precondition commented Oct 28, 2023

I just realized that calling keycodes/changeActive with the new iso value is important to change the selected tab in the keycodes picker to the appropriate one for the OS layout.

If you remove it, going from "English (USA)" (ANSI) to "Estonian (Estonia)" (ISO) will change the order of the tabs as expected but the ANSI will remain selected. The current behavior (without this PR) is to also change the selected tab to be ISO. To keep the same behavior, I reverted that change in f0d5c2a.

@noroadsleft
Copy link
Member

It's good to have screenshots to accompany UI changes. I'll try and review this today.

I'm not seeing any UI changes here. Near as I can tell, this is all orphaned code.

@noroadsleft noroadsleft merged commit db4e127 into qmk:master Oct 28, 2023
2 checks passed
@noroadsleft
Copy link
Member

Thanks!

@precondition precondition deleted the removeConfiguratorSettingsISO branch October 29, 2023 05:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants