From e8685a5802b296a349b22557e8390521480f93c7 Mon Sep 17 00:00:00 2001 From: Harrison Healey Date: Thu, 17 Apr 2025 12:05:35 -0400 Subject: [PATCH] MM-63313 Make theme setting radio buttons horizontal and update text (#30584) * MM-63313 Make theme setting radio buttons horizontal and update text * MM-63313 Add Playwright test for a11y of theme settings * Update snapshot * Run prettier on E2E tests * Address feedback * Ensure new test reliably passes on Firefox For whatever reason, Firefox lets you tab onto the Sidebar Styles panel while it's expanding, possibly because it's a scrollable container with overflowing content or because other browsers don't register the children of that panel as visible while the panel is animating open. Either way, we can look at the CSS on the panel to confirm when the transition is done. * Revert previous changes made to premade theme label alignment and size In the last PR, these were changed from generic divs to buttons, and the default browser style for buttons adds some extra padding and centres the button text by default, so we have to override that. * Adjust margins on inline radio group * Fix playwright test code styling * Fix bad import in E2E tests --- .../channels/settings/display_settings.ts | 58 +++++++++ .../channels/settings/settings_modal.ts | 18 +++ .../lib/src/ui/components/global_header.ts | 10 +- .../playwright/lib/src/ui/pages/channels.ts | 2 +- .../channels/theme_settings.spec.ts | 119 ++++++++++++++++++ .../highlight_without_notification.spec.ts | 63 +++++----- .../__snapshots__/color_chooser.test.tsx.snap | 1 + .../color_chooser/color_chooser.tsx | 7 +- .../user_settings_theme.tsx | 79 ++++++------ webapp/channels/src/i18n/en.json | 2 +- .../channels/src/sass/routes/_settings.scss | 9 +- 11 files changed, 288 insertions(+), 80 deletions(-) create mode 100644 e2e-tests/playwright/lib/src/ui/components/channels/settings/display_settings.ts create mode 100644 e2e-tests/playwright/specs/accessibility/channels/theme_settings.spec.ts diff --git a/e2e-tests/playwright/lib/src/ui/components/channels/settings/display_settings.ts b/e2e-tests/playwright/lib/src/ui/components/channels/settings/display_settings.ts new file mode 100644 index 0000000000..11ce1851bb --- /dev/null +++ b/e2e-tests/playwright/lib/src/ui/components/channels/settings/display_settings.ts @@ -0,0 +1,58 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import {expect, Locator} from '@playwright/test'; + +export type DisplaySettingsSection = + | 'theme' + | 'collapsedReplyThreads' + | 'clockDisplay' + | 'teammateNameDisplay' + | 'availabilityStatusOnPosts' + | 'lastActiveTime' + | 'timezone' + | 'showLinkPreviews' + | 'collapseImagePreviews' + | 'clickToReply' + | 'channelDisplayMode' + | 'oneClickReactions' + | 'language'; + +const sectionTitles: Record = { + theme: 'Theme', + collapsedReplyThreads: 'Threaded Discussions', + clockDisplay: 'Clock Display', + teammateNameDisplay: 'Teammate Name Display', + availabilityStatusOnPosts: 'Show online availability on profile images', + lastActiveTime: 'Share last active time', + timezone: 'Timezone', + showLinkPreviews: 'Website Link Previews', + collapseImagePreviews: 'Default Appearance of Image Previews', + clickToReply: 'Click to open threads', + channelDisplayMode: 'Channel Display', + oneClickReactions: 'Quick reactions on messages', + language: 'Language', +}; + +export default class DisplaySettings { + readonly container: Locator; + + constructor(container: Locator) { + this.container = container; + } + + async toBeVisible() { + await expect(this.container).toBeVisible(); + } + + async expandSection(section: DisplaySettingsSection) { + await this.container.getByText(sectionTitles[section]).click(); + await this.verifySectionIsExpanded(section); + } + + async verifySectionIsExpanded(section: DisplaySettingsSection) { + await expect(this.container.locator('.section-min', {hasText: sectionTitles[section]})).not.toBeVisible(); + + await expect(this.container.locator('.section-max', {hasText: sectionTitles[section]})).toBeVisible(); + } +} diff --git a/e2e-tests/playwright/lib/src/ui/components/channels/settings/settings_modal.ts b/e2e-tests/playwright/lib/src/ui/components/channels/settings/settings_modal.ts index eab2639124..7e8c282cc9 100644 --- a/e2e-tests/playwright/lib/src/ui/components/channels/settings/settings_modal.ts +++ b/e2e-tests/playwright/lib/src/ui/components/channels/settings/settings_modal.ts @@ -3,6 +3,7 @@ import {Locator, expect} from '@playwright/test'; +import DisplaySettings from './display_settings'; import NotificationsSettings from './notification_settings'; export default class SettingsModal { @@ -11,11 +12,17 @@ export default class SettingsModal { readonly notificationsSettingsTab; readonly notificationsSettings; + readonly displaySettingsTab; + readonly displaySettings; + constructor(container: Locator) { this.container = container; this.notificationsSettingsTab = container.locator('#notificationsButton'); this.notificationsSettings = new NotificationsSettings(container.locator('#notificationsSettings')); + + this.displaySettingsTab = container.locator('#displayButton'); + this.displaySettings = new DisplaySettings(container.locator('#displaySettings')); } async toBeVisible() { @@ -27,6 +34,17 @@ export default class SettingsModal { await this.notificationsSettingsTab.click(); await this.notificationsSettings.toBeVisible(); + + return this.notificationsSettings; + } + + async openDisplayTab() { + await expect(this.displaySettingsTab).toBeVisible(); + await this.displaySettingsTab.click(); + + await this.displaySettings.toBeVisible(); + + return this.displaySettings; } async closeModal() { diff --git a/e2e-tests/playwright/lib/src/ui/components/global_header.ts b/e2e-tests/playwright/lib/src/ui/components/global_header.ts index 5c001e2fe6..481c03c25a 100644 --- a/e2e-tests/playwright/lib/src/ui/components/global_header.ts +++ b/e2e-tests/playwright/lib/src/ui/components/global_header.ts @@ -3,7 +3,10 @@ import {Locator, expect} from '@playwright/test'; +import {ChannelsPage} from '../pages'; + export default class GlobalHeader { + readonly channelsPage: ChannelsPage; readonly container: Locator; readonly accountMenuButton; @@ -12,7 +15,8 @@ export default class GlobalHeader { readonly settingsButton; readonly searchBox; - constructor(container: Locator) { + constructor(channelsPage: ChannelsPage, container: Locator) { + this.channelsPage = channelsPage; this.container = container; this.accountMenuButton = container.getByRole('button', {name: "'s account menu"}); @@ -34,6 +38,10 @@ export default class GlobalHeader { async openSettings() { await expect(this.settingsButton).toBeVisible(); await this.settingsButton.click(); + + await this.channelsPage.settingsModal.toBeVisible(); + + return this.channelsPage.settingsModal; } async openRecentMentions() { diff --git a/e2e-tests/playwright/lib/src/ui/pages/channels.ts b/e2e-tests/playwright/lib/src/ui/pages/channels.ts index 7e900c01eb..27ec4b9419 100644 --- a/e2e-tests/playwright/lib/src/ui/pages/channels.ts +++ b/e2e-tests/playwright/lib/src/ui/pages/channels.ts @@ -36,7 +36,7 @@ export default class ChannelsPage { this.page = page; // The main areas of the app - this.globalHeader = new components.GlobalHeader(page.locator('#global-header')); + this.globalHeader = new components.GlobalHeader(this, page.locator('#global-header')); this.searchPopover = new components.SearchPopover(page.locator('#searchPopover')); this.centerView = new components.ChannelsCenterView(page.getByTestId('channel_view')); this.sidebarLeft = new components.ChannelsSidebarLeft(page.locator('#SidebarContainer')); diff --git a/e2e-tests/playwright/specs/accessibility/channels/theme_settings.spec.ts b/e2e-tests/playwright/specs/accessibility/channels/theme_settings.spec.ts new file mode 100644 index 0000000000..c601e0d0a1 --- /dev/null +++ b/e2e-tests/playwright/specs/accessibility/channels/theme_settings.spec.ts @@ -0,0 +1,119 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import {expect, test} from '@mattermost/playwright-lib'; + +test('Theme settings should be keyboard accessible', async ({axe, pw}) => { + const {user} = await pw.initSetup(); + + // # Log in as a user in new browser context + const {page, channelsPage} = await pw.testBrowser.login(user); + + // # Initialize Axe + const ab = axe.builder(page).disableRules([ + 'color-contrast', + + // Known issue: These fail due to the way we've grouped plugin setting tabs together in the LHS + 'aria-required-children', + 'aria-required-parent', + ]); + + // # Visit default channel page + await channelsPage.goto(); + await channelsPage.toBeVisible(); + + // # Open settings modal + const settingsModal = await channelsPage.globalHeader.openSettings(); + + // * The settings modal should have no accessibility violations + let accessibilityScanResults = await ab.analyze(); + expect(accessibilityScanResults.violations).toHaveLength(0); + + // # Open display tab + await settingsModal.container.focus(); + await page.keyboard.press('Tab'); + await page.keyboard.press('Tab'); + await page.keyboard.press('Tab'); + await page.keyboard.press('ArrowDown'); + + // * The display tab should be open + const {displaySettings} = settingsModal; + await displaySettings.toBeVisible(); + + // * The display tab should have no accessibility violations + accessibilityScanResults = await ab.analyze(); + expect(accessibilityScanResults.violations).toHaveLength(0); + + // # Open the theme section + await page.keyboard.press('Tab'); + await page.keyboard.press('Space'); + + // * The theme section should be open + await displaySettings.verifySectionIsExpanded('theme'); + + // * The Premade Themes option should be focused + await expect(page.getByLabel('Premade Themes')).toBeFocused(); + + // * The theme section for premade themes should have no accessibility violations + accessibilityScanResults = await ab.analyze(); + expect(accessibilityScanResults.violations).toHaveLength(0); + + // * Should be able to tab through the options + await page.keyboard.press('Tab'); + await expect(page.getByRole('button', {name: 'Denim'})).toBeFocused(); + await page.keyboard.press('Tab'); + await expect(page.getByRole('button', {name: 'Sapphire'})).toBeFocused(); + await page.keyboard.press('Tab'); + await expect(page.getByRole('button', {name: 'Quartz'})).toBeFocused(); + await page.keyboard.press('Tab'); + await expect(page.getByRole('button', {name: 'Indigo'})).toBeFocused(); + await page.keyboard.press('Tab'); + await expect(page.getByRole('button', {name: 'Onyx'})).toBeFocused(); + await page.keyboard.press('Tab'); + + // Note: There's no "Apply to all your teams" option because this user is only on one team + await expect(page.getByRole('link', {name: 'See other themes'})).toBeFocused(); + await page.keyboard.press('Tab'); + await expect(page.getByRole('button', {name: 'Save'})).toBeFocused(); + + // # Go back to the premade/custom option + await page.getByLabel('Premade Themes').focus(); + + // # Switch to the custom theme section + await page.keyboard.press('ArrowDown'); + + // * The Custom Theme option should be focused + await expect(page.getByLabel('Custom Theme')).toBeFocused(); + + // * Check the ARIA of the Sidebar Styles section + await page.keyboard.press('Tab'); + const sidebarStyles = page.getByRole('button', {name: 'Sidebar Styles'}); + await expect(sidebarStyles).toBeFocused(); + await expect(sidebarStyles).toHaveAttribute('aria-expanded', 'false'); + + // * Check that we can tab over the collapsed section to the next section and then back again + await page.keyboard.press('Tab'); + await expect(page.getByRole('button', {name: 'Center Channel Styles'})).toBeFocused(); + + await page.keyboard.press('Shift+Tab'); + await expect(sidebarStyles).toBeFocused(); + + // * Check that we can expand the section + await page.keyboard.press('Enter'); + await expect(sidebarStyles).toHaveAttribute('aria-expanded', 'true'); + + // # Wait for the expanding animation to be open + await expect(page.getByLabel('Sidebar Styles')).toHaveCSS('overflow-y', 'visible'); + + // * Check that we can tab through color pickers + await page.keyboard.press('Tab'); + await expect(page.getByLabel('Sidebar BG', {exact: true})).toBeFocused(); + await page.keyboard.press('Tab'); + await expect(page.getByLabel('Sidebar Text', {exact: true})).toBeFocused(); + await page.keyboard.press('Tab'); + await expect(page.getByLabel('Sidebar Header BG')).toBeFocused(); + + // * The theme section for custom themes should have no accessibility violations + accessibilityScanResults = await ab.analyze(); + expect(accessibilityScanResults.violations).toHaveLength(0); +}); diff --git a/e2e-tests/playwright/specs/functional/channels/settings/notifications/highlight_without_notification.spec.ts b/e2e-tests/playwright/specs/functional/channels/settings/notifications/highlight_without_notification.spec.ts index 1d772bbdf5..30c0568afc 100644 --- a/e2e-tests/playwright/specs/functional/channels/settings/notifications/highlight_without_notification.spec.ts +++ b/e2e-tests/playwright/specs/functional/channels/settings/notifications/highlight_without_notification.spec.ts @@ -26,16 +26,15 @@ test('MM-T5465-1 Should add the keyword when enter, comma or tab is pressed on t await channelsPage.centerView.postCreate.postMessage('Hello World'); // # Open settings modal - await channelsPage.globalHeader.openSettings(); - await channelsPage.settingsModal.toBeVisible(); + const settingsModal = await channelsPage.globalHeader.openSettings(); // # Open notifications tab - await channelsPage.settingsModal.openNotificationsTab(); + const notificationsSettings = await settingsModal.openNotificationsTab(); // # Open keywords that get highlighted section - await channelsPage.settingsModal.notificationsSettings.expandSection('keysWithHighlight'); + await notificationsSettings.expandSection('keysWithHighlight'); - const keywordsInput = await channelsPage.settingsModal.notificationsSettings.getKeywordsInput(); + const keywordsInput = await notificationsSettings.getKeywordsInput(); // # Enter keyword 1 await keywordsInput.fill(keywords[0]); @@ -56,7 +55,7 @@ test('MM-T5465-1 Should add the keyword when enter, comma or tab is pressed on t await keywordsInput.press('Enter'); // * Verify that the keywords have been added to the collapsed description - const keysWithHighlightDesc = channelsPage.settingsModal.notificationsSettings.keysWithHighlightDesc; + const keysWithHighlightDesc = notificationsSettings.keysWithHighlightDesc; await keysWithHighlightDesc.waitFor(); for (const keyword of keywords.slice(0, 3)) { expect(await keysWithHighlightDesc).toContainText(keyword); @@ -77,25 +76,24 @@ test('MM-T5465-2 Should highlight the keywords when a message is sent with the k await channelsPage.toBeVisible(); // # Open settings modal - await channelsPage.globalHeader.openSettings(); - await channelsPage.settingsModal.toBeVisible(); + const settingsModal = await channelsPage.globalHeader.openSettings(); // # Open notifications tab - await channelsPage.settingsModal.openNotificationsTab(); + const notificationsSettings = await settingsModal.openNotificationsTab(); // # Open keywords that get highlighted section - await channelsPage.settingsModal.notificationsSettings.expandSection('keysWithHighlight'); + await notificationsSettings.expandSection('keysWithHighlight'); // # Enter the keyword - const keywordsInput = await channelsPage.settingsModal.notificationsSettings.getKeywordsInput(); + const keywordsInput = await notificationsSettings.getKeywordsInput(); await keywordsInput.fill(keywords[3]); await keywordsInput.press('Tab'); // # Save the keyword - await channelsPage.settingsModal.notificationsSettings.save(); + await notificationsSettings.save(); // # Close the settings modal - await channelsPage.settingsModal.closeModal(); + await settingsModal.closeModal(); // # Post a message without the keyword const messageWithoutKeyword = 'This message does not contain the keyword'; @@ -132,25 +130,24 @@ test('MM-T5465-3 Should highlight the keywords when a message is sent with the k await channelsPage.toBeVisible(); // # Open settings modal - await channelsPage.globalHeader.openSettings(); - await channelsPage.settingsModal.toBeVisible(); + const settingsModal = await channelsPage.globalHeader.openSettings(); // # Open notifications tab - await channelsPage.settingsModal.openNotificationsTab(); + const notificationsSettings = await settingsModal.openNotificationsTab(); // # Open keywords that get highlighted section - await channelsPage.settingsModal.notificationsSettings.expandSection('keysWithHighlight'); + await notificationsSettings.expandSection('keysWithHighlight'); // # Enter the keyword - const keywordsInput = await channelsPage.settingsModal.notificationsSettings.getKeywordsInput(); + const keywordsInput = await notificationsSettings.getKeywordsInput(); await keywordsInput.fill(keywords[3]); await keywordsInput.press('Tab'); // # Save the keyword - await channelsPage.settingsModal.notificationsSettings.save(); + await notificationsSettings.save(); // # Close the settings modal - await channelsPage.settingsModal.closeModal(); + await settingsModal.closeModal(); // # Post a message without the keyword const messageWithoutKeyword = 'This message does not contain the keyword'; @@ -189,25 +186,24 @@ test('MM-T5465-4 Highlighted keywords should not appear in the Recent Mentions', await channelsPage.toBeVisible(); // # Open settings modal - await channelsPage.globalHeader.openSettings(); - await channelsPage.settingsModal.toBeVisible(); + const settingsModal = await channelsPage.globalHeader.openSettings(); // # Open notifications tab - await channelsPage.settingsModal.openNotificationsTab(); + const notificationsSettings = await settingsModal.openNotificationsTab(); // # Open keywords that get highlighted section - await channelsPage.settingsModal.notificationsSettings.expandSection('keysWithHighlight'); + await notificationsSettings.expandSection('keysWithHighlight'); // # Enter the keyword - const keywordsInput = await channelsPage.settingsModal.notificationsSettings.getKeywordsInput(); + const keywordsInput = await notificationsSettings.getKeywordsInput(); await keywordsInput.fill(keywords[0]); await keywordsInput.press('Tab'); // # Save the keyword - await channelsPage.settingsModal.notificationsSettings.save(); + await notificationsSettings.save(); // # Close the settings modal - await channelsPage.settingsModal.closeModal(); + await settingsModal.closeModal(); // # Open the recent mentions await channelsPage.globalHeader.openRecentMentions(); @@ -250,25 +246,24 @@ test('MM-T5465-5 Should highlight keywords in message sent from another user', a await channelsPage.toBeVisible(); // # Open settings modal - await channelsPage.globalHeader.openSettings(); - await channelsPage.settingsModal.toBeVisible(); + const settingsModal = await channelsPage.globalHeader.openSettings(); // # Open notifications tab - await channelsPage.settingsModal.openNotificationsTab(); + const notificationsSettings = await settingsModal.openNotificationsTab(); // # Open keywords that get highlighted section - await channelsPage.settingsModal.notificationsSettings.expandSection('keysWithHighlight'); + await notificationsSettings.expandSection('keysWithHighlight'); // # Enter the keyword - const keywordsInput = await channelsPage.settingsModal.notificationsSettings.getKeywordsInput(); + const keywordsInput = await notificationsSettings.getKeywordsInput(); await keywordsInput.fill(keywords[0]); await keywordsInput.press('Tab'); // # Save the keyword - await channelsPage.settingsModal.notificationsSettings.save(); + await notificationsSettings.save(); // # Close the settings modal - await channelsPage.settingsModal.closeModal(); + await settingsModal.closeModal(); // * Verify that the keywords are highlighted in the last message received const lastPostWithHighlight = await channelsPage.getLastPost(); diff --git a/webapp/channels/src/components/user_settings/display/user_settings_theme/color_chooser/__snapshots__/color_chooser.test.tsx.snap b/webapp/channels/src/components/user_settings/display/user_settings_theme/color_chooser/__snapshots__/color_chooser.test.tsx.snap index b7d948e3a4..ca664fc24f 100644 --- a/webapp/channels/src/components/user_settings/display/user_settings_theme/color_chooser/__snapshots__/color_chooser.test.tsx.snap +++ b/webapp/channels/src/components/user_settings/display/user_settings_theme/color_chooser/__snapshots__/color_chooser.test.tsx.snap @@ -4,6 +4,7 @@ exports[`components/user_settings/display/ColorChooser should match, init 1`] = diff --git a/webapp/channels/src/components/user_settings/display/user_settings_theme/color_chooser/color_chooser.tsx b/webapp/channels/src/components/user_settings/display/user_settings_theme/color_chooser/color_chooser.tsx index 5ba5369eec..6dc787ab03 100644 --- a/webapp/channels/src/components/user_settings/display/user_settings_theme/color_chooser/color_chooser.tsx +++ b/webapp/channels/src/components/user_settings/display/user_settings_theme/color_chooser/color_chooser.tsx @@ -19,7 +19,12 @@ export default function ColorChooser(props: Props) { return ( <> - + { if (this.props.allowCustomThemes) { inputs.push(
- -
-
, - ); - } - - if (this.props.allowCustomThemes) { - inputs.push( -
- +
+ +
+
+ +
, ); @@ -271,6 +262,12 @@ export default class ThemeSetting extends React.PureComponent { themeUI = ( + } inputs={
diff --git a/webapp/channels/src/i18n/en.json b/webapp/channels/src/i18n/en.json index 1b5fa601da..1a3422c97f 100644 --- a/webapp/channels/src/i18n/en.json +++ b/webapp/channels/src/i18n/en.json @@ -5698,7 +5698,7 @@ "user.settings.display.theme.customTheme": "Custom Theme", "user.settings.display.theme.describe": "Open to manage your theme", "user.settings.display.theme.otherThemes": "See other themes", - "user.settings.display.theme.themeColors": "Theme Colors", + "user.settings.display.theme.premadeThemes": "Premade Themes", "user.settings.display.theme.title": "Theme", "user.settings.display.timezone": "Timezone", "user.settings.display.title": "Display Settings", diff --git a/webapp/channels/src/sass/routes/_settings.scss b/webapp/channels/src/sass/routes/_settings.scss index bec112465e..d1f9032943 100644 --- a/webapp/channels/src/sass/routes/_settings.scss +++ b/webapp/channels/src/sass/routes/_settings.scss @@ -353,9 +353,11 @@ white-space: nowrap; } - .premadeThemeButton{ + .premadeThemeButton { + padding: 0; border: none; background: none; + text-align: start; } label { @@ -704,6 +706,11 @@ resize: none; } +.user-settings__radio-group-inline { + // Counteract the margin-left added by Bootstrap's radio-inline class + margin-left: -20px; +} + .user-settings__submit-checkbox { padding-top: 0; padding-bottom: 20px;