diff --git a/src/components/localization/InstallationLanguage.jsx b/src/components/localization/InstallationLanguage.jsx index 994e7a8d3..d66326cd4 100644 --- a/src/components/localization/InstallationLanguage.jsx +++ b/src/components/localization/InstallationLanguage.jsx @@ -17,10 +17,9 @@ import cockpit from "cockpit"; -import React, { useContext, useEffect, useState } from "react"; +import React, { useContext, useEffect } from "react"; import { - Alert, - Divider, + Button, Form, FormGroup, Menu, @@ -28,10 +27,12 @@ import { MenuGroup, MenuItem, MenuList, - MenuSearch, - MenuSearchInput, - SearchInput, Title, + TextInputGroup, + TextInputGroupMain, + TextInputGroupUtilities, + Title, } from "@patternfly/react-core"; +import { SearchIcon, TimesIcon } from "@patternfly/react-icons"; import { setLocale } from "../../apis/boss.js"; import { @@ -51,7 +52,6 @@ import "./InstallationLanguage.scss"; const _ = cockpit.gettext; const getLanguageEnglishName = lang => lang["english-name"].v; -const getLanguageId = lang => lang["language-id"].v; const getLanguageNativeName = lang => lang["native-name"].v; const getLocaleId = locale => locale["locale-id"].v; const getLocaleNativeName = locale => locale["native-name"].v; @@ -62,8 +62,8 @@ class LanguageSelector extends React.Component { this.state = { search: "", }; + this.initiallySelectedLanguage = props.language; - this.updateNativeName = this.updateNativeName.bind(this); this.renderOptions = this.renderOptions.bind(this); } @@ -80,19 +80,13 @@ class LanguageSelector extends React.Component { } } - async updateNativeName (localeItem) { - this.props.setNativeName(getLocaleNativeName(localeItem)); - } - renderOptions (filter) { const { commonLocales, languages } = this.props; const idPrefix = this.props.idPrefix; - const filterLow = filter.toLowerCase(); const filtered = []; + const filterLow = filter.toLowerCase(); - // Is set to true when the first instance of a selected item is found. - let foundSelected = false; // Returns a locale with a given code. const findLocaleWithId = (localeCode) => { for (const languageId in languages) { @@ -106,13 +100,11 @@ class LanguageSelector extends React.Component { console.warn(`Locale with code ${localeCode} not found.`); }; - // Returns a new instance of MenuItem from a given locale and with given prefix in it's key - // and id. + // Helper to create a menu item const createMenuItem = (locale, prefix) => { const isSelected = this.props.language === getLocaleId(locale); - // Creating a ref that will be applied to the selected language and cause it to scroll into view. - const scrollRef = (isSelected && !foundSelected) + const scrollRef = (isSelected) ? (ref) => { if (ref) { ref.scrollIntoView({ block: "center" }); @@ -120,72 +112,92 @@ class LanguageSelector extends React.Component { } : undefined; - const item = ( + return ( - {getLocaleNativeName(locale)} +
+ {getLocaleNativeName(locale)} +
); + }; - // Prevent assigning scrollRef twice to languages that are both in common list and the alphabetical list. - if (isSelected) { - foundSelected = true; - } + const onSearch = (locale) => ( + getLocaleNativeName(locale).toLowerCase() + .includes(filterLow) || + getLanguageNativeName(locale).toLowerCase() + .includes(filterLow) || + getLanguageEnglishName(locale).toLowerCase() + .includes(filterLow) + ); - return item; - }; + const suggestedItems = commonLocales + .map(findLocaleWithId) + .sort((a, b) => { + if (!a || !b) { + return 0; + } + // Sort alphabetically by native name but keep the default locale at the top + if (getLocaleId(a) === this.initiallySelectedLanguage) { + return -1; + } else if (getLocaleId(b) === this.initiallySelectedLanguage) { + return 1; + } + return getLocaleNativeName(a).localeCompare(getLocaleNativeName(b)); + }) + .filter(locale => locale && onSearch(locale)) + .map(locale => createMenuItem(locale, "option-common")); - // List common languages. - if (!filter) { + if (suggestedItems.length > 0) { filtered.push( - { - commonLocales - .map(findLocaleWithId) - .filter(locale => locale) - .map(locale => createMenuItem(locale, "option-common-")) - } + {suggestedItems} - ); } - // List alphabetically. - const languagesIds = Object.keys(languages).sort(); - for (const languageId of languagesIds) { - const languageItem = languages[languageId]; - const label = cockpit.format("$0 ($1)", getLanguageNativeName(languageItem.languageData), getLanguageEnglishName(languageItem.languageData)); - - if (!filter || label.toLowerCase().indexOf(filterLow) !== -1) { - filtered.push( - - {languageItem.locales.map(locale => createMenuItem(locale, "option-alpha-"))} - - ); - } + // List other languages (filtered by search if applicable) + const otherItems = Object.keys(languages) + .sort((a, b) => { + return getLanguageNativeName(languages[a].locales[0]).localeCompare(getLanguageNativeName(languages[b].locales[0])); + }) + .flatMap(languageId => { + const languageItem = languages[languageId]; + return languageItem.locales.filter(onSearch); + }) + .filter(locale => commonLocales.indexOf(getLocaleId(locale)) === -1) + .map(locale => createMenuItem(locale, "option-alpha")); + + if (otherItems.length > 0) { + filtered.push( + + {otherItems} + + ); } - if (this.state.search && filtered.length === 0) { + // Handle case when no results are found + if (filter && filtered.length === 0) { return [ @@ -209,11 +221,9 @@ class LanguageSelector extends React.Component { setLanguage({ lang: getLocaleId(localeItem) }) .then(() => setLocale({ locale: getLocaleId(localeItem) })) .catch(ex => { - console.info({ ex }); this.props.setStepNotification(ex); }); this.setState({ lang: item }); - this.updateNativeName(localeItem); fetch("po.js").then(response => response.text()) .then(body => { // always reset old translations @@ -245,54 +255,47 @@ class LanguageSelector extends React.Component { const options = this.renderOptions(this.state.search); return ( - - - - - {_("Find a language")} - - this.setState({ search: value })} - onClear={() => this.setState({ search: "" })} - /> - - - - - {options} - - - + <> + + } + value={this.state.search} + onChange={(event) => this.setState({ search: event.target.value })} + aria-label={_("Search for a language")} + /> + {this.state.search && ( + + + + )} + + + + + {options} + + + + ); } } const InstallationLanguage = ({ idPrefix, setIsFormValid, setStepNotification }) => { const { commonLocales, language, languages } = useContext(LanguageContext); - const [nativeName, setNativeName] = useState(false); useEffect(() => { setIsFormValid(language !== ""); @@ -306,18 +309,7 @@ const InstallationLanguage = ({ idPrefix, setIsFormValid, setStepNotification }) {_("Choose a language")}
- - {nativeName && ( - - {_("The chosen language will be used for installation and in the installed software. " + - "To use a different language, find it in the language list.")} - - )} + diff --git a/src/components/localization/InstallationLanguage.scss b/src/components/localization/InstallationLanguage.scss index 103aac6a7..e192749f1 100644 --- a/src/components/localization/InstallationLanguage.scss +++ b/src/components/localization/InstallationLanguage.scss @@ -1,8 +1,37 @@ -#language-alert { - margin-bottom: 1.25rem; +.installation-language-menu.pf-v5-c-menu.pf-m-scrollable { + max-width: 400px; + // heading: 84, footer: 44, content (about from header): 158, necessary padding underneath: 8px, + // 50px magic number + --pf-v5-c-menu__content--MaxHeight: calc(100vh - 84px - 44px - 158px - 8px - 50px); } +.installation-language-search { + margin-bottom: var(--pf-v5-global--spacer--sm); + max-width: 400px; +} + +.installation-language-menu { + border: var(--pf-v5-global--BorderWidth--sm) solid var(--pf-v5-global--BorderColor--100); +} + +/* +While this is something that needs to be fixed globally in an overrides, +we're only using the component here, so we have a local, namespaced fix, +to prevent surprises elsewhere. However, if we use the widget elsewhere, +it would also need this fix, so this is definitely a fix for PF +overrides, not a local fix. +*/ #installation-language-language-menu { - margin-top: 1rem; - margin-bottom: 2rem; + // Oddly, the spacing here isn't consistent with the reference on the website; + // it should be balanced on the top and bottom, not all just on the top + .pf-v5-c-menu__group-title { + padding-block: var(--pf-v5-global--spacer--sm); + + // As the top is smaller, it needs a little more space between it and + // the rest of the elements. The first one doesn't need this, however, + // so we only apply it to all headings that aren't the first one + .pf-v5-c-menu__group:not(:first-of-type) & { + margin-block-start: var(--pf-v5-global--spacer--sm); + } + } } diff --git a/test/check-language b/test/check-language index 5d41bc0fd..b6efeb49c 100755 --- a/test/check-language +++ b/test/check-language @@ -101,13 +101,13 @@ class TestLanguage(VirtInstallMachineCase): # Check that the language menu shows all menu entries when clicking the toggle button l.locale_option_visible('en_US') l.locale_option_visible('de_DE') - l.locale_option_visible('cs_CZ') + l.locale_option_visible('cs_CZ', is_common=False) # Check filtering on English names l.input_locale_search('cze') l.locale_option_visible('en_US', False) l.locale_option_visible('de_DE', False) - l.locale_option_visible('cs_CZ') + l.locale_option_visible('cs_CZ', is_common=False) # Check filtering on native names l.input_locale_search('Deutsch') @@ -139,7 +139,7 @@ class TestLanguage(VirtInstallMachineCase): # Expect language direction to be set to RTL for selected languages l.select_locale("he_IL", False) - l.check_selected_locale("he_IL") + l.check_selected_locale("he_IL", is_common=False) b.wait_attr("html", "dir", "rtl") # Expect language direction to be set to LTR when switching to EN_US from RTL language diff --git a/test/helpers/language.py b/test/helpers/language.py index 5ba7357f2..9c46458fa 100644 --- a/test/helpers/language.py +++ b/test/helpers/language.py @@ -39,29 +39,31 @@ def __init__(self, browser, machine): @log_step() def select_locale(self, locale, is_common=True): - common_prefix = "common-" if is_common else "alpha-" - if self.browser.val(f"#{self._step}-language-search .pf-v5-c-text-input-group__text-input") != "": + common_prefix = "common" if is_common else "alpha" + if self.browser.val(f".{self._step}-search .pf-v5-c-text-input-group__text-input") != "": self.input_locale_search("") - self.browser.click(f"#{self._step}-option-{common_prefix}{locale}") + self.browser.click(f"#{self._step}-option-{common_prefix}-{locale}") @log_step() def get_locale_search(self): - return self.browser.val(f"#{self._step}-language-search .pf-v5-c-text-input-group__text-input") + return self.browser.val(f".{self._step}-search .pf-v5-c-text-input-group__text-input") @log_step() def input_locale_search(self, text): - self.browser.set_input_text(f"#{self._step}-language-search .pf-v5-c-text-input-group__text-input", text) + self.browser.set_input_text(f".{self._step}-search .pf-v5-c-text-input-group__text-input", text) @log_step() - def locale_option_visible(self, locale, visible=True): + def locale_option_visible(self, locale, visible=True, is_common=True): + common_prefix = "common" if is_common else "alpha" if visible: - self.browser.wait_visible(f"#{self._step}-option-alpha-{locale}") + self.browser.wait_visible(f"#{self._step}-option-{common_prefix}-{locale}") else: - self.browser.wait_not_present(f"#{self._step}-option-alpha-{locale}") + self.browser.wait_not_present(f"#{self._step}-option-{common_prefix}-{locale}") @log_step(snapshot_before=True) - def check_selected_locale(self, locale): - self.browser.wait_visible(f"#{self._step}-option-alpha-{locale}.pf-m-selected") + def check_selected_locale(self, locale, is_common=True): + common_prefix = "common" if is_common else "alpha" + self.browser.wait_visible(f"#{self._step}-option-{common_prefix}-{locale}.pf-m-selected") def dbus_set_language(self, value): self.machine.execute(f'dbus-send --print-reply --bus="{self._bus_address}" \ diff --git a/test/reference b/test/reference index 726a2a6b7..ac83e09bd 160000 --- a/test/reference +++ b/test/reference @@ -1 +1 @@ -Subproject commit 726a2a6b79173de5a81a5c91d9967deefa5f676d +Subproject commit ac83e09bd2c3fd2d5220227d88b5d16438231b49