Skip to content

Commit a302e05

Browse files
hejsztynxkacperzolkiewskiszydlovsky
authored
fix: various mentions bugs and inconsistencies between platforms (#637)
# Summary There were various bugs and inconsistencies with mentions functionalities across every platform. Mainly it wasn't handled properly when you switched **directly** between different mentions, which caused improper event emission and the lists stayed visible - documented in the videos section iOS: - events not emitted when switching directly between mentions - duplicated `onChange` events when just moving a cursor across a mention - `_recentInputString` was not updated if no `onChangeText` prop was given Android: - events not emitted when switching directly between mentions - on selection changes, the mentions were not reanalyzed and no events were emitted - when `setMention` a duplicated space was inserted if there already was one - `setMention` wasn't replacing the surrounding text - after `setMention` the selection wasn't moved to the trailing space Web: - refactor on the event subscription hook for consistency - mention query was evaluated to a cursor position, not the whole written mention, which produced leftover text when inserting a mention - unnecessarily considered a finalized mention into the query - events not emitted when switching directly between mentions - when `setMention` a duplicated space was inserted if there already was one Example apps: - updated example apps with more idiomatic code. Earlier `onChange` mention event would handle popup list opening - something what `onStart` is for ## Test Plan Follow the text written in the attached videos, play around with selection changes, all events should emit correctly and the lists should function as expected. ## Screenshots / Videos (all platforms) events not emitting when switching between mentions https://github.com/user-attachments/assets/cc408357-ae3f-41ed-87ce-c6192149539b (web) incorrect query evaluation and inserting, doubled space https://github.com/user-attachments/assets/4981556a-c8c6-4139-855e-957fa9dc4754 (android) `setMention` not replacing surrounding text: https://github.com/user-attachments/assets/37fb805c-aaa1-4947-a8dd-4d1427ab1629 ## Compatibility | OS | Implemented | | ------- | :---------: | | iOS | ✅ | | Android | ✅ | | Web | ✅ | ## Checklist - [x] E2E tests are passing - [x] Required E2E tests have been added (if applicable) --------- Co-authored-by: Kacper Żółkiewski <74975508+kacperzolkiewski@users.noreply.github.com> Co-authored-by: Mikołaj Szydłowski <9szydlowski9@gmail.com>
1 parent 3a5d492 commit a302e05

26 files changed

Lines changed: 430 additions & 180 deletions
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
appId: swmansion.enriched.example
2+
---
3+
# fix PR #637 - mention popups not closing when switching between mentions
4+
5+
- launchApp
6+
7+
- tapOn:
8+
id: "toggle-screen-button"
9+
10+
- tapOn:
11+
id: "editor-input"
12+
13+
- inputText: "mentions #gen @J"
14+
15+
# user popup visible
16+
- runFlow:
17+
file: "../subflows/capture_or_assert_fullscreen_screenshot.yaml"
18+
env:
19+
SCREENSHOT_NAME: "mention_popup_closing_on_cursor_travel_1"
20+
21+
- tapOn:
22+
id: "focus-button"
23+
24+
- tapOn:
25+
id: "editor-input"
26+
point: "30%, 50%"
27+
28+
# channel popup visible
29+
- runFlow:
30+
file: "../subflows/capture_or_assert_fullscreen_screenshot.yaml"
31+
env:
32+
SCREENSHOT_NAME: "mention_popup_closing_on_cursor_travel_2"
33+
34+
- tapOn:
35+
id: "focus-button"
36+
37+
- tapOn:
38+
id: "editor-input"
39+
point: "10%, 50%"
40+
41+
# no popup visible
42+
- runFlow:
43+
file: "../subflows/capture_or_assert_fullscreen_screenshot.yaml"
44+
env:
45+
SCREENSHOT_NAME: "mention_popup_closing_on_cursor_travel_3"
75.3 KB
Loading
69.8 KB
Loading
63.9 KB
Loading
89.9 KB
Loading
82.1 KB
Loading
74.3 KB
Loading
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
appId: swmansion.enriched.example
2+
---
3+
- tapOn:
4+
id: 'blur-button'
5+
6+
- runFlow:
7+
file: '../../subflows/capture_or_assert_screenshot.yaml'
8+
env:
9+
ELEMENT_ID: 'full-screen'
10+
SCREENSHOT_PREFIX: 'enrichedInput'

.playwright/tests/mentions.spec.ts

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ const sel = {
1212
eventType: '[data-testid="mention-event-type"]',
1313
eventIndicator: '[data-testid="mention-event-indicator"]',
1414
eventText: '[data-testid="mention-event-text"]',
15+
lastEndEvent: '[data-testid="mention-last-end-event"]',
1516
htmlOutput: '[data-testid="mention-html-output"]',
1617
detectedCount: '[data-testid="mention-detected-count"]',
1718
detectedText: '[data-testid="mention-detected-text"]',
@@ -41,6 +42,9 @@ function eventIndicator(page: Page) {
4142
function eventText(page: Page) {
4243
return page.locator(sel.eventText);
4344
}
45+
function lastEndEvent(page: Page) {
46+
return page.locator(sel.lastEndEvent);
47+
}
4448
function htmlOutput(page: Page) {
4549
return page.locator(sel.htmlOutput);
4650
}
@@ -263,3 +267,49 @@ test('mention renders correctly', async ({ page }) => {
263267
);
264268
await expect(editorLocator(page)).toHaveScreenshot('mention-visual.png');
265269
});
270+
271+
test('switching to a different mention starts it and ends the previous one', async ({
272+
page,
273+
}) => {
274+
await gotoMentionTest(page);
275+
const editor = mentionEditor(page);
276+
await editor.click();
277+
await editor.pressSequentially('foo #g ', { delay: 80 });
278+
await expect(eventType(page)).toHaveText('change');
279+
await expect(eventIndicator(page)).toHaveText('#');
280+
await editor.pressSequentially('@', { delay: 80 });
281+
await expect(eventType(page)).toHaveText('start');
282+
await expect(eventIndicator(page)).toHaveText('@');
283+
await expect(lastEndEvent(page)).toHaveText('#');
284+
await editor.press('ArrowLeft');
285+
await editor.press('ArrowLeft'); // back to the '#' mention
286+
await expect(eventType(page)).toHaveText('change');
287+
await expect(eventIndicator(page)).toHaveText('#');
288+
await expect(lastEndEvent(page)).toHaveText('@');
289+
await editor.press('ArrowLeft');
290+
await editor.press('ArrowLeft');
291+
await editor.press('ArrowLeft'); // leaving the '#' mention
292+
await expect(eventType(page)).toHaveText('end');
293+
await expect(eventIndicator(page)).toHaveText('#');
294+
});
295+
296+
test("inserting a mention between text doesn't produce a double space", async ({
297+
page,
298+
}) => {
299+
await gotoMentionTest(page);
300+
const editor = mentionEditor(page);
301+
await editor.click();
302+
await editor.pressSequentially('example ', { delay: 80 });
303+
await editor.pressSequentially(' test', { delay: 80 });
304+
for (let i = 0; i < 5; i++) {
305+
await editor.press('ArrowLeft');
306+
}
307+
await editor.press('@');
308+
await page.locator(sel.setUserButton).click();
309+
await page.waitForTimeout(2000);
310+
await expect
311+
.poll(async () => await htmlOutput(page).textContent())
312+
.toEqual(
313+
'<html><p>example <mention text="Jane" indicator="@" id="1">Jane</mention> test</p></html>'
314+
);
315+
});

android/src/main/java/com/swmansion/enriched/textinput/events/MentionHandler.kt

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,17 +27,27 @@ class MentionHandler(
2727
indicator: String,
2828
text: String?,
2929
) {
30+
// switching directly to an active mention
31+
if (previousIndicator != indicator) {
32+
endMention()
33+
34+
// explicit startMention event before changeMention event
35+
if (!text.isNullOrEmpty()) {
36+
emitEvent(indicator, "")
37+
}
38+
}
39+
3040
emitEvent(indicator, text)
31-
previousIndicator = indicator
3241
}
3342

3443
private fun emitEvent(
3544
indicator: String,
3645
text: String?,
3746
) {
3847
// Do not emit events too often
39-
if (previousText == text) return
48+
if (previousIndicator == indicator && previousText == text) return
4049

50+
previousIndicator = indicator
4151
previousText = text
4252
val context = view.context as ReactContext
4353
val surfaceId = UIManagerHelper.getSurfaceId(context)

0 commit comments

Comments
 (0)