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

BUG: Fix ctkLanguageComboBox normalizing default language and selection #1179

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

Conversation

jcfr
Copy link
Member

@jcfr jcfr commented Jan 16, 2024

Discovered during the resolution of issues in ctkLanguageComboBoxTest, this commit addresses a bug in the normalization and selection of the default language in ctkLanguageComboBox.

It introduces the function normalizeLocaleCode to ensure that the default locale code is set to its normalized value if valid.

Furthermore, the updateLanguageItems() function is updated to select the default locale code if the user has not already made a selection.

@jcfr jcfr requested a review from lassoan January 16, 2024 01:14
jcfr added a commit to jcfr/SlicerLanguagePacks that referenced this pull request Jan 16, 2024
This change ensures the fix integrated through commontk/CTK#1179 (BUG: Fix
ctkLanguageComboBox normalizing default language and selection) is effective.
@jcfr jcfr force-pushed the fix-ctkLanguageComboBoxTest branch from 7af2ae7 to a7385c9 Compare January 16, 2024 04:29
@lassoan
Copy link
Member

lassoan commented Jan 16, 2024

Furthermore, the updateLanguageItems() function is updated to select the default locale code if the user has not already made a selection.

Could this have the unintended side effect that if the widget is used in the application settings then it writes in some language automatically (as if the user actively selected it)? That would not be nice, because we could not distinguish if the user selected English explicitly (in this case we should use English) or the user simply did not select a language (in this case we may decide in the future to use for example the operating system's language).

@jcfr jcfr force-pushed the fix-ctkLanguageComboBoxTest branch from a7385c9 to 9b849cf Compare January 16, 2024 04:52
Discovered during the resolution of issues in `ctkLanguageComboBoxTest`, this
commit addresses a bug in the normalization and selection of the default language
in `ctkLanguageComboBox`.

It introduces the function `normalizeLocaleCode` to ensure that the default locale
code is set to its normalized value if valid.

Furthermore, the `updateLanguageItems()` function is updated to select the default
locale code if the user has not already made a selection.

Co-authored-by: Andras Lasso <[email protected]>
@jcfr jcfr force-pushed the fix-ctkLanguageComboBoxTest branch from 4cb0268 to 9e7f262 Compare January 16, 2024 05:07
@jcfr
Copy link
Member Author

jcfr commented Jan 16, 2024

Could this have the unintended side effect that if the widget is used in the application settings then it writes in some language automatically (as if the user actively selected it)?

It seems the language combobox is trying to be both a manager and language selector, moving forward we should probably look into introducing an object for dealing with the scanning of a directory, dealing with default vs explicitly selected ...

This should probably be done in future improvements.

With this fix, the test now pass and the behavior reported in Slicer/SlicerLanguagePacks#57 is fixed.

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

Successfully merging this pull request may close these issues.

2 participants