From 220cd725ccee2b9a8022c2ae2620ca4db011d620 Mon Sep 17 00:00:00 2001 From: Harrison Healey Date: Wed, 8 Apr 2026 10:11:24 -0400 Subject: [PATCH] MM-66887 Fix results in Invite to Team modal (#35936) * Fix a bug in decomposeKorean that caused it to only decompose the first character * MM-66887 Fix Invite To Team modal suggesting users one key press behind * Cherry-pick updates to ime.ts from another branch * And cherry-pick another bit * Remove broken and unnecessary onBlur method from UsersEmailsInput See https://github.com/mattermost/mattermost/pull/35936/changes#r3047500882 for more information --- e2e-tests/playwright/lib/src/ime.ts | 121 +++++++++++------- e2e-tests/playwright/lib/src/index.ts | 2 +- .../search/find_channels_korean.spec.ts | 6 +- .../channels/search/search_box_korean.spec.ts | 4 +- .../invite_people_autocomplete.spec.ts | 105 +++++++++++++++ .../widgets/inputs/users_emails_input.tsx | 15 +-- 6 files changed, 188 insertions(+), 65 deletions(-) create mode 100644 e2e-tests/playwright/specs/functional/channels/team_settings/invite_people_autocomplete.spec.ts diff --git a/e2e-tests/playwright/lib/src/ime.ts b/e2e-tests/playwright/lib/src/ime.ts index 4e58bb60729..668413aabe2 100644 --- a/e2e-tests/playwright/lib/src/ime.ts +++ b/e2e-tests/playwright/lib/src/ime.ts @@ -1,7 +1,7 @@ // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. -import type {Page} from '@playwright/test'; +import type {CDPSession, Page} from '@playwright/test'; /** * This is Korean for "Test if Hangul is typed well". @@ -21,54 +21,81 @@ export const koreanTestPhrase = '한글이 잘 입력되는지 테스트'; * * Note: This only works on Chrome-based browsers because it relies on the Chrome Devtools Protocol (CDP). */ -export async function typeKoreanWithIme(page: Page, text: string) { +export async function typeHangulWithIme(page: Page, text: string) { const client = await page.context().newCDPSession(page); - for (const decomposed of decomposeKorean(text)) { - if (decomposed.jama) { - // # Type the individual jamo + const decomposed = decomposeKorean(text); + for (let i = 0; i < decomposed.length; i++) { + await typeHangulCharacterWithIme(client, decomposed[i], decomposed[i - 1]); + } - // The first one is typed as-is - await client.send('Input.imeSetComposition', { - selectionStart: -1, - selectionEnd: -1, - text: decomposed.jama[0], - }); - - // When you type the second one, the IME combines the two into the resulting character. Instead of reversing - // the math, we can do that by concatenating them and then normalizing the Unicode. - await client.send('Input.imeSetComposition', { - selectionStart: -1, - selectionEnd: -1, - text: (decomposed.jama[0] + decomposed.jama[1]).normalize('NFKD'), - }); - - // For the third one, we can't normalize the Unicode because there are some initial and final jama which - // look identical and normalize to the same value, so just use the original character - await client.send('Input.imeSetComposition', { - selectionStart: -1, - selectionEnd: -1, - text: decomposed.character, - }); - - // # End composition by inserting the complete character into the textbox - // Technically, this doesn't actually happen until the user types something else or clicks on the textbox, - // but it's cleaner to do now since we don't currently support searching for partially composed characters. - await client.send('Input.insertText', { - text: decomposed.character, - }); - } else { - // # Insert the character - await client.send('Input.insertText', { - text: decomposed.character, - }); - } + const lastCharacter = decomposed.at(-1); + if (lastCharacter && 'jamo' in lastCharacter) { + // # End composition by inserting the final character into the textbox + await client.send('Input.insertText', { + text: lastCharacter.character, + }); } await client.detach(); } -function decomposeKorean(text: string): Array<{character: string; jama?: string[]}> { +export async function typeHangulCharacterWithIme( + client: CDPSession, + current: HangleCharacter, + previous: HangleCharacter | undefined, +) { + if (previous && previous.jamo) { + // # End composition of the previous character by inserting it into the textbox + await client.send('Input.insertText', { + text: previous.character, + }); + } + + if (current.jamo) { + // # Type the individual jamo + + // The first one is typed as-is + await client.send('Input.imeSetComposition', { + selectionStart: 0, + selectionEnd: 0, + text: current.jamo[0], + }); + + // When you type the second one, the IME combines the two into the resulting character. Instead of reversing + // the math, we can do that by concatenating them and then normalizing the Unicode. + await client.send('Input.imeSetComposition', { + selectionStart: 0, + selectionEnd: 0, + text: (current.jamo[0] + current.jamo[1]).normalize('NFKD'), + }); + + // For the third one, we can't normalize the Unicode because there are some initial and final jamo which + // look identical and normalize to the same value, so just use the original character + await client.send('Input.imeSetComposition', { + selectionStart: 0, + selectionEnd: 0, + text: current.character, + }); + } else { + // # Insert the character + await client.send('Input.insertText', { + text: current.character, + }); + } +} + +/** + * HangleCharacter is an object containing either: + * - An non-Korean character + * - A Korean character and the array of jamo (syllables) that make up that character + */ +export interface HangleCharacter { + character: string; + jamo?: string[]; +} + +export function decomposeKorean(text: string): Array { // Adapted from https://useless-factor.blogspot.com/2007/08/unicode-implementers-guide-part-3.html and // https://web.archive.org/web/20190512031142/http://www.programminginkorean.com/programming/hangul-in-unicode/composing-syllables-in-unicode/ @@ -156,7 +183,7 @@ function decomposeKorean(text: string): Array<{character: string; jama?: string[ for (let i = 0; i < text.length; i++) { const character = text[i]; - const code = character.charCodeAt(i); + const code = character.charCodeAt(0); if (code >= hangulStart && code <= hangulEnd) { // This is a Hangul character, so we can break it down into the individual constants and vowel @@ -167,13 +194,13 @@ function decomposeKorean(text: string): Array<{character: string; jama?: string[ const medialIndex = Math.floor((syllableIndex % (21 * 28)) / 28); const finalIndex = syllableIndex % 28; - const jama = []; - jama.push(initial[initialIndex]); - jama.push(medial[medialIndex]); + const jamo = []; + jamo.push(initial[initialIndex]); + jamo.push(medial[medialIndex]); if (final[finalIndex]) { - jama.push(final[finalIndex]); + jamo.push(final[finalIndex]); } - result.push({character, jama}); + result.push({character, jamo}); } else { // This is some other character, so just add it separately result.push({character}); diff --git a/e2e-tests/playwright/lib/src/index.ts b/e2e-tests/playwright/lib/src/index.ts index dd1f278c4d4..7b2d7b32493 100644 --- a/e2e-tests/playwright/lib/src/index.ts +++ b/e2e-tests/playwright/lib/src/index.ts @@ -6,7 +6,7 @@ export {testConfig} from './test_config'; export {baseGlobalSetup} from './global_setup'; export {TestBrowser} from './browser_context'; export {getBlobFromAsset, getFileFromAsset} from './file'; -export {koreanTestPhrase, typeKoreanWithIme} from './ime'; +export {decomposeKorean, koreanTestPhrase, typeHangulCharacterWithIme, typeHangulWithIme} from './ime'; export {duration, wait} from './util'; export { diff --git a/e2e-tests/playwright/specs/functional/channels/search/find_channels_korean.spec.ts b/e2e-tests/playwright/specs/functional/channels/search/find_channels_korean.spec.ts index 7990282d288..12c2fa238cd 100644 --- a/e2e-tests/playwright/specs/functional/channels/search/find_channels_korean.spec.ts +++ b/e2e-tests/playwright/specs/functional/channels/search/find_channels_korean.spec.ts @@ -1,7 +1,7 @@ // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. -import {expect, koreanTestPhrase, test, typeKoreanWithIme} from '@mattermost/playwright-lib'; +import {expect, koreanTestPhrase, test, typeHangulWithIme} from '@mattermost/playwright-lib'; test('Find Channels modal handles Korean IME input correctly', async ({pw, browserName}) => { test.skip(browserName !== 'chromium', 'The API used to test this is only available in Chrome'); @@ -42,7 +42,7 @@ test('Find Channels modal handles Korean IME input correctly', async ({pw, brows const secondHalf = koreanTestPhrase.substring(5); // # Type the first half of the test phrase - await typeKoreanWithIme(page, firstHalf); + await typeHangulWithIme(page, firstHalf); // * Verify that characters are correctly composed and weren't doubled up await expect(input).toHaveValue(firstHalf); @@ -52,7 +52,7 @@ test('Find Channels modal handles Korean IME input correctly', async ({pw, brows await expect(page.getByRole('option', {name: partialMatchChannel.display_name, exact: true})).toBeVisible(); // # Type the second half of the test phrase - await typeKoreanWithIme(page, secondHalf); + await typeHangulWithIme(page, secondHalf); // * Verify that characters are correctly composed and weren't doubled up await expect(input).toHaveValue(koreanTestPhrase); diff --git a/e2e-tests/playwright/specs/functional/channels/search/search_box_korean.spec.ts b/e2e-tests/playwright/specs/functional/channels/search/search_box_korean.spec.ts index 1f7ada0987b..7498ffa4e05 100644 --- a/e2e-tests/playwright/specs/functional/channels/search/search_box_korean.spec.ts +++ b/e2e-tests/playwright/specs/functional/channels/search/search_box_korean.spec.ts @@ -1,7 +1,7 @@ // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. -import {expect, koreanTestPhrase, test, typeKoreanWithIme} from '@mattermost/playwright-lib'; +import {expect, koreanTestPhrase, test, typeHangulWithIme} from '@mattermost/playwright-lib'; test('Search box handles Korean IME correctly', async ({pw, browserName}) => { test.skip(browserName !== 'chromium', 'The API used to test this is only available in Chrome'); @@ -31,7 +31,7 @@ test('Search box handles Korean IME correctly', async ({pw, browserName}) => { // # Type into the textbox const searchText = 'in:' + koreanTestPhrase.substring(0, 3); - await typeKoreanWithIme(page, searchText); + await typeHangulWithIme(page, searchText); // * Verify that the text was typed correctly into the search box await expect(searchInput).toHaveValue(searchText); diff --git a/e2e-tests/playwright/specs/functional/channels/team_settings/invite_people_autocomplete.spec.ts b/e2e-tests/playwright/specs/functional/channels/team_settings/invite_people_autocomplete.spec.ts new file mode 100644 index 00000000000..0a529f70db5 --- /dev/null +++ b/e2e-tests/playwright/specs/functional/channels/team_settings/invite_people_autocomplete.spec.ts @@ -0,0 +1,105 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import {decomposeKorean, expect, koreanTestPhrase, test, typeHangulCharacterWithIme} from '@mattermost/playwright-lib'; + +test('MM-66937 Invite modal results match the current input state', async ({pw}) => { + const {adminUser, adminClient, team} = await pw.initSetup(); + + // # Create two users not on the team whose usernames share a prefix but differ at the end + const randomPrefix = await pw.random.id(8); + const user1 = await adminClient.createUser(await pw.random.user(randomPrefix + 'a'), '', ''); + const user2 = await adminClient.createUser(await pw.random.user(randomPrefix + 'b'), '', ''); + + // # Log in as admin + const {channelsPage} = await pw.testBrowser.login(adminUser); + await channelsPage.goto(team.name, 'town-square'); + await channelsPage.toBeVisible(); + + // # Open the Invite People modal + await channelsPage.sidebarLeft.teamMenuButton.click(); + await channelsPage.teamMenu.toBeVisible(); + await channelsPage.teamMenu.clickInvitePeople(); + const inviteModal = await channelsPage.getInvitePeopleModal(team.display_name); + await inviteModal.toBeVisible(); + + // # Type the prefix to filter out all users + await inviteModal.inviteInput.pressSequentially(randomPrefix); + + // * Verify that both users appear in the results initially + const listbox = inviteModal.container.getByRole('listbox'); + await expect(listbox.getByRole('option')).toHaveCount(2); + await expect(listbox.getByRole('option', {name: `@${user1.username}`})).toBeVisible(); + await expect(listbox.getByRole('option', {name: `@${user2.username}`})).toBeVisible(); + + // # Type an 'a' to filter the results + await inviteModal.inviteInput.press('a'); + + // * Verify that only user1 is now listed + await expect(listbox.getByRole('option')).toHaveCount(1); + await expect(listbox.getByRole('option', {name: `@${user1.username}`})).toBeVisible(); + await expect(listbox.getByRole('option', {name: `@${user2.username}`})).not.toBeAttached(); + + // # Backspace that 'a' + await inviteModal.inviteInput.press('Backspace'); + + // * Verify that both users are listed again + await expect(listbox.getByRole('option')).toHaveCount(2); + await expect(listbox.getByRole('option', {name: `@${user1.username}`})).toBeVisible(); + await expect(listbox.getByRole('option', {name: `@${user2.username}`})).toBeVisible(); + + // # Type a 'b' to filter the results + await inviteModal.inviteInput.press('b'); + + // * Verify that only user2 is now listed + await expect(listbox.getByRole('option')).toHaveCount(1); + await expect(listbox.getByRole('option', {name: `@${user1.username}`})).not.toBeAttached(); + await expect(listbox.getByRole('option', {name: `@${user2.username}`})).toBeVisible(); +}); + +test('MM-66937 Invite modal results match the current input state when typing in Korean', async ({browserName, pw}) => { + test.skip(browserName !== 'chromium', 'The API used to test this is only available in Chrome'); + + const {adminUser, adminClient, team} = await pw.initSetup(); + + // # Create a users with a Korean name (plus a prefix to avoid interfering test runs) + const randomPrefix = await pw.random.id(8); + const user = await adminClient.createUser( + { + ...(await pw.random.user(randomPrefix + 'a')), + first_name: randomPrefix + koreanTestPhrase, + }, + '', + '', + ); + + // # Log in as admin + const {channelsPage, page} = await pw.testBrowser.login(adminUser); + await channelsPage.goto(team.name, 'town-square'); + await channelsPage.toBeVisible(); + + // # Open the Invite People modal + await channelsPage.sidebarLeft.teamMenuButton.click(); + await channelsPage.teamMenu.toBeVisible(); + await channelsPage.teamMenu.clickInvitePeople(); + const inviteModal = await channelsPage.getInvitePeopleModal(team.display_name); + await inviteModal.toBeVisible(); + + // # Type the prefix to filter out other users + await inviteModal.inviteInput.pressSequentially(randomPrefix); + + // * Verify that the user appears in the results initially + const listbox = inviteModal.container.getByRole('listbox'); + await expect(listbox.getByRole('option')).toHaveCount(1); + await expect(listbox.getByRole('option', {name: `@${user.username}`})).toBeVisible(); + + // # Type all 3 keys that form a single Hangul character + const client = await page.context().newCDPSession(page); + await typeHangulCharacterWithIme(client, decomposeKorean(koreanTestPhrase)[0], undefined); + + // * Verify that the user is still listed + await expect(listbox.getByRole('option')).toHaveCount(1); + await expect(listbox.getByRole('option', {name: `@${user.username}`})).toBeVisible(); + + await client.detach(); +}); diff --git a/webapp/channels/src/components/widgets/inputs/users_emails_input.tsx b/webapp/channels/src/components/widgets/inputs/users_emails_input.tsx index 07d3bc7e885..59c522bd47e 100644 --- a/webapp/channels/src/components/widgets/inputs/users_emails_input.tsx +++ b/webapp/channels/src/components/widgets/inputs/users_emails_input.tsx @@ -34,7 +34,6 @@ type Props = { ariaLabel: string; usersLoader: (search: string, callback: (users: UserProfile[]) => void) => Promise | undefined; onUsersLoad?: (users: UserProfile[]) => void; - onBlur?: () => void; onChange: (change: Array) => void; showError?: boolean; errorMessage?: MessageDescriptor; @@ -361,7 +360,7 @@ export class UsersEmailsInput extends React.PureComponent { }); }; - optionsLoader = (_input: string, callback: (options: UserProfile[]) => void) => { + optionsLoader = (inputValue: string, callback: (options: UserProfile[]) => void) => { const customCallback = (options: UserProfile[]) => { this.setState({options}); const accessibleProfiles = options.map((user: UserProfile) => ({...user, label: user.username})); @@ -370,7 +369,7 @@ export class UsersEmailsInput extends React.PureComponent { this.props.onUsersLoad(options); } }; - const result = this.props.usersLoader(this.props.inputValue, customCallback); + const result = this.props.usersLoader(inputValue, customCallback); if (result && result.then) { result.then(customCallback); } @@ -384,13 +383,6 @@ export class UsersEmailsInput extends React.PureComponent { this.selectRef.current?.onInputChange(this.props.inputValue, {action: 'set-value', prevInputValue: this.props.inputValue}); }; - onBlur = () => { - this.selectRef.current?.onInputChange(this.props.inputValue, {action: 'input-blur', prevInputValue: this.state.prevValue}); - if (this.props.onBlur) { - this.props.onBlur(); - } - }; - appendDelimitedValues = async (values: string, delimiter: RegExp = pasteDelimiter): Promise => { const existingValues = this.formatValuesForCreatable(); const entries = [...new Set(values.split(delimiter).map((e) => e.trim()))]; @@ -560,8 +552,7 @@ export class UsersEmailsInput extends React.PureComponent { onInputChange={this.handleInputChange} inputValue={this.props.inputValue} openMenuOnFocus={true} - onFocus={() => this.onFocus} - onBlur={() => this.onBlur} + onFocus={this.onFocus} tabSelectsValue={true} value={values} aria-label={this.props.ariaLabel}