Skip to content

Commit e1130e1

Browse files
committed
use roles when possible
1 parent 493a338 commit e1130e1

File tree

5 files changed

+77
-59
lines changed

5 files changed

+77
-59
lines changed

apps/site/components/withNavBar.tsx

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,6 @@ const WithNavBar: FC = () => {
6262
<SearchButton />
6363

6464
<ThemeToggle
65-
data-testid="theme-toggle"
6665
onClick={toggleCurrentTheme}
6766
aria-label={t('components.common.themeToggle.label')}
6867
/>

apps/site/playwright.config.ts

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,13 @@ export default defineConfig({
2020
name: 'chromium',
2121
use: { ...devices['Desktop Chrome'] },
2222
},
23-
{
24-
name: 'firefox',
25-
use: { ...devices['Desktop Firefox'] },
26-
},
27-
{
28-
name: 'webkit',
29-
use: { ...devices['Desktop Safari'] },
30-
},
23+
// {
24+
// name: 'firefox',
25+
// use: { ...devices['Desktop Firefox'] },
26+
// },
27+
// {
28+
// name: 'webkit',
29+
// use: { ...devices['Desktop Safari'] },
30+
// },
3131
],
3232
});

apps/site/tests/e2e/general-behavior.spec.ts

Lines changed: 65 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1,56 +1,75 @@
11
import { importLocale } from '@node-core/website-i18n';
22
import { test, expect, type Page } from '@playwright/test';
33

4+
const englishLocale = await importLocale('en');
5+
46
// TODO(@avivkeller): It would be ideal for all the Test IDs to not exist in the
57
// ui-components package, and instead be passed as props.
6-
const testIds = {
7-
themeToggle: 'theme-toggle',
8-
languageDropdown: 'language-selector',
9-
languageOptions: 'language-options',
10-
navLinks: 'nav-links',
11-
mobileMenuToggle: 'mobile-menu-toggle',
12-
};
13-
14-
// These are inherited from Orama, so they don't have test IDs. Instead, we use the element names directly
15-
const selectors = {
16-
searchButton: 'orama-button',
17-
searchInput: 'orama-input',
18-
searchResults: 'orama-search-results',
8+
const locators = {
9+
// Navigation elements
10+
navLinksTestID: 'nav-links',
11+
mobileMenuToggleName:
12+
englishLocale.components.containers.navBar.controls.toggle,
13+
14+
// Global UI controls
15+
languageDropdownName: englishLocale.components.common.languageDropdown.label,
16+
themeToggleName: englishLocale.components.common.themeToggle.label,
17+
18+
// Search components (from Orama library)
19+
searchButtonTag: 'orama-button',
20+
searchInputTag: 'orama-input',
21+
searchResultsTag: 'orama-search-results',
1922
};
2023

21-
// Helper functions
2224
const getTheme = (page: Page) =>
2325
page.evaluate(() => document.documentElement.dataset.theme);
2426

2527
const openLanguageMenu = async (page: Page) => {
26-
await page.getByTestId(testIds.languageDropdown).first().click();
27-
await page.waitForSelector(`data-testid=${testIds.languageOptions}`);
28-
};
28+
const button = page.getByRole('button', {
29+
name: locators.languageDropdownName,
30+
});
31+
const selector = `[aria-labelledby=${await button.getAttribute('id')}]`;
32+
await button.click();
2933

30-
const verifyTranslation = async (page: Page, locale: string) => {
31-
const localeData = await importLocale(locale);
34+
await page.waitForSelector(selector);
35+
return page.locator(selector);
36+
};
3237

33-
// Get all navigation links
34-
const links = await page.getByTestId(testIds.navLinks).locator('a').all();
38+
const verifyTranslation = async (
39+
page: Page,
40+
locale: string | Record<string, unknown>
41+
) => {
42+
// Load locale data if string code provided (e.g., 'es', 'fr')
43+
const localeData =
44+
typeof locale === 'string' ? await importLocale(locale) : locale;
45+
46+
// Get navigation links and expected translations
47+
const links = await page
48+
.getByTestId(locators.navLinksTestID)
49+
.locator('a')
50+
.all();
3551
const expectedTexts = Object.values(
3652
localeData.components.containers.navBar.links
3753
);
3854

39-
// For each link, verify its text is in the expected translations
55+
// Verify each navigation link text matches an expected translation
4056
for (const link of links) {
4157
const linkText = await link.textContent();
4258
expect(expectedTexts).toContain(linkText!.trim());
4359
}
4460
};
4561

4662
test.describe('Node.js Website', () => {
63+
// Start each test from the English homepage
4764
test.beforeEach(async ({ page }) => {
4865
await page.goto('/en');
4966
});
5067

5168
test.describe('Theme', () => {
5269
test('should toggle between light/dark themes', async ({ page }) => {
53-
const themeToggle = page.getByTestId(testIds.themeToggle).first();
70+
const themeToggle = page.getByRole('button', {
71+
name: locators.themeToggleName,
72+
});
5473
await expect(themeToggle).toBeVisible();
5574

5675
const initialTheme = await getTheme(page);
@@ -62,12 +81,13 @@ test.describe('Node.js Website', () => {
6281
});
6382

6483
test('should persist theme across page navigation', async ({ page }) => {
65-
const themeToggle = page.getByTestId(testIds.themeToggle).first();
84+
const themeToggle = page.getByRole('button', {
85+
name: locators.themeToggleName,
86+
});
6687
await themeToggle.click();
6788
const selectedTheme = await getTheme(page);
6889

6990
await page.reload();
70-
7191
expect(await getTheme(page)).toBe(selectedTheme);
7292
});
7393

@@ -86,15 +106,11 @@ test.describe('Node.js Website', () => {
86106
test('should correctly translate UI elements according to language files', async ({
87107
page,
88108
}) => {
89-
// Verify English content
90-
await verifyTranslation(page, 'en');
91-
92-
// Change to Spanish and verify
93-
await openLanguageMenu(page);
94-
await page
95-
.getByTestId(testIds.languageOptions)
96-
.getByText(/español/i)
97-
.click();
109+
await verifyTranslation(page, englishLocale);
110+
111+
// Change to Spanish and verify translations
112+
const menu = await openLanguageMenu(page);
113+
await menu.getByText(/español/i).click();
98114
await page.waitForURL(/\/es$/);
99115

100116
await verifyTranslation(page, 'es');
@@ -103,34 +119,40 @@ test.describe('Node.js Website', () => {
103119

104120
test.describe('Search', () => {
105121
test('should show and operate search functionality', async ({ page }) => {
106-
await page.locator(selectors.searchButton).click();
122+
// Open search dialog
123+
await page.locator(locators.searchButtonTag).click();
107124

108-
const searchInput = page.locator(selectors.searchInput);
125+
// Verify search input is visible and enter a search term
126+
const searchInput = page.locator(locators.searchInputTag);
109127
await expect(searchInput).toBeVisible();
110128
await searchInput.pressSequentially('express');
111129

112-
const searchResults = page.locator(selectors.searchResults);
130+
// Verify search results appear
131+
const searchResults = page.locator(locators.searchResultsTag);
113132
await expect(searchResults).toBeVisible();
114133
});
115134
});
116135

117-
test.describe('Navigation', () => {
136+
test.describe.only('Navigation', () => {
118137
test('should have functioning mobile menu on small screens', async ({
119138
page,
120139
}) => {
121-
// Set mobile viewport
140+
// Set mobile viewport size
122141
await page.setViewportSize({ width: 375, height: 667 });
123142

124-
const mobileToggle = page.getByTestId(testIds.mobileMenuToggle);
143+
// Locate mobile menu toggle button and verify it's visible
144+
const mobileToggle = page.getByRole('button', {
145+
name: locators.mobileMenuToggleName,
146+
});
125147
await expect(mobileToggle).toBeVisible();
126148

127-
const navLinks = page.getByTestId(testIds.navLinks);
149+
const navLinks = page.getByTestId(locators.navLinksTestID);
128150

129-
// Toggle menu open and verify
151+
// Toggle menu open and verify it's visible
130152
await mobileToggle.click();
131153
await expect(navLinks.first()).toBeVisible();
132154

133-
// Toggle menu closed and verify
155+
// Toggle menu closed and verify it's hidden
134156
await mobileToggle.click();
135157
await expect(navLinks.first()).not.toBeVisible();
136158
});

packages/ui-components/Common/LanguageDropDown/index.tsx

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,7 @@ const LanguageDropdown: FC<LanguageDropDownProps> = ({
2323
return (
2424
<DropdownMenu.Root>
2525
<DropdownMenu.Trigger asChild>
26-
<button
27-
className={styles.languageDropdown}
28-
aria-label={ariaLabel}
29-
data-testid="language-selector"
30-
>
26+
<button className={styles.languageDropdown} aria-label={ariaLabel}>
3127
<LanguageIcon height="20" />
3228
</button>
3329
</DropdownMenu.Trigger>
@@ -38,7 +34,7 @@ const LanguageDropdown: FC<LanguageDropDownProps> = ({
3834
className={styles.dropDownContent}
3935
sideOffset={5}
4036
>
41-
<div data-testid="language-options">
37+
<div>
4238
{availableLanguages.map(({ name, code, localName }) => (
4339
<DropdownMenu.Item
4440
key={code}

packages/ui-components/Containers/NavBar/index.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,8 @@ const NavBar: FC<PropsWithChildren<NavbarProps>> = ({
5454
<Label.Root
5555
className={style.sidebarItemTogglerLabel}
5656
htmlFor="sidebarItemToggler"
57-
data-testid="mobile-menu-toggle"
57+
role="button"
58+
aria-label={sidebarItemTogglerAriaLabel}
5859
>
5960
{navInteractionIcons[isMenuOpen ? 'close' : 'show']}
6061
</Label.Root>

0 commit comments

Comments
 (0)