qmk / qmk_configurator

The QMK Configurator
http://config.qmk.fm
689 stars 343 forks source link

Remove unused ISO toggle code #1317

Closed precondition closed 11 months ago

precondition commented 11 months ago

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.

yanfali commented 11 months ago

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

precondition commented 11 months ago

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 https://github.com/qmk/qmk_configurator/pull/1317/commits/f0d5c2a1be89954dd1e1f3c4c8e1efbada0b0d24.

noroadsleft commented 11 months ago

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 commented 11 months ago

Thanks!