From caddf8b1c81cffa3f5fb2c4089fc02904686f738 Mon Sep 17 00:00:00 2001 From: Shubham Oulkar Date: Tue, 29 Jul 2025 20:14:43 +0530 Subject: [PATCH 1/8] fix aria-label a11y --- apps/site/components/withNavBar.tsx | 7 ++++++- packages/i18n/src/locales/en.json | 5 ++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/apps/site/components/withNavBar.tsx b/apps/site/components/withNavBar.tsx index 8cab6b824c919..d1c3bfde0ed26 100644 --- a/apps/site/components/withNavBar.tsx +++ b/apps/site/components/withNavBar.tsx @@ -39,6 +39,11 @@ const WithNavBar: FC = () => { const toggleCurrentTheme = () => setTheme(resolvedTheme === 'dark' ? 'light' : 'dark'); + const themeToggleAriaLabel = + resolvedTheme === 'dark' + ? t('components.common.themeToggle.label.light') + : t('components.common.themeToggle.label.dark'); + const changeLanguage = (locale: SimpleLocaleConfig) => replace(pathname!, { locale: locale.code }); @@ -63,7 +68,7 @@ const WithNavBar: FC = () => { Date: Tue, 29 Jul 2025 20:20:33 +0530 Subject: [PATCH 2/8] fix test --- apps/site/tests/e2e/general-behavior.spec.ts | 3 ++- packages/i18n/src/locales/en.json | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/apps/site/tests/e2e/general-behavior.spec.ts b/apps/site/tests/e2e/general-behavior.spec.ts index 78d455b81e411..7b3aad94411c0 100644 --- a/apps/site/tests/e2e/general-behavior.spec.ts +++ b/apps/site/tests/e2e/general-behavior.spec.ts @@ -13,7 +13,8 @@ const locators = { navLinksLocator: `[aria-label="${englishLocale.components.containers.navBar.controls.toggle}"] + div`, // Global UI controls languageDropdownName: englishLocale.components.common.languageDropdown.label, - themeToggleName: englishLocale.components.common.themeToggle.label, + // default light theme + themeToggleName: englishLocale.components.common.themeToggle.label.light, // Search components (from Orama library) searchButtonTag: 'orama-button', diff --git a/packages/i18n/src/locales/en.json b/packages/i18n/src/locales/en.json index 6d1aafac741a6..514c822b106d0 100644 --- a/packages/i18n/src/locales/en.json +++ b/packages/i18n/src/locales/en.json @@ -230,8 +230,8 @@ }, "themeToggle": { "label": { - "light": "Activate Light Mode", - "dark": "Activate Dark Mode" + "light": "Toggle Light Mode", + "dark": "Toggle Dark Mode" } } }, From d47d496f865300eb7cab801f0a22a68baf6fdfbc Mon Sep 17 00:00:00 2001 From: Shubham Oulkar Date: Tue, 29 Jul 2025 20:32:21 +0530 Subject: [PATCH 3/8] add a11y test for aria-label --- apps/site/tests/e2e/general-behavior.spec.ts | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/apps/site/tests/e2e/general-behavior.spec.ts b/apps/site/tests/e2e/general-behavior.spec.ts index 7b3aad94411c0..1a5a1b006f433 100644 --- a/apps/site/tests/e2e/general-behavior.spec.ts +++ b/apps/site/tests/e2e/general-behavior.spec.ts @@ -23,7 +23,9 @@ const locators = { }; const getTheme = (page: Page) => - page.evaluate(() => document.documentElement.dataset.theme); + page.evaluate( + () => document.documentElement.dataset.theme as 'light' | 'dark' + ); const openLanguageMenu = async (page: Page) => { const button = page.getByRole('button', { @@ -71,11 +73,22 @@ test.describe('Node.js Website', () => { await expect(themeToggle).toBeVisible(); const initialTheme = await getTheme(page); + const initialAriaLabel = await themeToggle.getAttribute('aria-label'); + expect(initialAriaLabel).toBe( + englishLocale.components.common.themeToggle.label[initialTheme] + ); + await themeToggle.click(); const newTheme = await getTheme(page); - expect(newTheme).not.toEqual(initialTheme); + const newAriaLabel = await themeToggle.getAttribute('aria-label'); + + expect(newTheme).not.toBe(initialTheme); expect(['light', 'dark']).toContain(newTheme); + + expect(newAriaLabel).toBe( + englishLocale.components.common.themeToggle.label[newTheme] + ); }); test('should persist theme across page navigation', async ({ page }) => { From fe4dd3d7a77c03e9069c6ae75fa492216322d332 Mon Sep 17 00:00:00 2001 From: Shubham Oulkar Date: Wed, 30 Jul 2025 09:28:46 +0530 Subject: [PATCH 4/8] fix i18n aria-label text --- apps/site/components/withNavBar.tsx | 4 ++-- apps/site/tests/e2e/general-behavior.spec.ts | 6 +++--- packages/i18n/src/locales/en.json | 6 ++---- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/apps/site/components/withNavBar.tsx b/apps/site/components/withNavBar.tsx index d1c3bfde0ed26..17f5ddc11eca1 100644 --- a/apps/site/components/withNavBar.tsx +++ b/apps/site/components/withNavBar.tsx @@ -41,8 +41,8 @@ const WithNavBar: FC = () => { const themeToggleAriaLabel = resolvedTheme === 'dark' - ? t('components.common.themeToggle.label.light') - : t('components.common.themeToggle.label.dark'); + ? t('components.common.themeToggle.light') + : t('components.common.themeToggle.dark'); const changeLanguage = (locale: SimpleLocaleConfig) => replace(pathname!, { locale: locale.code }); diff --git a/apps/site/tests/e2e/general-behavior.spec.ts b/apps/site/tests/e2e/general-behavior.spec.ts index 1a5a1b006f433..c718708734df1 100644 --- a/apps/site/tests/e2e/general-behavior.spec.ts +++ b/apps/site/tests/e2e/general-behavior.spec.ts @@ -14,7 +14,7 @@ const locators = { // Global UI controls languageDropdownName: englishLocale.components.common.languageDropdown.label, // default light theme - themeToggleName: englishLocale.components.common.themeToggle.label.light, + themeToggleName: englishLocale.components.common.themeToggle.light, // Search components (from Orama library) searchButtonTag: 'orama-button', @@ -75,7 +75,7 @@ test.describe('Node.js Website', () => { const initialTheme = await getTheme(page); const initialAriaLabel = await themeToggle.getAttribute('aria-label'); expect(initialAriaLabel).toBe( - englishLocale.components.common.themeToggle.label[initialTheme] + englishLocale.components.common.themeToggle[initialTheme] ); await themeToggle.click(); @@ -87,7 +87,7 @@ test.describe('Node.js Website', () => { expect(['light', 'dark']).toContain(newTheme); expect(newAriaLabel).toBe( - englishLocale.components.common.themeToggle.label[newTheme] + englishLocale.components.common.themeToggle[newTheme] ); }); diff --git a/packages/i18n/src/locales/en.json b/packages/i18n/src/locales/en.json index 514c822b106d0..5465c88e9882e 100644 --- a/packages/i18n/src/locales/en.json +++ b/packages/i18n/src/locales/en.json @@ -229,10 +229,8 @@ "label": "Choose Language" }, "themeToggle": { - "label": { - "light": "Toggle Light Mode", - "dark": "Toggle Dark Mode" - } + "light": "Switch to Light Mode", + "dark": "Switch to Dark Mode" } }, "metabar": { From d5a3392ab68015b2f81d8b845cd8d9f8c0374f04 Mon Sep 17 00:00:00 2001 From: Shubham Oulkar Date: Wed, 30 Jul 2025 11:18:22 +0530 Subject: [PATCH 5/8] fix hydration --- apps/site/components/withNavBar.tsx | 9 +++++++-- packages/i18n/src/locales/en.json | 1 + 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/apps/site/components/withNavBar.tsx b/apps/site/components/withNavBar.tsx index 17f5ddc11eca1..465e3a48d88d2 100644 --- a/apps/site/components/withNavBar.tsx +++ b/apps/site/components/withNavBar.tsx @@ -11,6 +11,7 @@ import type { SimpleLocaleConfig } from '@node-core/ui-components/types'; import dynamic from 'next/dynamic'; import { useLocale, useTranslations } from 'next-intl'; import { useTheme } from 'next-themes'; +import { useState, useEffect } from 'react'; import type { FC } from 'react'; import Link from '#site/components/Link'; @@ -35,12 +36,16 @@ const WithNavBar: FC = () => { const t = useTranslations(); const locale = useLocale(); + const [mounted, setMounted] = useState(false); + + useEffect(() => setMounted(true), []); const toggleCurrentTheme = () => setTheme(resolvedTheme === 'dark' ? 'light' : 'dark'); - const themeToggleAriaLabel = - resolvedTheme === 'dark' + const themeToggleAriaLabel = !mounted + ? t('components.common.themeToggle.loading') + : resolvedTheme === 'dark' ? t('components.common.themeToggle.light') : t('components.common.themeToggle.dark'); diff --git a/packages/i18n/src/locales/en.json b/packages/i18n/src/locales/en.json index 5465c88e9882e..4e8a5d2905bb6 100644 --- a/packages/i18n/src/locales/en.json +++ b/packages/i18n/src/locales/en.json @@ -229,6 +229,7 @@ "label": "Choose Language" }, "themeToggle": { + "loading": "Loading theme label...", "light": "Switch to Light Mode", "dark": "Switch to Dark Mode" } From 821f13485897f0c21681ed21ec740e35ef0203f7 Mon Sep 17 00:00:00 2001 From: Shubham Oulkar Date: Thu, 31 Jul 2025 20:18:04 +0530 Subject: [PATCH 6/8] refactor test --- apps/site/tests/e2e/general-behavior.spec.ts | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/apps/site/tests/e2e/general-behavior.spec.ts b/apps/site/tests/e2e/general-behavior.spec.ts index c718708734df1..ab4b5dc89dc9e 100644 --- a/apps/site/tests/e2e/general-behavior.spec.ts +++ b/apps/site/tests/e2e/general-behavior.spec.ts @@ -13,9 +13,12 @@ const locators = { navLinksLocator: `[aria-label="${englishLocale.components.containers.navBar.controls.toggle}"] + div`, // Global UI controls languageDropdownName: englishLocale.components.common.languageDropdown.label, - // default light theme + // Initially, the themeToggle's name will be the light mode i18n. themeToggleName: englishLocale.components.common.themeToggle.light, - + themeToggleAriaLabels: { + light: englishLocale.components.common.themeToggle.light, + dark: englishLocale.components.common.themeToggle.dark, + }, // Search components (from Orama library) searchButtonTag: 'orama-button', searchInputTag: 'orama-input', @@ -75,7 +78,7 @@ test.describe('Node.js Website', () => { const initialTheme = await getTheme(page); const initialAriaLabel = await themeToggle.getAttribute('aria-label'); expect(initialAriaLabel).toBe( - englishLocale.components.common.themeToggle[initialTheme] + locators.themeToggleAriaLabels[initialTheme] ); await themeToggle.click(); @@ -86,9 +89,7 @@ test.describe('Node.js Website', () => { expect(newTheme).not.toBe(initialTheme); expect(['light', 'dark']).toContain(newTheme); - expect(newAriaLabel).toBe( - englishLocale.components.common.themeToggle[newTheme] - ); + expect(newAriaLabel).toBe(locators.themeToggleAriaLabels[newTheme]); }); test('should persist theme across page navigation', async ({ page }) => { From 07420f19d4e295f947b66912d774b87fe353b0af Mon Sep 17 00:00:00 2001 From: Shubham Oulkar Date: Sat, 2 Aug 2025 16:13:24 +0530 Subject: [PATCH 7/8] fix test --- apps/site/tests/e2e/general-behavior.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/site/tests/e2e/general-behavior.spec.ts b/apps/site/tests/e2e/general-behavior.spec.ts index ab4b5dc89dc9e..93e589514cac3 100644 --- a/apps/site/tests/e2e/general-behavior.spec.ts +++ b/apps/site/tests/e2e/general-behavior.spec.ts @@ -14,7 +14,7 @@ const locators = { // Global UI controls languageDropdownName: englishLocale.components.common.languageDropdown.label, // Initially, the themeToggle's name will be the light mode i18n. - themeToggleName: englishLocale.components.common.themeToggle.light, + themeToggleName: englishLocale.components.common.themeToggle.loading, themeToggleAriaLabels: { light: englishLocale.components.common.themeToggle.light, dark: englishLocale.components.common.themeToggle.dark, From 61e8ea3105cd41114c7daf8e8c23affa566a212c Mon Sep 17 00:00:00 2001 From: Shubham Oulkar Date: Sun, 3 Aug 2025 12:31:10 +0530 Subject: [PATCH 8/8] Exclude ThemeToggle btn from SSR --- apps/site/components/withNavBar.tsx | 20 ++++++++------ apps/site/tests/e2e/general-behavior.spec.ts | 26 +++++++++---------- packages/i18n/src/locales/en.json | 1 - .../src/Containers/NavBar/index.module.css | 6 +++++ 4 files changed, 31 insertions(+), 22 deletions(-) diff --git a/apps/site/components/withNavBar.tsx b/apps/site/components/withNavBar.tsx index 465e3a48d88d2..24ca3d9d794f2 100644 --- a/apps/site/components/withNavBar.tsx +++ b/apps/site/components/withNavBar.tsx @@ -2,7 +2,6 @@ import LanguageDropdown from '@node-core/ui-components/Common/LanguageDropDown'; import Skeleton from '@node-core/ui-components/Common/Skeleton'; -import ThemeToggle from '@node-core/ui-components/Common/ThemeToggle'; import NavBar from '@node-core/ui-components/Containers/NavBar'; // TODO(@AvivKeller): I don't like that we are importing styles from another module import styles from '@node-core/ui-components/Containers/NavBar/index.module.css'; @@ -11,7 +10,6 @@ import type { SimpleLocaleConfig } from '@node-core/ui-components/types'; import dynamic from 'next/dynamic'; import { useLocale, useTranslations } from 'next-intl'; import { useTheme } from 'next-themes'; -import { useState, useEffect } from 'react'; import type { FC } from 'react'; import Link from '#site/components/Link'; @@ -28,6 +26,16 @@ const SearchButton = dynamic(() => import('#site/components/Common/Search'), { ), }); +const ThemeToggle = dynamic( + () => import('@node-core/ui-components/Common/ThemeToggle'), + { + ssr: false, + loading: () => ( + + ), + } +); + const WithNavBar: FC = () => { const { navigationItems } = useSiteNavigation(); const { resolvedTheme, setTheme } = useTheme(); @@ -36,16 +44,12 @@ const WithNavBar: FC = () => { const t = useTranslations(); const locale = useLocale(); - const [mounted, setMounted] = useState(false); - - useEffect(() => setMounted(true), []); const toggleCurrentTheme = () => setTheme(resolvedTheme === 'dark' ? 'light' : 'dark'); - const themeToggleAriaLabel = !mounted - ? t('components.common.themeToggle.loading') - : resolvedTheme === 'dark' + const themeToggleAriaLabel = + resolvedTheme === 'dark' ? t('components.common.themeToggle.light') : t('components.common.themeToggle.dark'); diff --git a/apps/site/tests/e2e/general-behavior.spec.ts b/apps/site/tests/e2e/general-behavior.spec.ts index 93e589514cac3..2fcb9f0fcc5b8 100644 --- a/apps/site/tests/e2e/general-behavior.spec.ts +++ b/apps/site/tests/e2e/general-behavior.spec.ts @@ -13,8 +13,6 @@ const locators = { navLinksLocator: `[aria-label="${englishLocale.components.containers.navBar.controls.toggle}"] + div`, // Global UI controls languageDropdownName: englishLocale.components.common.languageDropdown.label, - // Initially, the themeToggle's name will be the light mode i18n. - themeToggleName: englishLocale.components.common.themeToggle.loading, themeToggleAriaLabels: { light: englishLocale.components.common.themeToggle.light, dark: englishLocale.components.common.themeToggle.dark, @@ -30,6 +28,11 @@ const getTheme = (page: Page) => () => document.documentElement.dataset.theme as 'light' | 'dark' ); +const getCurrentAriaLabel = (theme: string) => + theme === 'dark' + ? locators.themeToggleAriaLabels.light + : locators.themeToggleAriaLabels.dark; + const openLanguageMenu = async (page: Page) => { const button = page.getByRole('button', { name: locators.languageDropdownName, @@ -71,30 +74,27 @@ test.describe('Node.js Website', () => { test.describe('Theme', () => { test('should toggle between light/dark themes', async ({ page }) => { const themeToggle = page.getByRole('button', { - name: locators.themeToggleName, + name: /Switch to (Light|Dark) Mode/i, }); - await expect(themeToggle).toBeVisible(); const initialTheme = await getTheme(page); - const initialAriaLabel = await themeToggle.getAttribute('aria-label'); - expect(initialAriaLabel).toBe( - locators.themeToggleAriaLabels[initialTheme] - ); + const initialAriaLabel = getCurrentAriaLabel(initialTheme); + let currentAriaLabel = await themeToggle.getAttribute('aria-label'); + expect(currentAriaLabel).toBe(initialAriaLabel); await themeToggle.click(); const newTheme = await getTheme(page); - const newAriaLabel = await themeToggle.getAttribute('aria-label'); + const newAriaLabel = getCurrentAriaLabel(newTheme); + currentAriaLabel = await themeToggle.getAttribute('aria-label'); expect(newTheme).not.toBe(initialTheme); - expect(['light', 'dark']).toContain(newTheme); - - expect(newAriaLabel).toBe(locators.themeToggleAriaLabels[newTheme]); + expect(currentAriaLabel).toBe(newAriaLabel); }); test('should persist theme across page navigation', async ({ page }) => { const themeToggle = page.getByRole('button', { - name: locators.themeToggleName, + name: /Switch to (Light|Dark) Mode/i, }); await themeToggle.click(); const selectedTheme = await getTheme(page); diff --git a/packages/i18n/src/locales/en.json b/packages/i18n/src/locales/en.json index 4e8a5d2905bb6..5465c88e9882e 100644 --- a/packages/i18n/src/locales/en.json +++ b/packages/i18n/src/locales/en.json @@ -229,7 +229,6 @@ "label": "Choose Language" }, "themeToggle": { - "loading": "Loading theme label...", "light": "Switch to Light Mode", "dark": "Switch to Dark Mode" } diff --git a/packages/ui-components/src/Containers/NavBar/index.module.css b/packages/ui-components/src/Containers/NavBar/index.module.css index 331f63f46d83c..764c0edd5ec7a 100644 --- a/packages/ui-components/src/Containers/NavBar/index.module.css +++ b/packages/ui-components/src/Containers/NavBar/index.module.css @@ -108,6 +108,12 @@ span.searchButtonSkeleton { } } +span.themeToggleSkeleton { + @apply size-9 + rounded-md + py-4; +} + .ghIconWrapper { @apply size-9 rounded-md