From d5cf137e7aee2690071c32c6874af738a48026b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Tue, 2 Jul 2024 09:09:29 +0100 Subject: [PATCH 01/38] feat(web): add TanStack Query --- web/package-lock.json | 25 +++++++++++++++++++++++++ web/package.json | 1 + web/src/App.jsx | 7 +++++-- 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/web/package-lock.json b/web/package-lock.json index e8e79ec4ce..32878954fa 100644 --- a/web/package-lock.json +++ b/web/package-lock.json @@ -12,6 +12,7 @@ "@patternfly/patternfly": "^5.1.0", "@patternfly/react-core": "^5.1.1", "@patternfly/react-table": "^5.1.1", + "@tanstack/react-query": "^5.49.2", "fast-sort": "^3.4.0", "ipaddr.js": "^2.1.0", "react": "^18.2.0", @@ -4210,6 +4211,30 @@ "url": "https://github.com/sponsors/gregberge" } }, + "node_modules/@tanstack/query-core": { + "version": "5.49.1", + "resolved": "https://registry.npmjs.org/@tanstack/query-core/-/query-core-5.49.1.tgz", + "integrity": "sha512-JnC9ndmD1KKS01Rt/ovRUB1tmwO7zkyXAyIxN9mznuJrcNtOrkmOnQqdJF2ib9oHzc2VxHomnEG7xyfo54Npkw==", + "funding": { + "type": "github", + "url": "https://github.com/sponsors/tannerlinsley" + } + }, + "node_modules/@tanstack/react-query": { + "version": "5.49.2", + "resolved": "https://registry.npmjs.org/@tanstack/react-query/-/react-query-5.49.2.tgz", + "integrity": "sha512-6rfwXDK9BvmHISbNFuGd+wY3P44lyW7lWiA9vIFGT/T0P9aHD1VkjTvcM4SDAIbAQ9ygEZZoLt7dlU1o3NjMVA==", + "dependencies": { + "@tanstack/query-core": "5.49.1" + }, + "funding": { + "type": "github", + "url": "https://github.com/sponsors/tannerlinsley" + }, + "peerDependencies": { + "react": "^18.0.0" + } + }, "node_modules/@testing-library/dom": { "version": "10.1.0", "resolved": "https://registry.npmjs.org/@testing-library/dom/-/dom-10.1.0.tgz", diff --git a/web/package.json b/web/package.json index 6678916e35..a033858d1f 100644 --- a/web/package.json +++ b/web/package.json @@ -103,6 +103,7 @@ "@patternfly/patternfly": "^5.1.0", "@patternfly/react-core": "^5.1.1", "@patternfly/react-table": "^5.1.1", + "@tanstack/react-query": "^5.49.2", "fast-sort": "^3.4.0", "ipaddr.js": "^2.1.0", "react": "^18.2.0", diff --git a/web/src/App.jsx b/web/src/App.jsx index 9cbad27b3e..8d4f716782 100644 --- a/web/src/App.jsx +++ b/web/src/App.jsx @@ -29,6 +29,9 @@ import { useInstallerClientStatus } from "~/context/installer"; import { useProduct } from "./context/product"; import { CONFIG, INSTALL, STARTUP } from "~/client/phase"; import { BUSY } from "~/client/status"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; + +const queryClient = new QueryClient(); /** * Main application component. @@ -70,10 +73,10 @@ function App() { if (!language) return null; return ( - <> + - + ); } From 1054332402b91676c87f537418a5d210ab47ceb3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Tue, 2 Jul 2024 09:15:27 +0100 Subject: [PATCH 02/38] feat(web): use React Query to fetch locales data --- web/src/components/l10n/L10nPage.jsx | 19 ++++- web/src/components/l10n/LocaleSelection.jsx | 37 ++++++--- web/src/context/l10n.jsx | 2 +- web/src/queries/l10n.js | 92 +++++++++++++++++++++ web/src/utils.js | 6 ++ 5 files changed, 142 insertions(+), 14 deletions(-) create mode 100644 web/src/queries/l10n.js diff --git a/web/src/components/l10n/L10nPage.jsx b/web/src/components/l10n/L10nPage.jsx index 5c1ee7dd14..2289ec2042 100644 --- a/web/src/components/l10n/L10nPage.jsx +++ b/web/src/components/l10n/L10nPage.jsx @@ -19,7 +19,7 @@ * find language contact information at www.suse.com. */ -import React from "react"; +import React, { useEffect, useState } from "react"; import { Gallery, GalleryItem, } from "@patternfly/react-core"; @@ -28,6 +28,7 @@ import { ButtonLink, CardField, Page } from "~/components/core"; import { _ } from "~/i18n"; import { useL10n } from "~/context/l10n"; import buttonStyles from '@patternfly/react-styles/css/components/Button/button'; +import { useConfig, useLocales } from "../../queries/l10n"; const Section = ({ label, value, children }) => { return ( @@ -51,9 +52,23 @@ export default function L10nPage() { const { selectedKeymap: keymap, selectedTimezone: timezone, - selectedLocales: [locale] } = useL10n(); + const [locale, setLocale] = useState(); + const { data: locales } = useLocales(); + const { isPending, data: localeId } = useConfig((i) => i.locales[0]); + + useEffect(() => { + if (!locales) return; + + const found = locales.find((l) => l.id === localeId); + if (found) { + setLocale(found); + } + }, [locales, localeId]); + + if (isPending) return; + return ( <> diff --git a/web/src/components/l10n/LocaleSelection.jsx b/web/src/components/l10n/LocaleSelection.jsx index 676e28e5e5..6842f74b14 100644 --- a/web/src/components/l10n/LocaleSelection.jsx +++ b/web/src/components/l10n/LocaleSelection.jsx @@ -27,45 +27,60 @@ import { } from "@patternfly/react-core"; import { useNavigate } from "react-router-dom"; import { _ } from "~/i18n"; -import { useL10n } from "~/context/l10n"; -import { useInstallerClient } from "~/context/installer"; import { ListSearch, Page } from "~/components/core"; import textStyles from '@patternfly/react-styles/css/utilities/Text/text'; +import { useLocales, useConfig, useConfigMutation } from "../../queries/l10n"; // TODO: Add documentation and typechecking // TODO: Evaluate if worth it extracting the selector export default function LocaleSelection() { - const { l10n } = useInstallerClient(); - const { locales, selectedLocales } = useL10n(); - const [selected, setSelected] = useState(selectedLocales[0]); - const [filteredLocales, setFilteredLocales] = useState(locales); + const { isPending, data: locales } = useLocales(); + const { data: config } = useConfig(); + const setConfig = useConfigMutation(); + const [initial, setInitial] = useState(); + const [selected, setSelected] = useState(); + const [filteredLocales, setFilteredLocales] = useState([]); const navigate = useNavigate(); const searchHelp = _("Filter by language, territory or locale code"); useEffect(() => { + if (isPending) return; + setFilteredLocales(locales); - }, [locales, setFilteredLocales]); + }, [locales, isPending, setFilteredLocales]); + + useEffect(() => { + if (!config) return; + + const initialLocale = config.locales[0]; + setInitial(initialLocale); + setSelected(initialLocale); + }, [config, setInitial, setSelected]); const onSubmit = async (e) => { e.preventDefault(); const dataForm = new FormData(e.target); const nextLocaleId = JSON.parse(dataForm.get("locale"))?.id; - if (nextLocaleId !== selectedLocales[0]?.id) { - await l10n.setLocales([nextLocaleId]); + if (nextLocaleId !== initial?.id) { + setConfig.mutate({ locales: [nextLocaleId] }); } navigate(".."); }; + if (isPending) { + return {_("Loading")}; + } + let localesList = filteredLocales.map((locale) => { return ( setSelected(locale)} + onChange={() => setSelected(locale.id)} label={ {locale.name} @@ -74,7 +89,7 @@ export default function LocaleSelection() { } value={JSON.stringify(locale)} - checked={locale === selected} + checked={locale.id === selected} /> ); }); diff --git a/web/src/context/l10n.jsx b/web/src/context/l10n.jsx index 52b3e5d87b..80363890ea 100644 --- a/web/src/context/l10n.jsx +++ b/web/src/context/l10n.jsx @@ -45,7 +45,7 @@ function L10nProvider({ children }) { const load = async () => { const timezones = await cancellablePromise(client.l10n.timezones()); const selectedTimezone = await cancellablePromise(client.l10n.getTimezone()); - const locales = await cancellablePromise(client.l10n.locales()); + const locales = []; const selectedLocales = await cancellablePromise(client.l10n.getLocales()); const keymaps = await cancellablePromise(client.l10n.keymaps()); const selectedKeymap = await cancellablePromise(client.l10n.getKeymap()); diff --git a/web/src/queries/l10n.js b/web/src/queries/l10n.js new file mode 100644 index 0000000000..afd8349f45 --- /dev/null +++ b/web/src/queries/l10n.js @@ -0,0 +1,92 @@ +/* + * Copyright (c) [2024] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of version 2 of the GNU General Public License as published + * by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, contact SUSE LLC. + * + * To contact SUSE LLC about this file by physical or electronic mail, you may + * find current contact information at www.suse.com. + */ + +import { useQueryClient, useMutation, useQuery } from "@tanstack/react-query"; +import { timezoneUTCOffset, identity } from "~/utils"; + +const useConfig = (select = identity) => { + return useQuery({ + queryKey: ["l10n", "config"], + queryFn: () => fetch("/api/l10n/config").then((res) => res.json()), + select + }); +}; + +const useLocales = () => useQuery({ + queryKey: ["locales"], + queryFn: async () => { + const response = await fetch("/api/l10n/locales"); + const locales = await response.json(); + return locales.map(({ id, language, territory }) => { + return { id, name: language, territory }; + }); + } +}); + +const useTimezones = () => useQuery({ + queryKey: ["timezones"], + queryFn: async () => { + const response = await fetch("/api/l10n/timezones"); + const timezones = await response.json(); + return timezones.map(({ code, parts, country }) => { + const offset = timezoneUTCOffset(code); + return { id: code, parts, country, offset }; + }); + } +}); + +const useKeymaps = () => useQuery({ + queryKey: ["keymaps"], + queryFn: async () => { + const response = await fetch("/api/l10n/keymaps"); + const keymaps = await response.json(); + return keymaps.map(({ id, description }) => { + return { id, name: description }; + }); + } +}); + +const useConfigMutation = () => { + const queryClient = useQueryClient(); + + const query = { + mutationFn: (newConfig) => + fetch("/api/l10n/config", { + method: "PATCH", + body: JSON.stringify(newConfig), + headers: { + "Content-Type": "application/json", + }, + }), + onSuccess: () => + queryClient.invalidateQueries({ queryKey: ["l10n", "config"] }) + , + }; + return useMutation(query); +}; + +export { + useConfigMutation, + useLocales, + useTimezones, + useKeymaps, + useConfig +}; diff --git a/web/src/utils.js b/web/src/utils.js index d8315dda1b..938e6af327 100644 --- a/web/src/utils.js +++ b/web/src/utils.js @@ -47,6 +47,11 @@ const isObject = (value) => ( */ const noop = () => undefined; +/** + * @return {function} identity function + */ +const identity = (i) => i; + /** * Returns a new array with a given collection split into two groups, the first holding elements * satisfying the filter and the second with those which do not. @@ -373,6 +378,7 @@ const timezoneUTCOffset = (timezone) => { export { noop, + identity, isObject, partition, compact, From f975f0f0fd74292a0be8343badc8b104a757d8cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Tue, 2 Jul 2024 16:38:58 +0100 Subject: [PATCH 03/38] refactor(web): move QueryClientProvider to app context --- web/src/App.jsx | 4 ++-- web/src/context/app.jsx | 25 +++++++++++++++---------- 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/web/src/App.jsx b/web/src/App.jsx index 8d4f716782..edee167beb 100644 --- a/web/src/App.jsx +++ b/web/src/App.jsx @@ -73,10 +73,10 @@ function App() { if (!language) return null; return ( - + <> - + ); } diff --git a/web/src/context/app.jsx b/web/src/context/app.jsx index d92e9d662e..d85d58fb9d 100644 --- a/web/src/context/app.jsx +++ b/web/src/context/app.jsx @@ -27,6 +27,9 @@ import { InstallerL10nProvider } from "./installerL10n"; import { L10nProvider } from "./l10n"; import { ProductProvider } from "./product"; import { IssuesProvider } from "./issues"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; + +const queryClient = new QueryClient(); /** * Combines all application providers. @@ -37,17 +40,19 @@ import { IssuesProvider } from "./issues"; function AppProviders({ children }) { return ( - - - - - {children} - - - - + + + + + + {children} + + + + + ); } -export { AppProviders }; +export { AppProviders, queryClient }; From 503e3722f2afa4652b4e8eebc17afafcb137ae0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 3 Jul 2024 08:53:33 +0100 Subject: [PATCH 04/38] refactor(web): expose the client's websocket --- web/src/client/index.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/web/src/client/index.js b/web/src/client/index.js index 8488197714..e1a7dbf8bd 100644 --- a/web/src/client/index.js +++ b/web/src/client/index.js @@ -129,6 +129,7 @@ const createClient = url => { const isConnected = () => client.ws?.isConnected() || false; const isRecoverable = () => !!client.ws?.isRecoverable(); + const ws = () => client.ws; return { l10n, @@ -145,7 +146,8 @@ const createClient = url => { isConnected, isRecoverable, onConnect: handler => client.ws.onOpen(handler), - onDisconnect: handler => client.ws.onClose(handler) + onDisconnect: handler => client.ws.onClose(handler), + ws: () => client.ws }; }; From e263dbea41d1a6905b080cd07557428b3b50b18c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 3 Jul 2024 08:56:59 +0100 Subject: [PATCH 05/38] refactor(web): use queries to listen for l10n changes --- web/src/components/l10n/L10nPage.jsx | 3 ++- web/src/context/l10n.jsx | 6 ------ web/src/queries/l10n.js | 20 +++++++++++++++++++- 3 files changed, 21 insertions(+), 8 deletions(-) diff --git a/web/src/components/l10n/L10nPage.jsx b/web/src/components/l10n/L10nPage.jsx index 2289ec2042..4f5a706471 100644 --- a/web/src/components/l10n/L10nPage.jsx +++ b/web/src/components/l10n/L10nPage.jsx @@ -28,7 +28,7 @@ import { ButtonLink, CardField, Page } from "~/components/core"; import { _ } from "~/i18n"; import { useL10n } from "~/context/l10n"; import buttonStyles from '@patternfly/react-styles/css/components/Button/button'; -import { useConfig, useLocales } from "../../queries/l10n"; +import { useConfig, useLocales, useL10nConfigChanges } from "../../queries/l10n"; const Section = ({ label, value, children }) => { return ( @@ -53,6 +53,7 @@ export default function L10nPage() { selectedKeymap: keymap, selectedTimezone: timezone, } = useL10n(); + useL10nConfigChanges(); const [locale, setLocale] = useState(); const { data: locales } = useLocales(); diff --git a/web/src/context/l10n.jsx b/web/src/context/l10n.jsx index 80363890ea..418e6875cf 100644 --- a/web/src/context/l10n.jsx +++ b/web/src/context/l10n.jsx @@ -68,12 +68,6 @@ function L10nProvider({ children }) { return client.l10n.onTimezoneChange(setSelectedTimezone); }, [client, setSelectedTimezone]); - useEffect(() => { - if (!client) return; - - return client.l10n.onLocalesChange(setSelectedLocales); - }, [client, setSelectedLocales]); - useEffect(() => { if (!client) return; diff --git a/web/src/queries/l10n.js b/web/src/queries/l10n.js index afd8349f45..ce028080d2 100644 --- a/web/src/queries/l10n.js +++ b/web/src/queries/l10n.js @@ -19,7 +19,9 @@ * find current contact information at www.suse.com. */ +import React from "react"; import { useQueryClient, useMutation, useQuery } from "@tanstack/react-query"; +import { useInstallerClient } from "~/context/installer"; import { timezoneUTCOffset, identity } from "~/utils"; const useConfig = (select = identity) => { @@ -83,10 +85,26 @@ const useConfigMutation = () => { return useMutation(query); }; +const useL10nConfigChanges = () => { + const queryClient = useQueryClient(); + const client = useInstallerClient(); + + React.useEffect(() => { + if (!client) return; + + return client.ws().onEvent(event => { + if (event.type === "L10nConfigChanged") { + queryClient.invalidateQueries({ queryKey: ["l10n", "config"] }); + } + }); + }, [queryClient, client]); +}; + export { useConfigMutation, useLocales, useTimezones, useKeymaps, - useConfig + useConfig, + useL10nConfigChanges }; From 7472ab8b7c4add31c9eac3cf4818369d952d08ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 3 Jul 2024 10:57:11 +0100 Subject: [PATCH 06/38] refactor(web): adapt L10nPage to use queries --- web/src/components/l10n/KeyboardSelection.jsx | 37 +++++++++++++------ web/src/components/l10n/L10nPage.jsx | 31 ++++++---------- web/src/components/l10n/LocaleSelection.jsx | 6 +-- web/src/components/l10n/TimezoneSelection.jsx | 36 +++++++++++++----- web/src/queries/l10n.js | 8 ++-- 5 files changed, 72 insertions(+), 46 deletions(-) diff --git a/web/src/components/l10n/KeyboardSelection.jsx b/web/src/components/l10n/KeyboardSelection.jsx index ca9e18bf67..b8d5b51fd6 100644 --- a/web/src/components/l10n/KeyboardSelection.jsx +++ b/web/src/components/l10n/KeyboardSelection.jsx @@ -27,46 +27,61 @@ import { } from "@patternfly/react-core"; import { useNavigate } from "react-router-dom"; import { _ } from "~/i18n"; -import { useL10n } from "~/context/l10n"; -import { useInstallerClient } from "~/context/installer"; import { ListSearch, Page } from "~/components/core"; import textStyles from '@patternfly/react-styles/css/utilities/Text/text'; +import { useKeymaps, useConfig, useConfigMutation } from "../../queries/l10n"; // TODO: Add documentation and typechecking // TODO: Evaluate if worth it extracting the selector export default function KeyboardSelection() { - const { l10n } = useInstallerClient(); - const { keymaps, selectedKeymap: currentKeymap } = useL10n(); - const [selected, setSelected] = useState(currentKeymap); + const { isPending, data: keymaps } = useKeymaps(); + const { data: config } = useConfig(); + const setConfig = useConfigMutation(); + const [initial, setInitial] = useState(); + const [selected, setSelected] = useState(); const [filteredKeymaps, setFilteredKeymaps] = useState(keymaps); const navigate = useNavigate(); - const sortedKeymaps = keymaps.sort((k1, k2) => k1.name > k2.name ? 1 : -1); const searchHelp = _("Filter by description or keymap code"); useEffect(() => { + if (isPending) return; + + const sortedKeymaps = keymaps.sort((k1, k2) => k1.name > k2.name ? 1 : -1); setFilteredKeymaps(sortedKeymaps); - }, [sortedKeymaps, setFilteredKeymaps]); + }, [isPending, keymaps, setFilteredKeymaps]); + + useEffect(() => { + if (!config) return; + + const initialKeymap = config.keymap; + setInitial(initialKeymap); + setSelected(initialKeymap); + }, [config, setInitial, setSelected]); const onSubmit = async (e) => { e.preventDefault(); const dataForm = new FormData(e.target); const nextKeymapId = JSON.parse(dataForm.get("keymap"))?.id; - if (nextKeymapId !== currentKeymap?.id) { - await l10n.setKeymap(nextKeymapId); + if (nextKeymapId !== initial) { + setConfig.mutate({ keymap: nextKeymapId }); } navigate(".."); }; + if (filteredKeymaps === undefined) { + return {_("Loading")}; + } + let keymapsList = filteredKeymaps.map((keymap) => { return ( setSelected(keymap)} + onChange={() => setSelected(keymap.id)} label={ <> @@ -75,7 +90,7 @@ export default function KeyboardSelection() { } value={JSON.stringify(keymap)} - defaultChecked={keymap === selected} + defaultChecked={keymap.id === selected} /> ); }); diff --git a/web/src/components/l10n/L10nPage.jsx b/web/src/components/l10n/L10nPage.jsx index 4f5a706471..3056e986f1 100644 --- a/web/src/components/l10n/L10nPage.jsx +++ b/web/src/components/l10n/L10nPage.jsx @@ -23,12 +23,9 @@ import React, { useEffect, useState } from "react"; import { Gallery, GalleryItem, } from "@patternfly/react-core"; -import { Link } from "react-router-dom"; import { ButtonLink, CardField, Page } from "~/components/core"; import { _ } from "~/i18n"; -import { useL10n } from "~/context/l10n"; -import buttonStyles from '@patternfly/react-styles/css/components/Button/button'; -import { useConfig, useLocales, useL10nConfigChanges } from "../../queries/l10n"; +import { useConfig, useLocales, useKeymaps, useTimezones, useL10nConfigChanges } from "../../queries/l10n"; const Section = ({ label, value, children }) => { return ( @@ -49,26 +46,22 @@ const Section = ({ label, value, children }) => { * @component */ export default function L10nPage() { - const { - selectedKeymap: keymap, - selectedTimezone: timezone, - } = useL10n(); useL10nConfigChanges(); - const [locale, setLocale] = useState(); - const { data: locales } = useLocales(); - const { isPending, data: localeId } = useConfig((i) => i.locales[0]); + const { isPending: localesPending, data: locales } = useLocales(); + const { isPending: timezonesPending, data: timezones } = useTimezones(); + const { isPending: keymapsPending, data: keymaps } = useKeymaps(); + const { isPending: configPending, data: config } = useConfig(); - useEffect(() => { - if (!locales) return; + if (localesPending || timezonesPending || keymapsPending || configPending) { + return; + } - const found = locales.find((l) => l.id === localeId); - if (found) { - setLocale(found); - } - }, [locales, localeId]); + const { locales: [localeId], keymap: keymapId, timezone: timezoneId } = config; - if (isPending) return; + const locale = locales.find((l) => l.id === localeId); + const keymap = keymaps.find((k) => k.id === keymapId); + const timezone = timezones.find((t) => t.id === timezoneId); return ( <> diff --git a/web/src/components/l10n/LocaleSelection.jsx b/web/src/components/l10n/LocaleSelection.jsx index 6842f74b14..9628e324e8 100644 --- a/web/src/components/l10n/LocaleSelection.jsx +++ b/web/src/components/l10n/LocaleSelection.jsx @@ -48,7 +48,7 @@ export default function LocaleSelection() { if (isPending) return; setFilteredLocales(locales); - }, [locales, isPending, setFilteredLocales]); + }, [isPending, locales, setFilteredLocales]); useEffect(() => { if (!config) return; @@ -63,14 +63,14 @@ export default function LocaleSelection() { const dataForm = new FormData(e.target); const nextLocaleId = JSON.parse(dataForm.get("locale"))?.id; - if (nextLocaleId !== initial?.id) { + if (nextLocaleId !== initial) { setConfig.mutate({ locales: [nextLocaleId] }); } navigate(".."); }; - if (isPending) { + if (filteredLocales === undefined) { return {_("Loading")}; } diff --git a/web/src/components/l10n/TimezoneSelection.jsx b/web/src/components/l10n/TimezoneSelection.jsx index 1b0fcf4b8b..3411e60135 100644 --- a/web/src/components/l10n/TimezoneSelection.jsx +++ b/web/src/components/l10n/TimezoneSelection.jsx @@ -31,8 +31,7 @@ import { ListSearch, Page } from "~/components/core"; import { useNavigate } from "react-router-dom"; import { _ } from "~/i18n"; import { timezoneTime } from "~/utils"; -import { useL10n } from "~/context/l10n"; -import { useInstallerClient } from "~/context/installer"; +import { useTimezones, useConfig, useConfigMutation } from "../../queries/l10n"; import textStyles from '@patternfly/react-styles/css/utilities/Text/text'; let date; @@ -40,7 +39,7 @@ let date; const timezoneWithDetails = (timezone) => { const offset = timezone.utcOffset; - if (offset === undefined) return timezone.id; + if (offset === undefined) return { ...timezone, details: timezone.id }; let utc = "UTC"; if (offset > 0) utc += `+${offset}`; @@ -61,35 +60,54 @@ const sortedTimezones = (timezones) => { // TODO: Refactor timezones/extendedTimezones thingy export default function TimezoneSelection() { date = new Date(); - const { l10n } = useInstallerClient(); - const { timezones, selectedTimezone: currentTimezone } = useL10n(); + const { data: timezones } = useTimezones(); + const { data: config } = useConfig(); + const setConfig = useConfigMutation(); + const [initial, setInitial] = useState(); + const [selected, setSelected] = useState(); const [displayTimezones, setDisplayTimezones] = useState([]); - const [selected, setSelected] = useState(currentTimezone); const [filteredTimezones, setFilteredTimezones] = useState([]); const navigate = useNavigate(); const searchHelp = _("Filter by territory, time zone code or UTC offset"); useEffect(() => { + if (timezones === undefined) return; + + const mapped = timezones.map(timezoneWithDetails); setDisplayTimezones(timezones.map(timezoneWithDetails)); }, [setDisplayTimezones, timezones]); useEffect(() => { + if (displayTimezones === undefined) return; + setFilteredTimezones(sortedTimezones(displayTimezones)); }, [setFilteredTimezones, displayTimezones]); + useEffect(() => { + if (!config) return; + + const initialTimezone = config.timezone; + setInitial(initialTimezone); + setSelected(initialTimezone); + }, [config, setInitial, setSelected]); + const onSubmit = async (e) => { e.preventDefault(); const dataForm = new FormData(e.target); const nextTimezoneId = JSON.parse(dataForm.get("timezone"))?.id; - if (nextTimezoneId !== currentTimezone?.id) { - await l10n.setTimezone(nextTimezoneId); + if (nextTimezoneId !== initial) { + setConfig.mutate({ timezone: nextTimezoneId }); } navigate(".."); }; + if (filteredTimezones === undefined) { + return {_("Loading")}; + } + let timezonesList = filteredTimezones.map((timezone) => { return ( } value={JSON.stringify(timezone)} - defaultChecked={timezone === selected} + defaultChecked={timezone.id === selected} /> ); }); diff --git a/web/src/queries/l10n.js b/web/src/queries/l10n.js index ce028080d2..f9ac3b0337 100644 --- a/web/src/queries/l10n.js +++ b/web/src/queries/l10n.js @@ -33,7 +33,7 @@ const useConfig = (select = identity) => { }; const useLocales = () => useQuery({ - queryKey: ["locales"], + queryKey: ["l10n", "locales"], queryFn: async () => { const response = await fetch("/api/l10n/locales"); const locales = await response.json(); @@ -44,19 +44,19 @@ const useLocales = () => useQuery({ }); const useTimezones = () => useQuery({ - queryKey: ["timezones"], + queryKey: ["l10n", "timezones"], queryFn: async () => { const response = await fetch("/api/l10n/timezones"); const timezones = await response.json(); return timezones.map(({ code, parts, country }) => { const offset = timezoneUTCOffset(code); - return { id: code, parts, country, offset }; + return { id: code, parts, country, utcOffset: offset }; }); } }); const useKeymaps = () => useQuery({ - queryKey: ["keymaps"], + queryKey: ["l10n", "keymaps"], queryFn: async () => { const response = await fetch("/api/l10n/keymaps"); const keymaps = await response.json(); From 5d6232a71bd88006a136c0e37442d7c65876f04c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 3 Jul 2024 11:16:06 +0100 Subject: [PATCH 07/38] refactor(web): adapt L10nSection to use queries --- web/src/components/overview/L10nSection.jsx | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/web/src/components/overview/L10nSection.jsx b/web/src/components/overview/L10nSection.jsx index 2f7955ac43..61654eb5db 100644 --- a/web/src/components/overview/L10nSection.jsx +++ b/web/src/components/overview/L10nSection.jsx @@ -23,15 +23,15 @@ import React from "react"; import { TextContent, Text, TextVariants } from "@patternfly/react-core"; import { Em } from "~/components/core"; import { _ } from "~/i18n"; -import { useL10n } from "~/context/l10n"; +import { useLocales, useConfig } from "~/queries/l10n"; export default function L10nSection() { - const { selectedLocales } = useL10n(); + const { isPending: isLocalesPending, data: locales } = useLocales(); + const { isPending: isConfigPending, data: config } = useConfig(); - const locale = selectedLocales[0]; - if (locale === undefined) { - return; - } + if (isLocalesPending || isConfigPending) return; + + const locale = locales.find((l) => l.id === config?.locales[0]); // TRANSLATORS: %s will be replaced by a language name and territory, example: // "English (United States)". From 2b83836bc1e6adec08670956e1e43b89a399bd05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 3 Jul 2024 11:23:24 +0100 Subject: [PATCH 08/38] refactor(web): use ~/queries instead of relative paths --- web/src/components/l10n/KeyboardSelection.jsx | 2 +- web/src/components/l10n/LocaleSelection.jsx | 2 +- web/src/components/l10n/TimezoneSelection.jsx | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/web/src/components/l10n/KeyboardSelection.jsx b/web/src/components/l10n/KeyboardSelection.jsx index b8d5b51fd6..b2cf84fd74 100644 --- a/web/src/components/l10n/KeyboardSelection.jsx +++ b/web/src/components/l10n/KeyboardSelection.jsx @@ -29,7 +29,7 @@ import { useNavigate } from "react-router-dom"; import { _ } from "~/i18n"; import { ListSearch, Page } from "~/components/core"; import textStyles from '@patternfly/react-styles/css/utilities/Text/text'; -import { useKeymaps, useConfig, useConfigMutation } from "../../queries/l10n"; +import { useKeymaps, useConfig, useConfigMutation } from "~/queries/l10n"; // TODO: Add documentation and typechecking // TODO: Evaluate if worth it extracting the selector diff --git a/web/src/components/l10n/LocaleSelection.jsx b/web/src/components/l10n/LocaleSelection.jsx index 9628e324e8..b4d7362093 100644 --- a/web/src/components/l10n/LocaleSelection.jsx +++ b/web/src/components/l10n/LocaleSelection.jsx @@ -29,7 +29,7 @@ import { useNavigate } from "react-router-dom"; import { _ } from "~/i18n"; import { ListSearch, Page } from "~/components/core"; import textStyles from '@patternfly/react-styles/css/utilities/Text/text'; -import { useLocales, useConfig, useConfigMutation } from "../../queries/l10n"; +import { useLocales, useConfig, useConfigMutation } from "~/queries/l10n"; // TODO: Add documentation and typechecking // TODO: Evaluate if worth it extracting the selector diff --git a/web/src/components/l10n/TimezoneSelection.jsx b/web/src/components/l10n/TimezoneSelection.jsx index 3411e60135..67a05f45ba 100644 --- a/web/src/components/l10n/TimezoneSelection.jsx +++ b/web/src/components/l10n/TimezoneSelection.jsx @@ -31,7 +31,7 @@ import { ListSearch, Page } from "~/components/core"; import { useNavigate } from "react-router-dom"; import { _ } from "~/i18n"; import { timezoneTime } from "~/utils"; -import { useTimezones, useConfig, useConfigMutation } from "../../queries/l10n"; +import { useTimezones, useConfig, useConfigMutation } from "~/queries/l10n"; import textStyles from '@patternfly/react-styles/css/utilities/Text/text'; let date; From eeeb61a0fac23568acb07ab4e3293ca063f5ca24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 3 Jul 2024 12:27:33 +0100 Subject: [PATCH 09/38] refactor(web): use functions to build queries --- web/src/components/l10n/KeyboardSelection.jsx | 7 +++--- web/src/components/l10n/L10nPage.jsx | 19 +++++++++----- web/src/components/l10n/LocaleSelection.jsx | 7 +++--- web/src/components/l10n/TimezoneSelection.jsx | 7 +++--- web/src/components/overview/L10nSection.jsx | 7 +++--- web/src/queries/l10n.js | 25 +++++++++---------- 6 files changed, 41 insertions(+), 31 deletions(-) diff --git a/web/src/components/l10n/KeyboardSelection.jsx b/web/src/components/l10n/KeyboardSelection.jsx index b2cf84fd74..b43ebc417d 100644 --- a/web/src/components/l10n/KeyboardSelection.jsx +++ b/web/src/components/l10n/KeyboardSelection.jsx @@ -29,13 +29,14 @@ import { useNavigate } from "react-router-dom"; import { _ } from "~/i18n"; import { ListSearch, Page } from "~/components/core"; import textStyles from '@patternfly/react-styles/css/utilities/Text/text'; -import { useKeymaps, useConfig, useConfigMutation } from "~/queries/l10n"; +import { keymapsQuery, configQuery, useConfigMutation } from "~/queries/l10n"; +import { useQuery } from "@tanstack/react-query"; // TODO: Add documentation and typechecking // TODO: Evaluate if worth it extracting the selector export default function KeyboardSelection() { - const { isPending, data: keymaps } = useKeymaps(); - const { data: config } = useConfig(); + const { isPending, data: keymaps } = useQuery(keymapsQuery()); + const { data: config } = useQuery(configQuery()); const setConfig = useConfigMutation(); const [initial, setInitial] = useState(); const [selected, setSelected] = useState(); diff --git a/web/src/components/l10n/L10nPage.jsx b/web/src/components/l10n/L10nPage.jsx index 3056e986f1..2aa3b2c696 100644 --- a/web/src/components/l10n/L10nPage.jsx +++ b/web/src/components/l10n/L10nPage.jsx @@ -19,13 +19,20 @@ * find language contact information at www.suse.com. */ -import React, { useEffect, useState } from "react"; +import React from "react"; import { Gallery, GalleryItem, } from "@patternfly/react-core"; import { ButtonLink, CardField, Page } from "~/components/core"; import { _ } from "~/i18n"; -import { useConfig, useLocales, useKeymaps, useTimezones, useL10nConfigChanges } from "../../queries/l10n"; +import { useQuery } from "@tanstack/react-query"; +import { + configQuery, + localesQuery, + keymapsQuery, + timezonesQuery, + useL10nConfigChanges +} from "~/queries/l10n"; const Section = ({ label, value, children }) => { return ( @@ -48,10 +55,10 @@ const Section = ({ label, value, children }) => { export default function L10nPage() { useL10nConfigChanges(); - const { isPending: localesPending, data: locales } = useLocales(); - const { isPending: timezonesPending, data: timezones } = useTimezones(); - const { isPending: keymapsPending, data: keymaps } = useKeymaps(); - const { isPending: configPending, data: config } = useConfig(); + const { isPending: localesPending, data: locales } = useQuery(localesQuery()); + const { isPending: timezonesPending, data: timezones } = useQuery(timezonesQuery()); + const { isPending: keymapsPending, data: keymaps } = useQuery(keymapsQuery()); + const { isPending: configPending, data: config } = useQuery(configQuery()); if (localesPending || timezonesPending || keymapsPending || configPending) { return; diff --git a/web/src/components/l10n/LocaleSelection.jsx b/web/src/components/l10n/LocaleSelection.jsx index b4d7362093..5f4770a4cb 100644 --- a/web/src/components/l10n/LocaleSelection.jsx +++ b/web/src/components/l10n/LocaleSelection.jsx @@ -29,13 +29,14 @@ import { useNavigate } from "react-router-dom"; import { _ } from "~/i18n"; import { ListSearch, Page } from "~/components/core"; import textStyles from '@patternfly/react-styles/css/utilities/Text/text'; -import { useLocales, useConfig, useConfigMutation } from "~/queries/l10n"; +import { useQuery } from "@tanstack/react-query"; +import { localesQuery, configQuery, useConfigMutation } from "~/queries/l10n"; // TODO: Add documentation and typechecking // TODO: Evaluate if worth it extracting the selector export default function LocaleSelection() { - const { isPending, data: locales } = useLocales(); - const { data: config } = useConfig(); + const { isPending, data: locales } = useQuery(localesQuery()); + const { data: config } = useQuery(configQuery()); const setConfig = useConfigMutation(); const [initial, setInitial] = useState(); const [selected, setSelected] = useState(); diff --git a/web/src/components/l10n/TimezoneSelection.jsx b/web/src/components/l10n/TimezoneSelection.jsx index 67a05f45ba..cf41ea99df 100644 --- a/web/src/components/l10n/TimezoneSelection.jsx +++ b/web/src/components/l10n/TimezoneSelection.jsx @@ -28,10 +28,11 @@ import { Text } from "@patternfly/react-core"; import { ListSearch, Page } from "~/components/core"; +import { useQuery } from "@tanstack/react-query"; import { useNavigate } from "react-router-dom"; import { _ } from "~/i18n"; import { timezoneTime } from "~/utils"; -import { useTimezones, useConfig, useConfigMutation } from "~/queries/l10n"; +import { timezonesQuery, configQuery, useConfigMutation } from "~/queries/l10n"; import textStyles from '@patternfly/react-styles/css/utilities/Text/text'; let date; @@ -60,8 +61,8 @@ const sortedTimezones = (timezones) => { // TODO: Refactor timezones/extendedTimezones thingy export default function TimezoneSelection() { date = new Date(); - const { data: timezones } = useTimezones(); - const { data: config } = useConfig(); + const { data: timezones } = useQuery(timezonesQuery()); + const { data: config } = useQuery(configQuery()); const setConfig = useConfigMutation(); const [initial, setInitial] = useState(); const [selected, setSelected] = useState(); diff --git a/web/src/components/overview/L10nSection.jsx b/web/src/components/overview/L10nSection.jsx index 61654eb5db..1b2c8112dc 100644 --- a/web/src/components/overview/L10nSection.jsx +++ b/web/src/components/overview/L10nSection.jsx @@ -23,11 +23,12 @@ import React from "react"; import { TextContent, Text, TextVariants } from "@patternfly/react-core"; import { Em } from "~/components/core"; import { _ } from "~/i18n"; -import { useLocales, useConfig } from "~/queries/l10n"; +import { localesQuery, configQuery } from "~/queries/l10n"; +import { useQuery } from "@tanstack/react-query"; export default function L10nSection() { - const { isPending: isLocalesPending, data: locales } = useLocales(); - const { isPending: isConfigPending, data: config } = useConfig(); + const { isPending: isLocalesPending, data: locales } = useQuery(localesQuery()); + const { isPending: isConfigPending, data: config } = useQuery(configQuery()); if (isLocalesPending || isConfigPending) return; diff --git a/web/src/queries/l10n.js b/web/src/queries/l10n.js index f9ac3b0337..0d02798a0a 100644 --- a/web/src/queries/l10n.js +++ b/web/src/queries/l10n.js @@ -20,19 +20,18 @@ */ import React from "react"; -import { useQueryClient, useMutation, useQuery } from "@tanstack/react-query"; +import { useQueryClient, useMutation } from "@tanstack/react-query"; import { useInstallerClient } from "~/context/installer"; -import { timezoneUTCOffset, identity } from "~/utils"; +import { timezoneUTCOffset } from "~/utils"; -const useConfig = (select = identity) => { - return useQuery({ +const configQuery = () => { + return { queryKey: ["l10n", "config"], queryFn: () => fetch("/api/l10n/config").then((res) => res.json()), - select - }); + }; }; -const useLocales = () => useQuery({ +const localesQuery = () => ({ queryKey: ["l10n", "locales"], queryFn: async () => { const response = await fetch("/api/l10n/locales"); @@ -43,7 +42,7 @@ const useLocales = () => useQuery({ } }); -const useTimezones = () => useQuery({ +const timezonesQuery = () => ({ queryKey: ["l10n", "timezones"], queryFn: async () => { const response = await fetch("/api/l10n/timezones"); @@ -55,7 +54,7 @@ const useTimezones = () => useQuery({ } }); -const useKeymaps = () => useQuery({ +const keymapsQuery = () => ({ queryKey: ["l10n", "keymaps"], queryFn: async () => { const response = await fetch("/api/l10n/keymaps"); @@ -101,10 +100,10 @@ const useL10nConfigChanges = () => { }; export { + configQuery, + keymapsQuery, + localesQuery, + timezonesQuery, useConfigMutation, - useLocales, - useTimezones, - useKeymaps, - useConfig, useL10nConfigChanges }; From 82c20df358d14bbb7d2a9cf2373c2fea04de808e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 3 Jul 2024 12:31:41 +0100 Subject: [PATCH 10/38] fix(web): minimize l10n data retrieving --- web/src/queries/l10n.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/web/src/queries/l10n.js b/web/src/queries/l10n.js index 0d02798a0a..a760fdae76 100644 --- a/web/src/queries/l10n.js +++ b/web/src/queries/l10n.js @@ -39,7 +39,8 @@ const localesQuery = () => ({ return locales.map(({ id, language, territory }) => { return { id, name: language, territory }; }); - } + }, + staleTime: Infinity }); const timezonesQuery = () => ({ @@ -51,7 +52,8 @@ const timezonesQuery = () => ({ const offset = timezoneUTCOffset(code); return { id: code, parts, country, utcOffset: offset }; }); - } + }, + staleTime: Infinity }); const keymapsQuery = () => ({ @@ -62,7 +64,8 @@ const keymapsQuery = () => ({ return keymaps.map(({ id, description }) => { return { id, name: description }; }); - } + }, + staleTime: Infinity }); const useConfigMutation = () => { From 37a7e8088058e74fb300e958be41672291294b92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Wed, 3 Jul 2024 12:30:08 +0100 Subject: [PATCH 11/38] refactor(web): move l10n routes to ~/routes --- web/src/components/l10n/L10nPage.jsx | 15 ++++---- web/src/components/l10n/routes.js | 57 ---------------------------- web/src/router.js | 2 +- 3 files changed, 8 insertions(+), 66 deletions(-) delete mode 100644 web/src/components/l10n/routes.js diff --git a/web/src/components/l10n/L10nPage.jsx b/web/src/components/l10n/L10nPage.jsx index 2aa3b2c696..727aab573d 100644 --- a/web/src/components/l10n/L10nPage.jsx +++ b/web/src/components/l10n/L10nPage.jsx @@ -20,12 +20,9 @@ */ import React from "react"; -import { - Gallery, GalleryItem, -} from "@patternfly/react-core"; -import { ButtonLink, CardField, Page } from "~/components/core"; -import { _ } from "~/i18n"; +import { Gallery, GalleryItem, } from "@patternfly/react-core"; import { useQuery } from "@tanstack/react-query"; +import { ButtonLink, CardField, Page } from "~/components/core"; import { configQuery, localesQuery, @@ -33,6 +30,8 @@ import { timezonesQuery, useL10nConfigChanges } from "~/queries/l10n"; +import { LOCALE_SELECTION_PATH, KEYMAP_SELECTION_PATH, TIMEZONE_SELECTION_PATH } from "~/routes/l10n"; +import { _ } from "~/i18n"; const Section = ({ label, value, children }) => { return ( @@ -83,7 +82,7 @@ export default function L10nPage() { label={_("Language")} value={locale ? `${locale.name} - ${locale.territory}` : _("Not selected yet")} > - + {locale ? _("Change") : _("Select")} @@ -94,7 +93,7 @@ export default function L10nPage() { label={_("Keyboard")} value={keymap ? keymap.name : _("Not selected yet")} > - + {keymap ? _("Change") : _("Select")} @@ -105,7 +104,7 @@ export default function L10nPage() { label={_("Time zone")} value={timezone ? (timezone.parts || []).join(' - ') : _("Not selected yet")} > - + {timezone ? _("Change") : _("Select")} diff --git a/web/src/components/l10n/routes.js b/web/src/components/l10n/routes.js deleted file mode 100644 index 1746dd782b..0000000000 --- a/web/src/components/l10n/routes.js +++ /dev/null @@ -1,57 +0,0 @@ -/* - * Copyright (c) [2024] SUSE LLC - * - * All Rights Reserved. - * - * This program is free software; you can redistribute it and/or modify it - * under the terms of version 2 of the GNU General Public License as published - * by the Free Software Foundation. - * - * This program is distributed in the hope that it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for - * more details. - * - * You should have received a copy of the GNU General Public License along - * with this program; if not, contact SUSE LLC. - * - * To contact SUSE LLC about this file by physical or electronic mail, you may - * find current contact information at www.suse.com. - */ - -import React from "react"; -import { Page } from "~/components/core"; -import L10nPage from "./L10nPage"; -import LocaleSelection from "./LocaleSelection"; -import KeymapSelection from "./KeyboardSelection"; -import TimezoneSelection from "./TimezoneSelection"; -import { N_ } from "~/i18n"; - -const routes = { - path: "/l10n", - element: , - handle: { - name: N_("Localization"), - icon: "globe" - }, - children: [ - { - index: true, - element: - }, - { - path: "language/select", - element: , - }, - { - path: "keymap/select", - element: , - }, - { - path: "timezone/select", - element: , - } - ] -}; - -export default routes; diff --git a/web/src/router.js b/web/src/router.js index 31628d73d1..5b4596ddd5 100644 --- a/web/src/router.js +++ b/web/src/router.js @@ -29,7 +29,7 @@ import { LoginPage } from "~/components/core"; import { OverviewPage } from "~/components/overview"; import { _ } from "~/i18n"; import overviewRoutes from "~/components/overview/routes"; -import l10nRoutes from "~/components/l10n/routes"; +import l10nRoutes from "~/routes/l10n"; import networkRoutes from "~/components/network/routes"; import { productsRoute } from "~/components/product/routes"; import storageRoutes from "~/components/storage/routes"; From 227c42f67cbc772481158c696113e534524efa81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 3 Jul 2024 12:58:30 +0100 Subject: [PATCH 12/38] refactor(web): adapt InstallerOptions to use queries --- web/src/components/core/InstallerOptions.jsx | 5 ++++- web/src/queries/l10n.js | 5 +++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/web/src/components/core/InstallerOptions.jsx b/web/src/components/core/InstallerOptions.jsx index ba241b968d..9f04bb59fd 100644 --- a/web/src/components/core/InstallerOptions.jsx +++ b/web/src/components/core/InstallerOptions.jsx @@ -31,6 +31,8 @@ import { localConnection } from "~/utils"; import { useInstallerL10n } from "~/context/installerL10n"; import supportedLanguages from "~/languages.json"; import { useL10n } from "~/context/l10n"; +import { useQuery } from "@tanstack/react-query"; +import { keymapsQuery } from "~/queries/l10n"; /** * @typedef {import("@patternfly/react-core").ButtonProps} ButtonProps @@ -50,13 +52,14 @@ export default function InstallerOptions() { } = useInstallerL10n(); const [language, setLanguage] = useState(initialLanguage); const [keymap, setKeymap] = useState(initialKeymap); - const { keymaps } = useL10n(); + const { isPending, data: keymaps } = useQuery(keymapsQuery()); const location = useLocation(); const [isOpen, setIsOpen] = useState(false); const [inProgress, setInProgress] = useState(false); // FIXME: Installer options should be available in the login too. if (["/login", "/products/progress"].includes(location.pathname)) return; + if (isPending) return; const open = () => setIsOpen(true); const close = () => { diff --git a/web/src/queries/l10n.js b/web/src/queries/l10n.js index a760fdae76..1c288b3c67 100644 --- a/web/src/queries/l10n.js +++ b/web/src/queries/l10n.js @@ -60,10 +60,11 @@ const keymapsQuery = () => ({ queryKey: ["l10n", "keymaps"], queryFn: async () => { const response = await fetch("/api/l10n/keymaps"); - const keymaps = await response.json(); - return keymaps.map(({ id, description }) => { + const json = await response.json(); + const keymaps = json.map(({ id, description }) => { return { id, name: description }; }); + return keymaps.sort((a, b) => (a.name < b.name) ? -1 : 1); }, staleTime: Infinity }); From 179b65e5a3bdde0c0ac4593bed5293f20881a713 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Wed, 3 Jul 2024 13:16:31 +0100 Subject: [PATCH 13/38] fix(web): add missing file Forgotten at 37a7e8088058e74fb300e958be41672291294b92 --- web/src/routes/l10n.js | 88 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 88 insertions(+) create mode 100644 web/src/routes/l10n.js diff --git a/web/src/routes/l10n.js b/web/src/routes/l10n.js new file mode 100644 index 0000000000..0df326b33a --- /dev/null +++ b/web/src/routes/l10n.js @@ -0,0 +1,88 @@ +/* + * Copyright (c) [2024] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of version 2 of the GNU General Public License as published + * by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, contact SUSE LLC. + * + * To contact SUSE LLC about this file by physical or electronic mail, you may + * find current contact information at www.suse.com. + */ + +import React from "react"; +import { Page } from "~/components/core"; +import { L10nPage, LocaleSelection, KeymapSelection, TimezoneSelection } from "~/components/l10n"; +import { queryClient } from "~/context/app"; +import { + configQuery, + localesQuery, + keymapsQuery, + timezonesQuery, + useL10nConfigChanges +} from "~/queries/l10n"; +import { N_ } from "~/i18n"; + +const L10N_PATH = "/l10n"; +const LOCALE_SELECTION_PATH = "locale/select"; +const KEYMAP_SELECTION_PATH = "keymap/select"; +const TIMEZONE_SELECTION_PATH = "timezone/select"; + +const l10nLoader = async () => { + const config = await queryClient.fetchQuery(configQuery()); + const locales = await queryClient.fetchQuery(localesQuery()); + const keymaps = await queryClient.fetchQuery(keymapsQuery()); + const timezones = await queryClient.fetchQuery(timezonesQuery()); + + const { locales: [localeId], keymap: keymapId, timezone: timezoneId } = config; + const locale = locales.find((l) => l.id === localeId); + const keymap = keymaps.find((k) => k.id === keymapId); + const timezone = timezones.find((t) => t.id === timezoneId); + + return { locale, keymap, timezone }; +}; + +const routes = { + path: L10N_PATH, + element: , + loader: l10nLoader, + handle: { + name: N_("Localization"), + icon: "globe" + }, + children: [ + { + index: true, + element: + }, + { + path: LOCALE_SELECTION_PATH, + element: , + }, + { + path: KEYMAP_SELECTION_PATH, + element: , + }, + { + path: TIMEZONE_SELECTION_PATH, + element: , + } + ] +}; + +export default routes; +export { + L10N_PATH, + LOCALE_SELECTION_PATH, + KEYMAP_SELECTION_PATH, + TIMEZONE_SELECTION_PATH +}; From 79ab6076d946755feae9c0180280f22092569b5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 3 Jul 2024 15:59:33 +0100 Subject: [PATCH 14/38] refactor(web): drop the l10n context --- web/src/context/app.jsx | 13 ++--- web/src/context/l10n.jsx | 118 --------------------------------------- web/src/test-utils.js | 5 +- 3 files changed, 6 insertions(+), 130 deletions(-) delete mode 100644 web/src/context/l10n.jsx diff --git a/web/src/context/app.jsx b/web/src/context/app.jsx index d85d58fb9d..d22ccce51e 100644 --- a/web/src/context/app.jsx +++ b/web/src/context/app.jsx @@ -24,7 +24,6 @@ import React from "react"; import { InstallerClientProvider } from "./installer"; import { InstallerL10nProvider } from "./installerL10n"; -import { L10nProvider } from "./l10n"; import { ProductProvider } from "./product"; import { IssuesProvider } from "./issues"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; @@ -42,13 +41,11 @@ function AppProviders({ children }) { - - - - {children} - - - + + + {children} + + diff --git a/web/src/context/l10n.jsx b/web/src/context/l10n.jsx deleted file mode 100644 index 418e6875cf..0000000000 --- a/web/src/context/l10n.jsx +++ /dev/null @@ -1,118 +0,0 @@ -/* - * Copyright (c) [2023] SUSE LLC - * - * All Rights Reserved. - * - * This program is free software; you can redistribute it and/or modify it - * under the terms of version 2 of the GNU General Public License as published - * by the Free Software Foundation. - * - * This program is distributed in the hope that it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for - * more details. - * - * You should have received a copy of the GNU General Public License along - * with this program; if not, contact SUSE LLC. - * - * To contact SUSE LLC about this file by physical or electronic mail, you may - * find current contact information at www.suse.com. - */ - -import React, { useContext, useEffect, useState } from "react"; -import { useCancellablePromise } from "~/utils"; -import { useInstallerClient } from "./installer"; - -/** - * @typedef {import ("~/client/l10n").Locale} Locale - * @typedef {import ("~/client/l10n").Keymap} Keymap - * @typedef {import ("~/client/l10n").Timezone} Timezone - */ - -const L10nContext = React.createContext({}); - -function L10nProvider({ children }) { - const client = useInstallerClient(); - const { cancellablePromise } = useCancellablePromise(); - const [timezones, setTimezones] = useState(); - const [selectedTimezone, setSelectedTimezone] = useState(); - const [locales, setLocales] = useState(); - const [selectedLocales, setSelectedLocales] = useState(); - const [keymaps, setKeymaps] = useState(); - const [selectedKeymap, setSelectedKeymap] = useState(); - - useEffect(() => { - const load = async () => { - const timezones = await cancellablePromise(client.l10n.timezones()); - const selectedTimezone = await cancellablePromise(client.l10n.getTimezone()); - const locales = []; - const selectedLocales = await cancellablePromise(client.l10n.getLocales()); - const keymaps = await cancellablePromise(client.l10n.keymaps()); - const selectedKeymap = await cancellablePromise(client.l10n.getKeymap()); - setTimezones(timezones); - setSelectedTimezone(selectedTimezone); - setLocales(locales); - setSelectedLocales(selectedLocales); - setKeymaps(keymaps); - setSelectedKeymap(selectedKeymap); - }; - - if (client) { - load().catch(console.error); - } - }, [cancellablePromise, client, setKeymaps, setLocales, setSelectedKeymap, setSelectedLocales, setSelectedTimezone, setTimezones]); - - useEffect(() => { - if (!client) return; - - return client.l10n.onTimezoneChange(setSelectedTimezone); - }, [client, setSelectedTimezone]); - - useEffect(() => { - if (!client) return; - - return client.l10n.onKeymapChange(setSelectedKeymap); - }, [client, setSelectedKeymap]); - - const value = { timezones, selectedTimezone, locales, selectedLocales, keymaps, selectedKeymap }; - return {children}; -} - -/** - * Localization context. - * @function - * - * @typedef {object} L10nContext - * @property {Locale[]} locales - * @property {Keymap[]} keymaps - * @property {Timezone[]} timezones - * @property {Locale[]} selectedLocales - * @property {Keymap|undefined} selectedKeymap - * @property {Timezone|undefined} selectedTimezone - * - * @returns {L10nContext} - */ -function useL10n() { - const context = useContext(L10nContext); - - if (!context) { - throw new Error("useL10n must be used within a L10nProvider"); - } - - const { - timezones = [], - selectedTimezone: selectedTimezoneId, - locales = [], - selectedLocales: selectedLocalesId = [], - keymaps = [], - selectedKeymap: selectedKeymapId - } = context; - - const selectedTimezone = timezones.find(t => t.id === selectedTimezoneId); - const selectedLocales = selectedLocalesId.map(id => locales.find(l => l.id === id)); - const selectedKeymap = keymaps.find(k => k.id === selectedKeymapId); - - return { timezones, selectedTimezone, locales, selectedLocales, keymaps, selectedKeymap }; -} - -export { L10nProvider, useL10n }; diff --git a/web/src/test-utils.js b/web/src/test-utils.js index 7f64aa3602..19026c8f93 100644 --- a/web/src/test-utils.js +++ b/web/src/test-utils.js @@ -35,7 +35,6 @@ import { InstallerClientProvider } from "~/context/installer"; import { noop, isObject } from "./utils"; import cockpit from "./lib/cockpit"; import { InstallerL10nProvider } from "./context/installerL10n"; -import { L10nProvider } from "./context/l10n"; /** * Internal mock for manipulating routes, using ["/"] by default @@ -100,9 +99,7 @@ const Providers = ({ children, withL10n }) => { return ( - - {children} - + {children} ); From 4c02c9c36a5b6267f08db6c40149222557b083cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 3 Jul 2024 16:00:47 +0100 Subject: [PATCH 15/38] test(web): adapt test-utils to react-query --- web/src/test-utils.js | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/web/src/test-utils.js b/web/src/test-utils.js index 19026c8f93..cc9c3fd988 100644 --- a/web/src/test-utils.js +++ b/web/src/test-utils.js @@ -35,6 +35,7 @@ import { InstallerClientProvider } from "~/context/installer"; import { noop, isObject } from "./utils"; import cockpit from "./lib/cockpit"; import { InstallerL10nProvider } from "./context/installerL10n"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; /** * Internal mock for manipulating routes, using ["/"] by default @@ -119,10 +120,14 @@ const Providers = ({ children, withL10n }) => { * @see #plainRender for rendering without installer providers */ const installerRender = (ui, options = {}) => { + const queryClient = new QueryClient({}); + const Wrapper = ({ children }) => ( - {children} + + {children} + ); @@ -156,6 +161,25 @@ const plainRender = (ui, options = {}) => { ); }; +/** + * Wrapper around react-testing-library#render for rendering components with the + * QueryClientProvider from TanStack Query. + * + * @todo Unify the render functions once the HTTP client has been replaced with + * TanStack Query. + */ +const queryRender = (ui, options = {}) => { + const queryClient = new QueryClient({}); + + const wrapper = ({ children }) => ( + + {children} + + ); + + return plainRender(ui, {...options, wrapper}); +} + /** * Creates a function to register callbacks * @@ -208,6 +232,7 @@ const resetLocalStorage = (initialState) => { export { plainRender, installerRender, + queryRender, createCallbackMock, mockGettext, mockNavigateFn, From 059ba548c03ce0a472b53adfc821052d7df64f56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 3 Jul 2024 15:56:05 +0100 Subject: [PATCH 16/38] test(web): adapt basic L10nPage test to react-query --- web/src/components/core/InstallerOptions.jsx | 1 - web/src/components/l10n/L10nPage.test.jsx | 73 +++++++++----------- 2 files changed, 33 insertions(+), 41 deletions(-) diff --git a/web/src/components/core/InstallerOptions.jsx b/web/src/components/core/InstallerOptions.jsx index 9f04bb59fd..440ad04545 100644 --- a/web/src/components/core/InstallerOptions.jsx +++ b/web/src/components/core/InstallerOptions.jsx @@ -30,7 +30,6 @@ import { _ } from "~/i18n"; import { localConnection } from "~/utils"; import { useInstallerL10n } from "~/context/installerL10n"; import supportedLanguages from "~/languages.json"; -import { useL10n } from "~/context/l10n"; import { useQuery } from "@tanstack/react-query"; import { keymapsQuery } from "~/queries/l10n"; diff --git a/web/src/components/l10n/L10nPage.test.jsx b/web/src/components/l10n/L10nPage.test.jsx index a4c675c510..72613de5d5 100644 --- a/web/src/components/l10n/L10nPage.test.jsx +++ b/web/src/components/l10n/L10nPage.test.jsx @@ -20,11 +20,13 @@ */ import React from "react"; -import { screen, waitFor, within } from "@testing-library/react"; +import { render, screen, waitFor, within } from "@testing-library/react"; -import { installerRender, plainRender } from "~/test-utils"; -import { L10nPage } from "~/components/l10n"; -import { createClient } from "~/client"; +import { installerRender, plainRender, queryRender } from "~/test-utils"; +import L10nPage from "~/components/l10n/L10nPage"; +import { QueryClient } from "@tanstack/query-core"; +import { QueryClientProvider } from "@tanstack/react-query"; +import { MemoryRouter } from "react-router"; const locales = [ { id: "de_DE.UTF8", name: "German", territory: "Germany" }, @@ -44,53 +46,44 @@ const timezones = [ { id: "america/new_york", parts: ["Americas", "New York"] } ]; +jest.mock("~/queries/l10n", () => ({ + localesQuery: () => ({ + queryKey: ["l10n", "locales"], + queryFn: jest.fn().mockResolvedValue(locales) + }), + timezonesQuery: () => ({ + queryKey: ["l10n", "timezones"], + queryFn: jest.fn().mockResolvedValue(timezones) + }), + keymapsQuery: () => ({ + queryKey: ["l10n", "keymaps"], + queryFn: jest.fn().mockResolvedValue(keymaps) + }), + configQuery: () => ({ + queryKey: ["l10n", "config"], + queryFn: jest.fn().mockResolvedValue({ + locales: mockSelectedLocales, + timezone: mockSelectedTimezone, + keymap: mockSelectedKeymap + }) + }), + useL10nConfigChanges: jest.fn() +})); + let mockL10nClient; let mockSelectedLocales; let mockSelectedKeymap; let mockSelectedTimezone; -jest.mock("~/client"); - -jest.mock("~/context/l10n", () => ({ - ...jest.requireActual("~/context/l10n"), - useL10n: () => ({ - locales, - selectedLocales: mockSelectedLocales, - keymaps, - selectedKeymap: mockSelectedKeymap, - timezones, - selectedTimezone: mockSelectedTimezone - }) -})); - -jest.mock("~/context/product", () => ({ - ...jest.requireActual("~/context/product"), - useProduct: () => ({ - selectedProduct : { name: "Test" } - }) -})); - -createClient.mockImplementation(() => ( - { - l10n: mockL10nClient - } -)); - beforeEach(() => { - mockL10nClient = { - setLocales: jest.fn().mockResolvedValue(), - setKeymap: jest.fn().mockResolvedValue(), - setTimezone: jest.fn().mockResolvedValue() - }; - mockSelectedLocales = []; mockSelectedKeymap = undefined; mockSelectedTimezone = undefined; }); -it.skip("renders a section for configuring the language", () => { - plainRender(); - screen.getByText("Language"); +it.only("renders a section for configuring the language", async () => { + queryRender(); + await screen.findByText("Language"); }); describe.skip("if there is no selected language", () => { From 8bbac2c208da1fc704699ebf36d475a7297def60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Wed, 3 Jul 2024 17:00:18 +0100 Subject: [PATCH 17/38] refactor(web): improve l10n selectors using RR loaders Part of the code was accidentally sent as part of 179b65e5a3bd. --- web/src/components/l10n/KeyboardSelection.jsx | 72 +++++----------- web/src/components/l10n/L10nPage.jsx | 26 +----- web/src/components/l10n/LocaleSelection.jsx | 71 +++++----------- web/src/components/l10n/TimezoneSelection.jsx | 82 +++++-------------- web/src/routes/l10n.js | 8 +- 5 files changed, 71 insertions(+), 188 deletions(-) diff --git a/web/src/components/l10n/KeyboardSelection.jsx b/web/src/components/l10n/KeyboardSelection.jsx index b43ebc417d..12dddfe100 100644 --- a/web/src/components/l10n/KeyboardSelection.jsx +++ b/web/src/components/l10n/KeyboardSelection.jsx @@ -19,79 +19,49 @@ * find current contact information at www.suse.com. */ -import React, { useEffect, useState } from "react"; -import { - Form, FormGroup, - Radio, - Text -} from "@patternfly/react-core"; -import { useNavigate } from "react-router-dom"; -import { _ } from "~/i18n"; +import React, { useState } from "react"; +import { Form, FormGroup, Radio, Text } from "@patternfly/react-core"; +import { useLoaderData, useNavigate } from "react-router-dom"; import { ListSearch, Page } from "~/components/core"; +import { _ } from "~/i18n"; +import { useConfigMutation } from "~/queries/l10n"; import textStyles from '@patternfly/react-styles/css/utilities/Text/text'; -import { keymapsQuery, configQuery, useConfigMutation } from "~/queries/l10n"; -import { useQuery } from "@tanstack/react-query"; // TODO: Add documentation and typechecking // TODO: Evaluate if worth it extracting the selector export default function KeyboardSelection() { - const { isPending, data: keymaps } = useQuery(keymapsQuery()); - const { data: config } = useQuery(configQuery()); - const setConfig = useConfigMutation(); - const [initial, setInitial] = useState(); - const [selected, setSelected] = useState(); - const [filteredKeymaps, setFilteredKeymaps] = useState(keymaps); const navigate = useNavigate(); + const setConfig = useConfigMutation(); + const { keymaps, keymap: currentKeymap } = useLoaderData(); + const [selected, setSelected] = useState(currentKeymap.id); + const [filteredKeymaps, setFilteredKeymaps] = useState( + keymaps.sort((k1, k2) => k1.name > k2.name ? 1 : -1) + ); const searchHelp = _("Filter by description or keymap code"); - useEffect(() => { - if (isPending) return; - - const sortedKeymaps = keymaps.sort((k1, k2) => k1.name > k2.name ? 1 : -1); - setFilteredKeymaps(sortedKeymaps); - }, [isPending, keymaps, setFilteredKeymaps]); - - useEffect(() => { - if (!config) return; - - const initialKeymap = config.keymap; - setInitial(initialKeymap); - setSelected(initialKeymap); - }, [config, setInitial, setSelected]); - const onSubmit = async (e) => { e.preventDefault(); - const dataForm = new FormData(e.target); - const nextKeymapId = JSON.parse(dataForm.get("keymap"))?.id; - - if (nextKeymapId !== initial) { - setConfig.mutate({ keymap: nextKeymapId }); - } - - navigate(".."); + setConfig.mutate({ keymap: selected }); + navigate(-1); }; - if (filteredKeymaps === undefined) { - return {_("Loading")}; - } - - let keymapsList = filteredKeymaps.map((keymap) => { + let keymapsList = filteredKeymaps.map(({ id, name }) => { return ( setSelected(keymap.id)} + onChange={() => setSelected(id)} label={ <> - {keymap.name} - {keymap.id} + {name} + {id} } - value={JSON.stringify(keymap)} - defaultChecked={keymap.id === selected} + value={id} + isChecked={id === selected} /> ); }); diff --git a/web/src/components/l10n/L10nPage.jsx b/web/src/components/l10n/L10nPage.jsx index 727aab573d..c1a9a02db7 100644 --- a/web/src/components/l10n/L10nPage.jsx +++ b/web/src/components/l10n/L10nPage.jsx @@ -21,13 +21,9 @@ import React from "react"; import { Gallery, GalleryItem, } from "@patternfly/react-core"; -import { useQuery } from "@tanstack/react-query"; +import { useLoaderData } from "react-router-dom"; import { ButtonLink, CardField, Page } from "~/components/core"; import { - configQuery, - localesQuery, - keymapsQuery, - timezonesQuery, useL10nConfigChanges } from "~/queries/l10n"; import { LOCALE_SELECTION_PATH, KEYMAP_SELECTION_PATH, TIMEZONE_SELECTION_PATH } from "~/routes/l10n"; @@ -53,21 +49,7 @@ const Section = ({ label, value, children }) => { */ export default function L10nPage() { useL10nConfigChanges(); - - const { isPending: localesPending, data: locales } = useQuery(localesQuery()); - const { isPending: timezonesPending, data: timezones } = useQuery(timezonesQuery()); - const { isPending: keymapsPending, data: keymaps } = useQuery(keymapsQuery()); - const { isPending: configPending, data: config } = useQuery(configQuery()); - - if (localesPending || timezonesPending || keymapsPending || configPending) { - return; - } - - const { locales: [localeId], keymap: keymapId, timezone: timezoneId } = config; - - const locale = locales.find((l) => l.id === localeId); - const keymap = keymaps.find((k) => k.id === keymapId); - const timezone = timezones.find((t) => t.id === timezoneId); + const { locale, timezone, keymap } = useLoaderData(); return ( <> @@ -93,7 +75,7 @@ export default function L10nPage() { label={_("Keyboard")} value={keymap ? keymap.name : _("Not selected yet")} > - + {keymap ? _("Change") : _("Select")} @@ -104,7 +86,7 @@ export default function L10nPage() { label={_("Time zone")} value={timezone ? (timezone.parts || []).join(' - ') : _("Not selected yet")} > - + {timezone ? _("Change") : _("Select")} diff --git a/web/src/components/l10n/LocaleSelection.jsx b/web/src/components/l10n/LocaleSelection.jsx index 5f4770a4cb..d256c0294a 100644 --- a/web/src/components/l10n/LocaleSelection.jsx +++ b/web/src/components/l10n/LocaleSelection.jsx @@ -19,78 +19,47 @@ * find current contact information at www.suse.com. */ -import React, { useEffect, useState } from "react"; -import { - Flex, - Form, FormGroup, - Radio, -} from "@patternfly/react-core"; -import { useNavigate } from "react-router-dom"; -import { _ } from "~/i18n"; +import React, { useState } from "react"; +import { Flex, Form, FormGroup, Radio } from "@patternfly/react-core"; +import { useLoaderData, useNavigate } from "react-router-dom"; import { ListSearch, Page } from "~/components/core"; +import { _ } from "~/i18n"; +import { useConfigMutation } from "~/queries/l10n"; import textStyles from '@patternfly/react-styles/css/utilities/Text/text'; -import { useQuery } from "@tanstack/react-query"; -import { localesQuery, configQuery, useConfigMutation } from "~/queries/l10n"; // TODO: Add documentation and typechecking // TODO: Evaluate if worth it extracting the selector export default function LocaleSelection() { - const { isPending, data: locales } = useQuery(localesQuery()); - const { data: config } = useQuery(configQuery()); - const setConfig = useConfigMutation(); - const [initial, setInitial] = useState(); - const [selected, setSelected] = useState(); - const [filteredLocales, setFilteredLocales] = useState([]); const navigate = useNavigate(); + const setConfig = useConfigMutation(); + const { locales, locale: currentLocale } = useLoaderData(); + const [selected, setSelected] = useState(currentLocale.id); + const [filteredLocales, setFilteredLocales] = useState(locales); const searchHelp = _("Filter by language, territory or locale code"); - useEffect(() => { - if (isPending) return; - - setFilteredLocales(locales); - }, [isPending, locales, setFilteredLocales]); - - useEffect(() => { - if (!config) return; - - const initialLocale = config.locales[0]; - setInitial(initialLocale); - setSelected(initialLocale); - }, [config, setInitial, setSelected]); - const onSubmit = async (e) => { e.preventDefault(); - const dataForm = new FormData(e.target); - const nextLocaleId = JSON.parse(dataForm.get("locale"))?.id; - - if (nextLocaleId !== initial) { - setConfig.mutate({ locales: [nextLocaleId] }); - } - - navigate(".."); + setConfig.mutate({ locales: selected }); + navigate(-1); }; - if (filteredLocales === undefined) { - return {_("Loading")}; - } - - let localesList = filteredLocales.map((locale) => { + let localesList = filteredLocales.map(({ id, name, territory }) => { return ( setSelected(locale.id)} + onChange={() => setSelected(id)} label={ - {locale.name} - {locale.territory} - {locale.id} + {name} + {territory} + {id} } - value={JSON.stringify(locale)} - checked={locale.id === selected} + value={id} + isChecked={id === selected} /> ); }); diff --git a/web/src/components/l10n/TimezoneSelection.jsx b/web/src/components/l10n/TimezoneSelection.jsx index cf41ea99df..4650bfa52b 100644 --- a/web/src/components/l10n/TimezoneSelection.jsx +++ b/web/src/components/l10n/TimezoneSelection.jsx @@ -19,20 +19,13 @@ * find current contact information at www.suse.com. */ -import React, { useEffect, useState } from "react"; -import { - Divider, - Flex, - Form, FormGroup, - Radio, - Text -} from "@patternfly/react-core"; +import React, { useState } from "react"; +import { Divider, Flex, Form, FormGroup, Radio, Text } from "@patternfly/react-core"; +import { useLoaderData, useNavigate } from "react-router-dom"; import { ListSearch, Page } from "~/components/core"; -import { useQuery } from "@tanstack/react-query"; -import { useNavigate } from "react-router-dom"; import { _ } from "~/i18n"; import { timezoneTime } from "~/utils"; -import { timezonesQuery, configQuery, useConfigMutation } from "~/queries/l10n"; +import { useConfigMutation } from "~/queries/l10n"; import textStyles from '@patternfly/react-styles/css/utilities/Text/text'; let date; @@ -61,77 +54,44 @@ const sortedTimezones = (timezones) => { // TODO: Refactor timezones/extendedTimezones thingy export default function TimezoneSelection() { date = new Date(); - const { data: timezones } = useQuery(timezonesQuery()); - const { data: config } = useQuery(configQuery()); - const setConfig = useConfigMutation(); - const [initial, setInitial] = useState(); - const [selected, setSelected] = useState(); - const [displayTimezones, setDisplayTimezones] = useState([]); - const [filteredTimezones, setFilteredTimezones] = useState([]); const navigate = useNavigate(); + const setConfig = useConfigMutation(); + const { timezones, timezone: currentTimezone } = useLoaderData(); + const displayTimezones = timezones.map(timezoneWithDetails); + const [selected, setSelected] = useState(currentTimezone.id); + const [filteredTimezones, setFilteredTimezones] = useState(sortedTimezones(displayTimezones)); const searchHelp = _("Filter by territory, time zone code or UTC offset"); - useEffect(() => { - if (timezones === undefined) return; - - const mapped = timezones.map(timezoneWithDetails); - setDisplayTimezones(timezones.map(timezoneWithDetails)); - }, [setDisplayTimezones, timezones]); - - useEffect(() => { - if (displayTimezones === undefined) return; - - setFilteredTimezones(sortedTimezones(displayTimezones)); - }, [setFilteredTimezones, displayTimezones]); - - useEffect(() => { - if (!config) return; - - const initialTimezone = config.timezone; - setInitial(initialTimezone); - setSelected(initialTimezone); - }, [config, setInitial, setSelected]); - const onSubmit = async (e) => { e.preventDefault(); - const dataForm = new FormData(e.target); - const nextTimezoneId = JSON.parse(dataForm.get("timezone"))?.id; - - if (nextTimezoneId !== initial) { - setConfig.mutate({ timezone: nextTimezoneId }); - } - - navigate(".."); + setConfig.mutate({ timezone: selected }); + navigate(-1); }; - if (filteredTimezones === undefined) { - return {_("Loading")}; - } - - let timezonesList = filteredTimezones.map((timezone) => { + let timezonesList = filteredTimezones.map(({ id, country, details, parts }) => { return ( setSelected(timezone)} + onChange={() => setSelected(id)} label={ <> - {timezone.parts.join('-')} - {timezone.country} + {parts.join('-')} + {country} } description={ - {timezoneTime(timezone.id, { date }) || ""} + {timezoneTime(id, { date }) || ""} -
{timezone.details}
+
{details}
 } - value={JSON.stringify(timezone)} - defaultChecked={timezone.id === selected} + value={id} + isChecked={id === selected} /> ); }); diff --git a/web/src/routes/l10n.js b/web/src/routes/l10n.js index 0df326b33a..b6d26c10ff 100644 --- a/web/src/routes/l10n.js +++ b/web/src/routes/l10n.js @@ -28,7 +28,6 @@ import { localesQuery, keymapsQuery, timezonesQuery, - useL10nConfigChanges } from "~/queries/l10n"; import { N_ } from "~/i18n"; @@ -48,13 +47,12 @@ const l10nLoader = async () => { const keymap = keymaps.find((k) => k.id === keymapId); const timezone = timezones.find((t) => t.id === timezoneId); - return { locale, keymap, timezone }; + return { locales, locale, keymaps, keymap, timezones, timezone }; }; const routes = { path: L10N_PATH, element: , - loader: l10nLoader, handle: { name: N_("Localization"), icon: "globe" @@ -62,18 +60,22 @@ const routes = { children: [ { index: true, + loader: l10nLoader, element: }, { path: LOCALE_SELECTION_PATH, + loader: l10nLoader, element: , }, { path: KEYMAP_SELECTION_PATH, + loader: l10nLoader, element: , }, { path: TIMEZONE_SELECTION_PATH, + loader: l10nLoader, element: , } ] From a8543eba036676caf6a63d9ecc2d1a1e280a4a40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 3 Jul 2024 17:16:05 +0100 Subject: [PATCH 18/38] fix(web): listen for L10n changes from App --- web/src/App.jsx | 2 ++ web/src/components/l10n/L10nPage.jsx | 4 ---- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/web/src/App.jsx b/web/src/App.jsx index edee167beb..bef32b5860 100644 --- a/web/src/App.jsx +++ b/web/src/App.jsx @@ -30,6 +30,7 @@ import { useProduct } from "./context/product"; import { CONFIG, INSTALL, STARTUP } from "~/client/phase"; import { BUSY } from "~/client/status"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { useL10nConfigChanges } from "~/queries/l10n"; const queryClient = new QueryClient(); @@ -45,6 +46,7 @@ function App() { const { connected, error, phase, status } = useInstallerClientStatus(); const { selectedProduct, products } = useProduct(); const { language } = useInstallerL10n(); + useL10nConfigChanges(); const Content = () => { if (error) return ; diff --git a/web/src/components/l10n/L10nPage.jsx b/web/src/components/l10n/L10nPage.jsx index c1a9a02db7..57a0c6b41e 100644 --- a/web/src/components/l10n/L10nPage.jsx +++ b/web/src/components/l10n/L10nPage.jsx @@ -23,9 +23,6 @@ import React from "react"; import { Gallery, GalleryItem, } from "@patternfly/react-core"; import { useLoaderData } from "react-router-dom"; import { ButtonLink, CardField, Page } from "~/components/core"; -import { - useL10nConfigChanges -} from "~/queries/l10n"; import { LOCALE_SELECTION_PATH, KEYMAP_SELECTION_PATH, TIMEZONE_SELECTION_PATH } from "~/routes/l10n"; import { _ } from "~/i18n"; @@ -48,7 +45,6 @@ const Section = ({ label, value, children }) => { * @component */ export default function L10nPage() { - useL10nConfigChanges(); const { locale, timezone, keymap } = useLoaderData(); return ( From bc6ff019c51ecb175a5fa1841a4de58237d4d61b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 3 Jul 2024 17:16:43 +0100 Subject: [PATCH 19/38] fix(web): fix locales update --- web/src/components/l10n/LocaleSelection.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/src/components/l10n/LocaleSelection.jsx b/web/src/components/l10n/LocaleSelection.jsx index d256c0294a..9f143b76dd 100644 --- a/web/src/components/l10n/LocaleSelection.jsx +++ b/web/src/components/l10n/LocaleSelection.jsx @@ -40,7 +40,7 @@ export default function LocaleSelection() { const onSubmit = async (e) => { e.preventDefault(); - setConfig.mutate({ locales: selected }); + setConfig.mutate({ locales: [selected] }); navigate(-1); }; From ccaa0f07ce54f8f63f80c3577cb66f12395705fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Thu, 4 Jul 2024 10:44:49 +0100 Subject: [PATCH 20/38] fix(web): revalidate loader on config change By making use of https://reactrouter.com/en/main/hooks/use-revalidator See https://github.com/remix-run/react-router/discussions/10381#discussioncomment-5713606 too --- web/src/queries/l10n.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/web/src/queries/l10n.js b/web/src/queries/l10n.js index 1c288b3c67..bd7b9f9db0 100644 --- a/web/src/queries/l10n.js +++ b/web/src/queries/l10n.js @@ -20,6 +20,7 @@ */ import React from "react"; +import { useRevalidator } from "react-router-dom"; import { useQueryClient, useMutation } from "@tanstack/react-query"; import { useInstallerClient } from "~/context/installer"; import { timezoneUTCOffset } from "~/utils"; @@ -91,6 +92,7 @@ const useConfigMutation = () => { const useL10nConfigChanges = () => { const queryClient = useQueryClient(); const client = useInstallerClient(); + const revalidator = useRevalidator(); React.useEffect(() => { if (!client) return; @@ -98,9 +100,10 @@ const useL10nConfigChanges = () => { return client.ws().onEvent(event => { if (event.type === "L10nConfigChanged") { queryClient.invalidateQueries({ queryKey: ["l10n", "config"] }); + revalidator.revalidate(); } }); - }, [queryClient, client]); + }, [queryClient, client, revalidator]); }; export { From c0367bea31b6dcc7903a2ce69a3f9ee4696d309e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Thu, 4 Jul 2024 13:08:28 +0100 Subject: [PATCH 21/38] fix(web): add a role to CardField * The label is not properly set yet. --- web/src/components/core/CardField.jsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/web/src/components/core/CardField.jsx b/web/src/components/core/CardField.jsx index 5a2ca1b7c0..e38356a010 100644 --- a/web/src/components/core/CardField.jsx +++ b/web/src/components/core/CardField.jsx @@ -48,8 +48,9 @@ const CardField = ({ cardDescriptionProps = {} }) => { + // TODO: replace aria-label with the proper aria-labelledby return ( - + From 122bf2029d80ed410c93d3d520afd64c7b702b2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Thu, 4 Jul 2024 13:14:42 +0100 Subject: [PATCH 22/38] test(web): write a new test for L10nPage --- web/src/components/l10n/L10nPage.test.jsx | 385 +++------------------- 1 file changed, 45 insertions(+), 340 deletions(-) diff --git a/web/src/components/l10n/L10nPage.test.jsx b/web/src/components/l10n/L10nPage.test.jsx index 72613de5d5..8c9268da25 100644 --- a/web/src/components/l10n/L10nPage.test.jsx +++ b/web/src/components/l10n/L10nPage.test.jsx @@ -20,377 +20,82 @@ */ import React from "react"; -import { render, screen, waitFor, within } from "@testing-library/react"; - -import { installerRender, plainRender, queryRender } from "~/test-utils"; +import { render, screen, within } from "@testing-library/react"; import L10nPage from "~/components/l10n/L10nPage"; -import { QueryClient } from "@tanstack/query-core"; -import { QueryClientProvider } from "@tanstack/react-query"; -import { MemoryRouter } from "react-router"; - -const locales = [ - { id: "de_DE.UTF8", name: "German", territory: "Germany" }, - { id: "en_US.UTF8", name: "English", territory: "United States" }, - { id: "es_ES.UTF8", name: "Spanish", territory: "Spain" } -]; -const keymaps = [ - { id: "de", name: "German" }, - { id: "us", name: "English" }, - { id: "es", name: "Spanish" } -]; +let mockLoadedData; -const timezones = [ - { id: "asia/bangkok", parts: ["Asia", "Bangkok"] }, - { id: "atlantic/canary", parts: ["Atlantic", "Canary"] }, - { id: "america/new_york", parts: ["Americas", "New York"] } -]; - -jest.mock("~/queries/l10n", () => ({ - localesQuery: () => ({ - queryKey: ["l10n", "locales"], - queryFn: jest.fn().mockResolvedValue(locales) - }), - timezonesQuery: () => ({ - queryKey: ["l10n", "timezones"], - queryFn: jest.fn().mockResolvedValue(timezones) - }), - keymapsQuery: () => ({ - queryKey: ["l10n", "keymaps"], - queryFn: jest.fn().mockResolvedValue(keymaps) - }), - configQuery: () => ({ - queryKey: ["l10n", "config"], - queryFn: jest.fn().mockResolvedValue({ - locales: mockSelectedLocales, - timezone: mockSelectedTimezone, - keymap: mockSelectedKeymap - }) - }), - useL10nConfigChanges: jest.fn() +jest.mock('react-router-dom', () => ({ + ...jest.requireActual("react-router-dom"), + useLoaderData: () => mockLoadedData, + // TODO: mock the link because it needs a working router. + Link: ({ children }) => })); -let mockL10nClient; -let mockSelectedLocales; -let mockSelectedKeymap; -let mockSelectedTimezone; - beforeEach(() => { - mockSelectedLocales = []; - mockSelectedKeymap = undefined; - mockSelectedTimezone = undefined; + mockLoadedData = { + locale: { id: "en_US.UTF-8", name: "English", territory: "United States" }, + keymap: { id: "us", name: "English" }, + timezone: { id: "Europe/Berlin", parts: ["Europe", "Berlin"]} + }; }); -it.only("renders a section for configuring the language", async () => { - queryRender(); - await screen.findByText("Language"); +it("renders a section for configuring the language", () => { + render(); + const region = screen.getByRole("region", { name: "Language" }) + within(region).getByText("English - United States"), + within(region).getByText("Change"); }); -describe.skip("if there is no selected language", () => { +describe("if there is no selected language", () => { beforeEach(() => { - mockSelectedLocales = []; + mockLoadedData.locale = undefined; }); it("renders a button for selecting a language", () => { - plainRender(); - screen.getByText("Language not selected yet"); - screen.getByRole("button", { name: "Select language" }); - }); -}); - -describe.skip("if there is a selected language", () => { - beforeEach(() => { - mockSelectedLocales = [{ id: "es_ES.UTF8", name: "Spanish", territory: "Spain" }]; - }); - - it("renders a button for changing the language", () => { - plainRender(); - screen.getByText("Spanish - Spain"); - screen.getByRole("button", { name: "Change language" }); - }); -}); - -describe.skip("when the button for changing the language is clicked", () => { - beforeEach(() => { - mockSelectedLocales = [{ id: "es_ES.UTF8", name: "Spanish", territory: "Spain" }]; - }); - - it("opens a popup for selecting the language", async () => { - const { user } = installerRender(); - - const button = screen.getByRole("button", { name: "Change language" }); - await user.click(button); - - const popup = await screen.findByRole("dialog"); - within(popup).getByText("Select language"); - within(popup).getByRole("row", { name: /German/ }); - within(popup).getByRole("row", { name: /English/ }); - within(popup).getByRole("row", { name: /Spanish/, selected: true }); - }); - - it("allows filtering languages", async () => { - const { user } = installerRender(); - - const button = screen.getByRole("button", { name: "Change language" }); - await user.click(button); - - const popup = await screen.findByRole("dialog"); - const searchInput = within(popup).getByRole("search"); - - await user.type(searchInput, "ish"); - - await waitFor(() => ( - expect(within(popup).queryByRole("row", { name: /German/ })).not.toBeInTheDocument()) - ); - within(popup).getByRole("row", { name: /English/ }); - within(popup).getByRole("row", { name: /Spanish/ }); - }); - - describe("if the popup is canceled", () => { - it("closes the popup without selecting a new language", async () => { - const { user } = installerRender(); - - const button = screen.getByRole("button", { name: "Change language" }); - await user.click(button); - - const popup = await screen.findByRole("dialog"); - const option = within(popup).getByRole("row", { name: /English/ }); - - await user.click(option); - const cancel = within(popup).getByRole("button", { name: "Cancel" }); - await user.click(cancel); - - expect(mockL10nClient.setLocales).not.toHaveBeenCalled(); - expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); - }); - }); - - describe("if the popup is accepted", () => { - it("closes the popup selecting the new language", async () => { - const { user } = installerRender(); - - const button = screen.getByRole("button", { name: "Change language" }); - await user.click(button); - - const popup = await screen.findByRole("dialog"); - const option = within(popup).getByRole("row", { name: /English/ }); - - await user.click(option); - const accept = within(popup).getByRole("button", { name: "Accept" }); - await user.click(accept); - - expect(mockL10nClient.setLocales).toHaveBeenCalledWith(["en_US.UTF8"]); - expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); - }); + render(); + const region = screen.getByRole("region", { name: "Language" }) + within(region).getByText("Not selected yet"); + within(region).getByText("Select"); }); }); -it.skip("renders a section for configuring the keyboard", () => { - plainRender(); - screen.getByText("Keyboard"); +it("renders a section for configuring the keyboard", () => { + render(); + const region = screen.getByRole("region", { name: "Keyboard" }) + within(region).getByText("English"), + within(region).getByText("Change"); }); -describe.skip("if there is no selected keyboard", () => { +describe("if there is no selected keyboard", () => { beforeEach(() => { - mockSelectedKeymap = undefined; + mockLoadedData.keymap = undefined; }); it("renders a button for selecting a keyboard", () => { - plainRender(); - screen.getByText("Keyboard not selected yet"); - screen.getByRole("button", { name: "Select keyboard" }); - }); -}); - -describe.skip("if there is a selected keyboard", () => { - beforeEach(() => { - mockSelectedKeymap = { id: "es", name: "Spanish" }; - }); - - it("renders a button for changing the keyboard", () => { - plainRender(); - screen.getByText("Spanish"); - screen.getByRole("button", { name: "Change keyboard" }); - }); -}); - -describe.skip("when the button for changing the keyboard is clicked", () => { - beforeEach(() => { - mockSelectedKeymap = { id: "es", name: "Spanish" }; - }); - - it("opens a popup for selecting the keyboard", async () => { - const { user } = installerRender(); - - const button = screen.getByRole("button", { name: "Change keyboard" }); - await user.click(button); - - const popup = await screen.findByRole("dialog"); - within(popup).getByText("Select keyboard"); - within(popup).getByRole("row", { name: /German/ }); - within(popup).getByRole("row", { name: /English/ }); - within(popup).getByRole("row", { name: /Spanish/, selected: true }); - }); - - it("allows filtering keyboards", async () => { - const { user } = installerRender(); - - const button = screen.getByRole("button", { name: "Change keyboard" }); - await user.click(button); - - const popup = await screen.findByRole("dialog"); - const searchInput = within(popup).getByRole("search"); - - await user.type(searchInput, "ish"); - - await waitFor(() => ( - expect(within(popup).queryByRole("row", { name: /German/ })).not.toBeInTheDocument()) - ); - within(popup).getByRole("row", { name: /English/ }); - within(popup).getByRole("row", { name: /Spanish/ }); - }); - - describe("if the popup is canceled", () => { - it("closes the popup without selecting a new keyboard", async () => { - const { user } = installerRender(); - - const button = screen.getByRole("button", { name: "Change keyboard" }); - await user.click(button); - - const popup = await screen.findByRole("dialog"); - const option = within(popup).getByRole("row", { name: /English/ }); - - await user.click(option); - const cancel = within(popup).getByRole("button", { name: "Cancel" }); - await user.click(cancel); - - expect(mockL10nClient.setKeymap).not.toHaveBeenCalled(); - expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); - }); - }); - - describe("if the popup is accepted", () => { - it("closes the popup selecting the new keyboard", async () => { - const { user } = installerRender(); - - const button = screen.getByRole("button", { name: "Change keyboard" }); - await user.click(button); - - const popup = await screen.findByRole("dialog"); - const option = within(popup).getByRole("row", { name: /English/ }); - - await user.click(option); - const accept = within(popup).getByRole("button", { name: "Accept" }); - await user.click(accept); - - expect(mockL10nClient.setKeymap).toHaveBeenCalledWith("us"); - expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); - }); + render(); + const region = screen.getByRole("region", { name: "Keyboard" }) + within(region).getByText("Not selected yet"); + within(region).getByText("Select"); }); }); -it.skip("renders a section for configuring the time zone", () => { - plainRender(); - screen.getByText("Time zone"); +it("renders a section for configuring the time zone", () => { + render(); + const region = screen.getByRole("region", { name: "Time zone" }) + within(region).getByText("Europe - Berlin"), + within(region).getByText("Change"); }); -describe.skip("if there is no selected time zone", () => { +describe("if there is no selected time zone", () => { beforeEach(() => { - mockSelectedTimezone = undefined; + mockLoadedData.timezone = undefined; }); it("renders a button for selecting a time zone", () => { - plainRender(); - screen.getByText("Time zone not selected yet"); - screen.getByRole("button", { name: "Select time zone" }); - }); -}); - -describe.skip("if there is a selected time zone", () => { - beforeEach(() => { - mockSelectedTimezone = { id: "atlantic/canary", parts: ["Atlantic", "Canary"] }; - }); - - it("renders a button for changing the time zone", () => { - plainRender(); - screen.getByText("Atlantic - Canary"); - screen.getByRole("button", { name: "Change time zone" }); - }); -}); - -describe.skip("when the button for changing the time zone is clicked", () => { - beforeEach(() => { - mockSelectedTimezone = { id: "atlantic/canary", parts: ["Atlantic", "Canary"] }; - }); - - it("opens a popup for selecting the time zone", async () => { - const { user } = installerRender(); - - const button = screen.getByRole("button", { name: "Change time zone" }); - await user.click(button); - - const popup = await screen.findByRole("dialog"); - within(popup).getByText("Select time zone"); - within(popup).getByRole("row", { name: /Bangkok/ }); - within(popup).getByRole("row", { name: /Canary/, selected: true }); - within(popup).getByRole("row", { name: /New York/ }); - }); - - it("allows filtering time zones", async () => { - const { user } = installerRender(); - - const button = screen.getByRole("button", { name: "Change time zone" }); - await user.click(button); - - const popup = await screen.findByRole("dialog"); - const searchInput = within(popup).getByRole("search"); - - await user.type(searchInput, "new"); - - await waitFor(() => ( - expect(within(popup).queryByRole("row", { name: /Bangkok/ })).not.toBeInTheDocument()) - ); - await waitFor(() => ( - expect(within(popup).queryByRole("row", { name: /Canary/ })).not.toBeInTheDocument()) - ); - within(popup).getByRole("row", { name: /New York/ }); - }); - - describe("if the popup is canceled", () => { - it("closes the popup without selecting a new time zone", async () => { - const { user } = installerRender(); - - const button = screen.getByRole("button", { name: "Change time zone" }); - await user.click(button); - - const popup = await screen.findByRole("dialog"); - const option = within(popup).getByRole("row", { name: /New York/ }); - - await user.click(option); - const cancel = within(popup).getByRole("button", { name: "Cancel" }); - await user.click(cancel); - - expect(mockL10nClient.setTimezone).not.toHaveBeenCalled(); - expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); - }); - }); - - describe("if the popup is accepted", () => { - it("closes the popup selecting the new time zone", async () => { - const { user } = installerRender(); - - const button = screen.getByRole("button", { name: "Change time zone" }); - await user.click(button); - - const popup = await screen.findByRole("dialog"); - const option = within(popup).getByRole("row", { name: /Bangkok/ }); - - await user.click(option); - const accept = within(popup).getByRole("button", { name: "Accept" }); - await user.click(accept); - - expect(mockL10nClient.setTimezone).toHaveBeenCalledWith("asia/bangkok"); - expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); - }); + render(); + const region = screen.getByRole("region", { name: "Time zone" }) + within(region).getByText("Not selected yet"); + within(region).getByText("Select"); }); }); From 0fd96ffec28d3103f045d21fdae845c55a5ce572 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Thu, 4 Jul 2024 13:17:32 +0100 Subject: [PATCH 23/38] test: drop the queryRender helper --- web/src/test-utils.js | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/web/src/test-utils.js b/web/src/test-utils.js index cc9c3fd988..22e46e64aa 100644 --- a/web/src/test-utils.js +++ b/web/src/test-utils.js @@ -161,25 +161,6 @@ const plainRender = (ui, options = {}) => { ); }; -/** - * Wrapper around react-testing-library#render for rendering components with the - * QueryClientProvider from TanStack Query. - * - * @todo Unify the render functions once the HTTP client has been replaced with - * TanStack Query. - */ -const queryRender = (ui, options = {}) => { - const queryClient = new QueryClient({}); - - const wrapper = ({ children }) => ( - - {children} - - ); - - return plainRender(ui, {...options, wrapper}); -} - /** * Creates a function to register callbacks * @@ -232,7 +213,6 @@ const resetLocalStorage = (initialState) => { export { plainRender, installerRender, - queryRender, createCallbackMock, mockGettext, mockNavigateFn, From e018d5f6ad3fa7708b14614a781961abddc3c947 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Thu, 4 Jul 2024 14:15:06 +0100 Subject: [PATCH 24/38] doc(web): document l10n queries --- web/src/queries/l10n.js | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/web/src/queries/l10n.js b/web/src/queries/l10n.js index bd7b9f9db0..d644a5c2d8 100644 --- a/web/src/queries/l10n.js +++ b/web/src/queries/l10n.js @@ -25,6 +25,9 @@ import { useQueryClient, useMutation } from "@tanstack/react-query"; import { useInstallerClient } from "~/context/installer"; import { timezoneUTCOffset } from "~/utils"; +/** + * Returns a query for retrieving the localization configuration + */ const configQuery = () => { return { queryKey: ["l10n", "config"], @@ -32,6 +35,9 @@ const configQuery = () => { }; }; +/** + * Returns a query for retrieving the list of known locales + */ const localesQuery = () => ({ queryKey: ["l10n", "locales"], queryFn: async () => { @@ -44,6 +50,9 @@ const localesQuery = () => ({ staleTime: Infinity }); +/** + * Returns a query for retrieving the list of known timezones + */ const timezonesQuery = () => ({ queryKey: ["l10n", "timezones"], queryFn: async () => { @@ -57,6 +66,9 @@ const timezonesQuery = () => ({ staleTime: Infinity }); +/** + * Returns a query for retrieving the list of known keymaps + */ const keymapsQuery = () => ({ queryKey: ["l10n", "keymaps"], queryFn: async () => { @@ -70,6 +82,11 @@ const keymapsQuery = () => ({ staleTime: Infinity }); +/** + * Hook that builds a mutation to update the l10n configuration + * + * It does not require to call `useMutation`. + */ const useConfigMutation = () => { const queryClient = useQueryClient(); @@ -89,6 +106,13 @@ const useConfigMutation = () => { return useMutation(query); }; + +/** + * Hook that returns a useEffect to listen for L10nConfigChanged events + * + * When the configuration changes, it invalidates the config query and forces the router to + * revalidate its data (executing the loaders again). + */ const useL10nConfigChanges = () => { const queryClient = useQueryClient(); const client = useInstallerClient(); From 3cdcaf68886edbef26857444546b07674e2cb4ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Thu, 4 Jul 2024 14:17:52 +0100 Subject: [PATCH 25/38] fix(web): make ESLint happy --- web/src/components/l10n/L10nPage.test.jsx | 20 ++++++++++---------- web/src/queries/l10n.js | 1 - 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/web/src/components/l10n/L10nPage.test.jsx b/web/src/components/l10n/L10nPage.test.jsx index 8c9268da25..1406d0a99c 100644 --- a/web/src/components/l10n/L10nPage.test.jsx +++ b/web/src/components/l10n/L10nPage.test.jsx @@ -36,14 +36,14 @@ beforeEach(() => { mockLoadedData = { locale: { id: "en_US.UTF-8", name: "English", territory: "United States" }, keymap: { id: "us", name: "English" }, - timezone: { id: "Europe/Berlin", parts: ["Europe", "Berlin"]} + timezone: { id: "Europe/Berlin", parts: ["Europe", "Berlin"] } }; }); it("renders a section for configuring the language", () => { render(); - const region = screen.getByRole("region", { name: "Language" }) - within(region).getByText("English - United States"), + const region = screen.getByRole("region", { name: "Language" }); + within(region).getByText("English - United States"); within(region).getByText("Change"); }); @@ -54,7 +54,7 @@ describe("if there is no selected language", () => { it("renders a button for selecting a language", () => { render(); - const region = screen.getByRole("region", { name: "Language" }) + const region = screen.getByRole("region", { name: "Language" }); within(region).getByText("Not selected yet"); within(region).getByText("Select"); }); @@ -62,8 +62,8 @@ describe("if there is no selected language", () => { it("renders a section for configuring the keyboard", () => { render(); - const region = screen.getByRole("region", { name: "Keyboard" }) - within(region).getByText("English"), + const region = screen.getByRole("region", { name: "Keyboard" }); + within(region).getByText("English"); within(region).getByText("Change"); }); @@ -74,7 +74,7 @@ describe("if there is no selected keyboard", () => { it("renders a button for selecting a keyboard", () => { render(); - const region = screen.getByRole("region", { name: "Keyboard" }) + const region = screen.getByRole("region", { name: "Keyboard" }); within(region).getByText("Not selected yet"); within(region).getByText("Select"); }); @@ -82,8 +82,8 @@ describe("if there is no selected keyboard", () => { it("renders a section for configuring the time zone", () => { render(); - const region = screen.getByRole("region", { name: "Time zone" }) - within(region).getByText("Europe - Berlin"), + const region = screen.getByRole("region", { name: "Time zone" }); + within(region).getByText("Europe - Berlin"); within(region).getByText("Change"); }); @@ -94,7 +94,7 @@ describe("if there is no selected time zone", () => { it("renders a button for selecting a time zone", () => { render(); - const region = screen.getByRole("region", { name: "Time zone" }) + const region = screen.getByRole("region", { name: "Time zone" }); within(region).getByText("Not selected yet"); within(region).getByText("Select"); }); diff --git a/web/src/queries/l10n.js b/web/src/queries/l10n.js index d644a5c2d8..d75d25f37a 100644 --- a/web/src/queries/l10n.js +++ b/web/src/queries/l10n.js @@ -106,7 +106,6 @@ const useConfigMutation = () => { return useMutation(query); }; - /** * Hook that returns a useEffect to listen for L10nConfigChanged events * From 58a27ff5eefea1a3290405cf3d473b972026c883 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Thu, 4 Jul 2024 14:46:46 +0100 Subject: [PATCH 26/38] refactor(web): remove unused methods from L10nClient --- web/src/client/l10n.js | 241 ----------------------------------------- 1 file changed, 241 deletions(-) diff --git a/web/src/client/l10n.js b/web/src/client/l10n.js index 2cdb210969..29e5344f80 100644 --- a/web/src/client/l10n.js +++ b/web/src/client/l10n.js @@ -20,28 +20,6 @@ */ // @ts-check -import { timezoneUTCOffset } from "~/utils"; - -/** - * @typedef {object} Timezone - * @property {string} id - Timezone id (e.g., "Atlantic/Canary"). - * @property {Array} parts - Name of the timezone parts (e.g., ["Atlantic", "Canary"]). - * @property {string} country - Name of the country associated to the zone or empty string (e.g., "Spain"). - * @property {number} utcOffset - UTC offset. - */ - -/** - * @typedef {object} Locale - * @property {string} id - Language id (e.g., "en_US.UTF-8"). - * @property {string} name - Language name (e.g., "English"). - * @property {string} territory - Territory name (e.g., "United States"). - */ - -/** - * @typedef {object} Keymap - * @property {string} id - Keyboard id (e.g., "us"). - * @property {string} name - Keyboard name (e.g., "English (US)"). - */ /** * Manages localization. @@ -107,225 +85,6 @@ class L10nClient { async setUIKeymap(id) { return this.client.patch("/l10n/config", { uiKeymap: id }); } - - /** - * All possible timezones for the target system. - * - * @return {Promise>} - */ - async timezones() { - const response = await this.client.get("/l10n/timezones"); - if (!response.ok) { - console.error("Failed to get localization config: ", response); - return []; - } - const timezones = await response.json(); - return timezones.map(this.buildTimezone); - } - - /** - * Timezone selected for the target system. - * - * @return {Promise} Id of the timezone. - */ - async getTimezone() { - const config = await this.getConfig(); - return config.timezone; - } - - /** - * Sets the timezone for the target system. - * - * @param {string} id - Id of the timezone. - * @return {Promise} - */ - async setTimezone(id) { - this.setConfig({ timezone: id }); - } - - /** - * Available locales to install in the target system. - * - * TODO: find a better name because it is rather confusing (e.g., 'locales' and 'getLocales'). - * - * @return {Promise>} - */ - async locales() { - const response = await this.client.get("/l10n/locales"); - if (!response.ok) { - console.error("Failed to get localization config: ", response); - return []; - } - const locales = await response.json(); - return locales.map(this.buildLocale); - } - - /** - * Locales selected to install in the target system. - * - * @return {Promise>} Ids of the locales. - */ - async getLocales() { - const config = await this.getConfig(); - return config.locales; - } - - /** - * Sets the locales to install in the target system. - * - * @param {Array} ids - Ids of the locales. - * @return {Promise} - */ - async setLocales(ids) { - this.setConfig({ locales: ids }); - } - - /** - * Available keymaps to install in the target system. - * - * Note that name is localized to the current selected UI language: - * { id: "es", name: "Spanish (ES)" } - * - * @return {Promise>} - */ - async keymaps() { - const response = await this.client.get("/l10n/keymaps"); - if (!response.ok) { - console.error("Failed to get localization config: ", response); - return []; - } - const keymaps = await response.json(); - return keymaps.map(this.buildKeymap); - } - - /** - * Keymap selected to install in the target system. - * - * @return {Promise} Id of the keymap. - */ - async getKeymap() { - const config = await this.getConfig(); - return config.keymap; - } - - /** - * Sets the keymap to install in the target system. - * - * @param {string} id - Id of the keymap. - * @return {Promise} - */ - async setKeymap(id) { - this.setConfig({ keymap: id }); - } - - /** - * Register a callback to run when the timezone configuration changes. - * - * @param {(timezone: string) => void} handler - Function to call when Timezone changes. - * @return {import ("./http").RemoveFn} Function to disable the callback. - */ - onTimezoneChange(handler) { - return this.client.onEvent("L10nConfigChanged", ({ timezone }) => { - if (timezone) { - handler(timezone); - } - }); - } - - /** - * Register a callback to run when the locales configuration changes. - * - * @param {(locales: string[]) => void} handler - Function to call when Locales changes. - * @return {import ("./http").RemoveFn} Function to disable the callback. - */ - onLocalesChange(handler) { - return this.client.onEvent("L10nConfigChanged", ({ locales }) => { - if (locales) { - handler(locales); - } - }); - } - - /** - * Register a callback to run when the keymap configuration changes. - * - * @param {(keymap: string) => void} handler - Function to call when Keymap changes. - * @return {import ("./http").RemoveFn} Function to disable the callback. - */ - onKeymapChange(handler) { - return this.client.onEvent("L10nConfigChanged", ({ keymap }) => { - if (keymap) { - handler(keymap); - } - }); - } - - /** - * @private - * Convenience method to get l10n the configuration. - * - * @return {Promise} Localization configuration. - */ - async getConfig() { - const response = await this.client.get("/l10n/config"); - if (!response.ok) { - console.error("Failed to get localization config: ", response); - return {}; - } - return await response.json(); - } - - /** - * @private - * - * Convenience method to set l10n the configuration. - * - * @param {object} data - Configuration to update. It can just part of the configuration. - * @return {Promise} - */ - async setConfig(data) { - return this.client.patch("/l10n/config", data); - } - - /** - * @private - * - * @param {object} timezone - Timezone data. - * @param {string} timezone.code - Timezone identifier. - * @param {Array} timezone.parts - Localized parts of the timezone identifier. - * @param {string} timezone.country - Timezone country. - * @return {Timezone} - */ - buildTimezone({ code, parts, country }) { - const utcOffset = timezoneUTCOffset(code); - - return ({ id: code, parts, country, utcOffset }); - } - - /** - * @private - * - * @param {object} locale - Locale data. - * @param {string} locale.id - Identifier. - * @param {string} locale.language - Name. - * @param {string} locale.territory - Territory. - * @return {Locale} - */ - buildLocale({ id, language, territory }) { - return ({ id, name: language, territory }); - } - - /** - * @private - * - * @param {object} keymap - Keymap data - * @param {string} keymap.id - Id (e.g., "us"). - * @param {string} keymap.description - Keymap description (e.g., "English (US)"). - * @return {Keymap} - */ - buildKeymap({ id, description }) { - return ({ id, name: description }); - } } export { L10nClient }; From f633c9517857995d4c73af25fc992339bf530f45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Thu, 4 Jul 2024 14:54:34 +0100 Subject: [PATCH 27/38] test(web): fix App tests --- web/src/App.test.jsx | 17 +++++++---------- web/src/test-utils.js | 12 +++++++++++- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/web/src/App.test.jsx b/web/src/App.test.jsx index 303891708f..d3fe46be4b 100644 --- a/web/src/App.test.jsx +++ b/web/src/App.test.jsx @@ -20,13 +20,14 @@ */ import React from "react"; -import { act, screen } from "@testing-library/react"; +import { screen } from "@testing-library/react"; import { installerRender } from "~/test-utils"; import App from "./App"; import { createClient } from "~/client"; import { STARTUP, CONFIG, INSTALL } from "~/client/phase"; import { IDLE, BUSY } from "~/client/status"; +import { useL10nConfigChanges } from "./queries/l10n"; jest.mock("~/client"); @@ -44,6 +45,11 @@ jest.mock("~/context/product", () => ({ } })); +jest.mock("~/queries/l10n", () => ({ + ...jest.requireActual("~/queries/l10n"), + useL10nConfigChanges: () => jest.fn() +})); + const mockClientStatus = { connected: true, error: false, @@ -70,18 +76,9 @@ describe("App", () => { createClient.mockImplementation(() => { return { l10n: { - locales: jest.fn().mockResolvedValue([["en_us", "English", "United States"]]), - getLocales: jest.fn().mockResolvedValue(["en_us"]), - timezones: jest.fn().mockResolvedValue([]), - getTimezone: jest.fn().mockResolvedValue("Europe/Berlin"), - keymaps: jest.fn().mockResolvedValue([]), - getKeymap: jest.fn().mockResolvedValue(undefined), getUIKeymap: jest.fn().mockResolvedValue("en"), getUILocale: jest.fn().mockResolvedValue("en_us"), setUILocale: jest.fn().mockResolvedValue("en_us"), - onTimezoneChange: jest.fn(), - onLocalesChange: jest.fn(), - onKeymapChange: jest.fn() } }; }); diff --git a/web/src/test-utils.js b/web/src/test-utils.js index 22e46e64aa..162418cd52 100644 --- a/web/src/test-utils.js +++ b/web/src/test-utils.js @@ -51,6 +51,14 @@ const initialRoutes = jest.fn().mockReturnValue(["/"]); */ const mockNavigateFn = jest.fn(); +/** + * Allows checking when the useRevalidator function has been called + * + * @example + * expect(mockUseRevalidator).toHaveBeenCalled() + */ +const mockUseRevalidator = jest.fn(); + /** * Allows manipulating MemoryRouter routes for testing purpose * @@ -68,7 +76,8 @@ jest.mock('react-router-dom', () => ({ ...jest.requireActual("react-router-dom"), useNavigate: () => mockNavigateFn, Navigate: ({ to: route }) => <>Navigating to {route}, - Outlet: () => <>Outlet Content + Outlet: () => <>Outlet Content, + useRevalidator: () => mockUseRevalidator })); const Providers = ({ children, withL10n }) => { @@ -217,5 +226,6 @@ export { mockGettext, mockNavigateFn, mockRoutes, + mockUseRevalidator, resetLocalStorage }; From 6f16921808672c8787293fbe905aa32e29a2e6d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Thu, 4 Jul 2024 14:59:07 +0100 Subject: [PATCH 28/38] refactor(web): drop unused L10nClient tests --- web/src/client/l10n.test.js | 129 ------------------------------------ 1 file changed, 129 deletions(-) delete mode 100644 web/src/client/l10n.test.js diff --git a/web/src/client/l10n.test.js b/web/src/client/l10n.test.js deleted file mode 100644 index e24dbb7192..0000000000 --- a/web/src/client/l10n.test.js +++ /dev/null @@ -1,129 +0,0 @@ -/* - * Copyright (c) [2022-2023] SUSE LLC - * - * All Rights Reserved. - * - * This program is free software; you can redistribute it and/or modify it - * under the terms of version 2 of the GNU General Public License as published - * by the Free Software Foundation. - * - * This program is distributed in the hope that it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for - * more details. - * - * You should have received a copy of the GNU General Public License along - * with this program; if not, contact SUSE LLC. - * - * To contact SUSE LLC about this file by physical or electronic mail, you may - * find current contact information at www.suse.com. - */ - -// @ts-check - -import { HTTPClient } from "./http"; -import { L10nClient } from "./l10n"; - -jest.mock("./dbus"); - -const mockJsonFn = jest.fn(); -const mockGetFn = jest.fn().mockImplementation(() => { - return { ok: true, json: mockJsonFn }; -}); -const mockPatchFn = jest.fn().mockImplementation(() => { - return { ok: true }; -}); - -jest.mock("./http", () => { - return { - HTTPClient: jest.fn().mockImplementation(() => { - return { - get: mockGetFn, - patch: mockPatchFn, - }; - }), - }; -}); - -let client; - -const locales = [ - { - id: "en_US.UTF-8", - language: "English", - territory: "United States", - }, - { - id: "es_ES.UTF-8", - language: "Spanish", - territory: "Spain", - }, -]; - -const config = { - locales: [ - "en_US.UTF-8", - ], - keymap: "us", - timezone: "Europe/Berlin", - uiLocale: "en_US.UTF-8", - uiKeymap: "us", -}; - -beforeEach(() => { - client = new L10nClient(new HTTPClient(new URL("http://localhost"))); -}); - -describe("#locales", () => { - beforeEach(() => { - mockJsonFn.mockResolvedValue(locales); - }); - - it("returns the list of available locales", async () => { - const locales = await client.locales(); - - expect(locales).toEqual([ - { id: "en_US.UTF-8", name: "English", territory: "United States" }, - { id: "es_ES.UTF-8", name: "Spanish", territory: "Spain" }, - ]); - expect(mockGetFn).toHaveBeenCalledWith("/l10n/locales"); - }); -}); - -describe("#getConfig", () => { - beforeEach(() => { - mockJsonFn.mockResolvedValue(config); - }); - - it("returns the list of selected locales", async () => { - const l10nConfig = await client.getConfig(); - - expect(l10nConfig).toEqual(config); - expect(mockGetFn).toHaveBeenCalledWith("/l10n/config"); - }); -}); - -describe("#setConfig", () => { - beforeEach(() => { - mockJsonFn.mockResolvedValue(config); - }); - - it("updates the l10n configuration", async () => { - await client.setConfig(config); - client.setConfig(config); - expect(mockPatchFn).toHaveBeenCalledWith("/l10n/config", config); - }); -}); - -describe("#getLocales", () => { - beforeEach(() => { - mockJsonFn.mockResolvedValue(config); - }); - - it("returns the list of selected locales", async () => { - const locales = await client.getLocales(); - - expect(locales).toEqual(["en_US.UTF-8"]); - expect(mockGetFn).toHaveBeenCalledWith("/l10n/config"); - }); -}); From 4766a2d95d103ec33a04f1c752b1b9e5463d9e3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Thu, 4 Jul 2024 15:15:53 +0100 Subject: [PATCH 29/38] feat(web): wrap plainRender on a QueryClientProvider * In case you need something really minimal, use the "render" function from React Testing Library. --- web/src/test-utils.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/web/src/test-utils.js b/web/src/test-utils.js index 162418cd52..7aaa06f560 100644 --- a/web/src/test-utils.js +++ b/web/src/test-utils.js @@ -162,10 +162,17 @@ const installerRender = (ui, options = {}) => { * core/Sidebar as part of the layout. */ const plainRender = (ui, options = {}) => { + const queryClient = new QueryClient({}); + + const Wrapper = ({ children }) => ( + + {children} + + ); return ( { user: userEvent.setup(), - ...render(ui, options) + ...render(ui, { wrapper: Wrapper, ...options }) } ); }; From b893146dbaf3c571d2a134784bb75d9a739dec9f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Thu, 4 Jul 2024 15:17:00 +0100 Subject: [PATCH 30/38] test(web): fix L10nSectino tests --- .../components/overview/L10nSection.test.jsx | 60 +++++-------------- 1 file changed, 16 insertions(+), 44 deletions(-) diff --git a/web/src/components/overview/L10nSection.test.jsx b/web/src/components/overview/L10nSection.test.jsx index 4a813225bb..7ddf3347f2 100644 --- a/web/src/components/overview/L10nSection.test.jsx +++ b/web/src/components/overview/L10nSection.test.jsx @@ -20,58 +20,30 @@ */ import React from "react"; -import { act, screen } from "@testing-library/react"; -import { createCallbackMock, installerRender } from "~/test-utils"; +import { screen } from "@testing-library/react"; +import { plainRender } from "~/test-utils"; import { L10nSection } from "~/components/overview"; -import { createClient } from "~/client"; jest.mock("~/client"); +jest.mock("~/queries/l10n", () => ({ + localesQuery: () => ({ + queryKey: ["l10n", "locales"], + queryFn: jest.fn().mockResolvedValue(locales) + }), + configQuery: () => ({ + queryKey: ["l10n", "config"], + queryFn: jest.fn().mockResolvedValue({ locales: ["en_US.UTF-8"]}) + }) +})); + const locales = [ - { id: "en_US", name: "English", territory: "United States" }, - { id: "de_DE", name: "German", territory: "Germany" } + { id: "en_US.UTF-8", name: "English", territory: "United States" }, + { id: "de_DE.UTF-8", name: "German", territory: "Germany" } ]; -const l10nClientMock = { - locales: jest.fn().mockResolvedValue(locales), - getLocales: jest.fn().mockResolvedValue(["en_US"]), - getUILocale: jest.fn().mockResolvedValue("en_US"), - getUIKeymap: jest.fn().mockResolvedValue("en"), - keymaps: jest.fn().mockResolvedValue([]), - getKeymap: jest.fn().mockResolvedValue(undefined), - timezones: jest.fn().mockResolvedValue([]), - getTimezone: jest.fn().mockResolvedValue(undefined), - onLocalesChange: jest.fn(), - onKeymapChange: jest.fn(), - onTimezoneChange: jest.fn() -}; - -beforeEach(() => { - // if defined outside, the mock is cleared automatically - createClient.mockImplementation(() => { - return { - l10n: l10nClientMock - }; - }); -}); - it("displays the selected locale", async () => { - installerRender(, { withL10n: true }); + plainRender(, { withL10n: true }); await screen.findByText("English (United States)"); }); - -describe("when the selected locales change", () => { - it("updates the proposal", async () => { - const [mockFunction, callbacks] = createCallbackMock(); - l10nClientMock.onLocalesChange = mockFunction; - - installerRender(, { withL10n: true }); - await screen.findByText("English (United States)"); - - const [cb] = callbacks; - act(() => cb(["de_DE"])); - - await screen.findByText("German (Germany)"); - }); -}); From 5236f86caadc17d41dc8167acf82c7d1cae16318 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Thu, 4 Jul 2024 15:20:11 +0100 Subject: [PATCH 31/38] test(web): remove uneeded mock from L10nSection test --- web/src/components/overview/L10nSection.test.jsx | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/web/src/components/overview/L10nSection.test.jsx b/web/src/components/overview/L10nSection.test.jsx index 7ddf3347f2..dad4cde147 100644 --- a/web/src/components/overview/L10nSection.test.jsx +++ b/web/src/components/overview/L10nSection.test.jsx @@ -24,7 +24,10 @@ import { screen } from "@testing-library/react"; import { plainRender } from "~/test-utils"; import { L10nSection } from "~/components/overview"; -jest.mock("~/client"); +const locales = [ + { id: "en_US.UTF-8", name: "English", territory: "United States" }, + { id: "de_DE.UTF-8", name: "German", territory: "Germany" } +]; jest.mock("~/queries/l10n", () => ({ localesQuery: () => ({ @@ -33,15 +36,10 @@ jest.mock("~/queries/l10n", () => ({ }), configQuery: () => ({ queryKey: ["l10n", "config"], - queryFn: jest.fn().mockResolvedValue({ locales: ["en_US.UTF-8"]}) + queryFn: jest.fn().mockResolvedValue({ locales: ["en_US.UTF-8"] }) }) })); -const locales = [ - { id: "en_US.UTF-8", name: "English", territory: "United States" }, - { id: "de_DE.UTF-8", name: "German", territory: "Germany" } -]; - it("displays the selected locale", async () => { plainRender(, { withL10n: true }); From 82267a95a97e62f7d7efdb6bb21db06f888b7608 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Thu, 4 Jul 2024 16:11:43 +0100 Subject: [PATCH 32/38] feat(web): add a hook for invalidating cached data --- web/src/queries/hooks.js | 65 +++++++++++++++++++++++++++++++++++ web/src/queries/hooks.test.js | 49 ++++++++++++++++++++++++++ web/src/queries/l10n.js | 10 +++--- 3 files changed, 118 insertions(+), 6 deletions(-) create mode 100644 web/src/queries/hooks.js create mode 100644 web/src/queries/hooks.test.js diff --git a/web/src/queries/hooks.js b/web/src/queries/hooks.js new file mode 100644 index 0000000000..1c64660b2f --- /dev/null +++ b/web/src/queries/hooks.js @@ -0,0 +1,65 @@ +/* + * Copyright (c) [2024] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of version 2 of the GNU General Public License as published + * by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, contact SUSE LLC. + * + * To contact SUSE LLC about this file by physical or electronic mail, you may + * find current contact information at www.suse.com. + */ + +import { useRevalidator } from "react-router-dom"; +import { useQueryClient } from "@tanstack/react-query"; + +/** + * Allows invalidating cached data + * + * This hook is useful for marking data as outdated and retrieve it again. To do so, it performs two important steps + * - ask @tanstack/react-query to invalidate query matching given key + * - ask react-router-dom for a revalidation of loaded data + * + * TODO: rethink the revalidation; we may decide to keep the outdated data + * instead, but warning the user about it (as Github does when reviewing a PR, + * for example) + * + * TODO: allow to specify more than one queryKey + * + * To know more, please visit the documentation of these dependencies + * + * - https://tanstack.com/query/v5/docs/framework/react/guides/query-invalidation + * - https://reactrouter.com/en/main/hooks/use-revalidator#userevalidator + * + * @example + * + * const dataInvalidator = useDataInvalidator(); + * + * useEffect(() => { + * dataInvalidator({ queryKey: ["user", "auth"] }) + * }, [dataInvalidator]); + */ +const useDataInvalidator = () => { + const queryClient = useQueryClient(); + const revalidator = useRevalidator(); + + const dataInvalidator = ({ queryKey }) => { + if (queryKey) queryClient.invalidateQueries({ queryKey }); + revalidator.revalidate(); + }; + + return dataInvalidator; +}; + +export { + useDataInvalidator +}; diff --git a/web/src/queries/hooks.test.js b/web/src/queries/hooks.test.js new file mode 100644 index 0000000000..ad3602f2fe --- /dev/null +++ b/web/src/queries/hooks.test.js @@ -0,0 +1,49 @@ +/* + * Copyright (c) [2024] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of version 2 of the GNU General Public License as published + * by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, contact SUSE LLC. + * + * To contact SUSE LLC about this file by physical or electronic mail, you may + * find current contact information at www.suse.com. + */ + +import React from "react"; +import { renderHook } from "@testing-library/react"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { useDataInvalidator } from "~/queries/hooks.js"; + +const mockRevalidateFn = jest.fn(); +jest.mock("react-router-dom", () => ({ + ...jest.requireActual("react-router-dom"), + useRevalidator: () => ({ revalidate: mockRevalidateFn }) +})); + +const queryClient = new QueryClient(); +jest.spyOn(queryClient, "invalidateQueries"); +const wrapper = ({ children }) => ( + + {children} + +); + +describe("useDataInvalidator", () => { + it("forces a data/cache refresh", () => { + const { result } = renderHook(() => useDataInvalidator(), { wrapper }); + const { current: dataInvalidator } = result; + dataInvalidator({ queryKey: "fakeQuery" }) + expect(queryClient.invalidateQueries).toHaveBeenCalledWith({ queryKey: "fakeQuery" }); + expect(mockRevalidateFn).toHaveBeenCalled(); + }); +}); diff --git a/web/src/queries/l10n.js b/web/src/queries/l10n.js index d75d25f37a..59af07a1f4 100644 --- a/web/src/queries/l10n.js +++ b/web/src/queries/l10n.js @@ -20,9 +20,9 @@ */ import React from "react"; -import { useRevalidator } from "react-router-dom"; import { useQueryClient, useMutation } from "@tanstack/react-query"; import { useInstallerClient } from "~/context/installer"; +import { useDataInvalidator } from "~/queries/hooks"; import { timezoneUTCOffset } from "~/utils"; /** @@ -113,20 +113,18 @@ const useConfigMutation = () => { * revalidate its data (executing the loaders again). */ const useL10nConfigChanges = () => { - const queryClient = useQueryClient(); + const dataInvalidator = useDataInvalidator(); const client = useInstallerClient(); - const revalidator = useRevalidator(); React.useEffect(() => { if (!client) return; return client.ws().onEvent(event => { if (event.type === "L10nConfigChanged") { - queryClient.invalidateQueries({ queryKey: ["l10n", "config"] }); - revalidator.revalidate(); + dataInvalidator({ queryKey: ["l10n", "config"] }); } }); - }, [queryClient, client, revalidator]); + }, [client, dataInvalidator]); }; export { From 2ead88474d30f3d53baeca531c40e9460f31c399 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Thu, 4 Jul 2024 17:10:33 +0100 Subject: [PATCH 33/38] fix(web): please ESLint --- web/src/queries/hooks.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/src/queries/hooks.test.js b/web/src/queries/hooks.test.js index ad3602f2fe..6db7d4b3ee 100644 --- a/web/src/queries/hooks.test.js +++ b/web/src/queries/hooks.test.js @@ -42,7 +42,7 @@ describe("useDataInvalidator", () => { it("forces a data/cache refresh", () => { const { result } = renderHook(() => useDataInvalidator(), { wrapper }); const { current: dataInvalidator } = result; - dataInvalidator({ queryKey: "fakeQuery" }) + dataInvalidator({ queryKey: "fakeQuery" }); expect(queryClient.invalidateQueries).toHaveBeenCalledWith({ queryKey: "fakeQuery" }); expect(mockRevalidateFn).toHaveBeenCalled(); }); From b8412721b0005065400f4f7e106abbe40f513321 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Fri, 5 Jul 2024 09:53:58 +0100 Subject: [PATCH 34/38] test(web): add tests for l10n selectors --- .../l10n/KeyboardSelection.test.jsx | 57 +++++++++++++++++++ .../components/l10n/LocaleSelection.test.jsx | 57 +++++++++++++++++++ .../l10n/TimezoneSelection.test.jsx | 57 +++++++++++++++++++ 3 files changed, 171 insertions(+) create mode 100644 web/src/components/l10n/KeyboardSelection.test.jsx create mode 100644 web/src/components/l10n/LocaleSelection.test.jsx create mode 100644 web/src/components/l10n/TimezoneSelection.test.jsx diff --git a/web/src/components/l10n/KeyboardSelection.test.jsx b/web/src/components/l10n/KeyboardSelection.test.jsx new file mode 100644 index 0000000000..fd733dde25 --- /dev/null +++ b/web/src/components/l10n/KeyboardSelection.test.jsx @@ -0,0 +1,57 @@ +/* + * Copyright (c) [2024] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of version 2 of the GNU General Public License as published + * by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, contact SUSE LLC. + * + * To contact SUSE LLC about this file by physical or electronic mail, you may + * find current contact information at www.suse.com. + */ + +import React from "react"; +import KeyboardSelection from "./KeyboardSelection"; +import userEvent from "@testing-library/user-event"; +import { screen } from "@testing-library/react"; +import { mockNavigateFn, plainRender } from "~/test-utils"; + +const keymaps = [ + { id: "us", name: "English" }, + { id: "es", name: "Spanish" } +]; + +const mockConfigMutation = { + mutate: jest.fn() +}; + +jest.mock("~/queries/l10n", () => ({ + ...jest.requireActual("~/queries/l10n"), + useConfigMutation: () => mockConfigMutation +})); + +jest.mock("react-router-dom", () => ({ + ...jest.requireActual("react-router-dom"), + useNavigate: () => mockNavigateFn, + useLoaderData: () => ({ keymaps, keymap: keymaps[0] }) +})); + +it("allows changing the keyboard", async () => { + plainRender(); + + const option = await screen.findByText("Spanish"); + await userEvent.click(option); + const button = await screen.findByRole("button", { name: "Select" }); + await userEvent.click(button); + expect(mockConfigMutation.mutate).toHaveBeenCalledWith({ keymap: "es" }); + expect(mockNavigateFn).toHaveBeenCalledWith(-1); +}); diff --git a/web/src/components/l10n/LocaleSelection.test.jsx b/web/src/components/l10n/LocaleSelection.test.jsx new file mode 100644 index 0000000000..1fc001578f --- /dev/null +++ b/web/src/components/l10n/LocaleSelection.test.jsx @@ -0,0 +1,57 @@ +/* + * Copyright (c) [2023-2024] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of version 2 of the GNU General Public License as published + * by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, contact SUSE LLC. + * + * To contact SUSE LLC about this file by physical or electronic mail, you may + * find current contact information at www.suse.com. + */ + +import React from "react"; +import LocaleSelection from "./LocaleSelection"; +import userEvent from "@testing-library/user-event"; +import { screen } from "@testing-library/react"; +import { mockNavigateFn, plainRender } from "~/test-utils"; + +const locales = [ + { id: "en_US.UTF-8", name: "English", territory: "United States" }, + { id: "es_ES.UTF-8", name: "Spanish", territory: "Spain" } +]; + +const mockConfigMutation = { + mutate: jest.fn() +}; + +jest.mock("~/queries/l10n", () => ({ + ...jest.requireActual("~/queries/l10n"), + useConfigMutation: () => mockConfigMutation +})); + +jest.mock("react-router-dom", () => ({ + ...jest.requireActual("react-router-dom"), + useNavigate: () => mockNavigateFn, + useLoaderData: () => ({ locales, locale: locales[0] }) +})); + +it("allows changing the keyboard", async () => { + plainRender(); + + const option = await screen.findByText("Spanish"); + await userEvent.click(option); + const button = await screen.findByRole("button", { name: "Select" }); + await userEvent.click(button); + expect(mockConfigMutation.mutate).toHaveBeenCalledWith({ locales: ["es_ES.UTF-8"] }); + expect(mockNavigateFn).toHaveBeenCalledWith(-1); +}); diff --git a/web/src/components/l10n/TimezoneSelection.test.jsx b/web/src/components/l10n/TimezoneSelection.test.jsx new file mode 100644 index 0000000000..efecd089fb --- /dev/null +++ b/web/src/components/l10n/TimezoneSelection.test.jsx @@ -0,0 +1,57 @@ +/* + * Copyright (c) [2024] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of version 2 of the GNU General Public License as published + * by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, contact SUSE LLC. + * + * To contact SUSE LLC about this file by physical or electronic mail, you may + * find current contact information at www.suse.com. + */ + +import React from "react"; +import TimezoneSelection from "./TimezoneSelection"; +import userEvent from "@testing-library/user-event"; +import { screen } from "@testing-library/react"; +import { mockNavigateFn, plainRender } from "~/test-utils"; + +const timezones = [ + { id: "Europe/Berlin", parts: ["Europe", "Berlin"], country: "Germany", utcOffset: 1 }, + { id: "Europe/Madrid", parts: ["Europe", "Madrid"], country: "Spain", utfOffset: 1 } +]; + +const mockConfigMutation = { + mutate: jest.fn() +}; + +jest.mock("~/queries/l10n", () => ({ + ...jest.requireActual("~/queries/l10n"), + useConfigMutation: () => mockConfigMutation +})); + +jest.mock("react-router-dom", () => ({ + ...jest.requireActual("react-router-dom"), + useNavigate: () => mockNavigateFn, + useLoaderData: () => ({ timezones, timezone: timezones[0] }) +})); + +it("allows changing the keyboard", async () => { + plainRender(); + + const option = await screen.findByText("Europe-Madrid"); + await userEvent.click(option); + const button = await screen.findByRole("button", { name: "Select" }); + await userEvent.click(button); + expect(mockConfigMutation.mutate).toHaveBeenCalledWith({ timezone: "Europe/Madrid" }); + expect(mockNavigateFn).toHaveBeenCalledWith(-1); +}); From c9ee944f6423438af71ebc018e1f8b314fa4f3fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Mon, 8 Jul 2024 10:33:59 +0100 Subject: [PATCH 35/38] fix(web): drop repeated invalidation --- web/src/queries/l10n.js | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/web/src/queries/l10n.js b/web/src/queries/l10n.js index 59af07a1f4..75d193251b 100644 --- a/web/src/queries/l10n.js +++ b/web/src/queries/l10n.js @@ -88,8 +88,6 @@ const keymapsQuery = () => ({ * It does not require to call `useMutation`. */ const useConfigMutation = () => { - const queryClient = useQueryClient(); - const query = { mutationFn: (newConfig) => fetch("/api/l10n/config", { @@ -98,10 +96,7 @@ const useConfigMutation = () => { headers: { "Content-Type": "application/json", }, - }), - onSuccess: () => - queryClient.invalidateQueries({ queryKey: ["l10n", "config"] }) - , + }) }; return useMutation(query); }; From b4e169aba6c2ea9884b1699abb1e9e47ff37831a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Mon, 8 Jul 2024 11:59:12 +0100 Subject: [PATCH 36/38] doc(web): update the changes file --- web/package/agama-web-ui.changes | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/web/package/agama-web-ui.changes b/web/package/agama-web-ui.changes index ee19817591..b8caf21bba 100644 --- a/web/package/agama-web-ui.changes +++ b/web/package/agama-web-ui.changes @@ -1,3 +1,11 @@ +------------------------------------------------------------------- +Mon Jul 8 10:56:14 UTC 2024 - Imobach Gonzalez Sosa + +- Introduce TanStack Query to handle for data fetching and state + management (gh#openSUSE/agama#1439). +- Replace the l10n context with a solution based on TanStack + Query. + ------------------------------------------------------------------- Wed Jul 3 07:17:55 UTC 2024 - Imobach Gonzalez Sosa From cbbb727a012bb80fce723ad9e75b0b0f175692a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Mon, 8 Jul 2024 13:18:04 +0100 Subject: [PATCH 37/38] doc(fix): fix changes entry --- web/package/agama-web-ui.changes | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/web/package/agama-web-ui.changes b/web/package/agama-web-ui.changes index b8caf21bba..9dda2dcb85 100644 --- a/web/package/agama-web-ui.changes +++ b/web/package/agama-web-ui.changes @@ -1,8 +1,8 @@ ------------------------------------------------------------------- Mon Jul 8 10:56:14 UTC 2024 - Imobach Gonzalez Sosa -- Introduce TanStack Query to handle for data fetching and state - management (gh#openSUSE/agama#1439). +- Introduce TanStack Query for data fetching and state management + (gh#openSUSE/agama#1439). - Replace the l10n context with a solution based on TanStack Query. From 8838564fdf6475419313b1728ab9400b1e96c718 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Mon, 8 Jul 2024 13:23:08 +0100 Subject: [PATCH 38/38] fix(service): fix manager tests --- service/test/agama/software/manager_test.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/service/test/agama/software/manager_test.rb b/service/test/agama/software/manager_test.rb index 0b24c2b8a5..8643608c20 100644 --- a/service/test/agama/software/manager_test.rb +++ b/service/test/agama/software/manager_test.rb @@ -253,7 +253,8 @@ expect(products).to all(be_a(Agama::Software::Product)) expect(products).to contain_exactly( an_object_having_attributes(id: "Tumbleweed"), - an_object_having_attributes(id: "MicroOS") + an_object_having_attributes(id: "MicroOS"), + an_object_having_attributes(id: "Leap_16.0") ) end end @@ -335,7 +336,7 @@ expect(proposal).to receive(:set_resolvables) .with("agama", :pattern, [], { optional: true }) expect(proposal).to receive(:set_resolvables) - .with("agama", :package, ["NetworkManager", "openSUSE-repos"]) + .with("agama", :package, ["NetworkManager", "openSUSE-repos-Tumbleweed"]) expect(proposal).to receive(:set_resolvables) .with("agama", :package, [], { optional: true }) subject.propose