From 6ef5bf8543cdaabfb671eb2ba4bff1c5d8a09aca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Musia=C5=82?= Date: Fri, 23 May 2025 17:40:42 +0200 Subject: [PATCH 1/4] fix firefox arrow navigation bug --- .gitignore | 3 + src/MarkdownTextInput.web.tsx | 11 +++ src/web/utils/__tests__/firefoxUtils.test.ts | 72 ++++++++++++++++++++ src/web/utils/firefoxUtils.ts | 26 +++++++ 4 files changed, 112 insertions(+) create mode 100644 src/web/utils/__tests__/firefoxUtils.test.ts create mode 100644 src/web/utils/firefoxUtils.ts diff --git a/.gitignore b/.gitignore index d6cadc5d0..b0b314aae 100644 --- a/.gitignore +++ b/.gitignore @@ -79,3 +79,6 @@ lib/ # react-native-live-markdown .build_complete + +# jest +coverage/ diff --git a/src/MarkdownTextInput.web.tsx b/src/MarkdownTextInput.web.tsx index a93833a84..3109b79a4 100644 --- a/src/MarkdownTextInput.web.tsx +++ b/src/MarkdownTextInput.web.tsx @@ -24,6 +24,8 @@ import {getElementHeight, getPlaceholderValue, isEventComposing, normalizeValue, import {idGenerator, parseToReactDOMStyle, processMarkdownStyle} from './web/utils/webStyleUtils'; import {forceRefreshAllImages} from './web/inputElements/inlineImage'; import type {MarkdownRange, InlineImagesInputProps} from './commonTypes'; +import BrowserUtils from './web/utils/browserUtils'; +import {handleFirefoxRightArrowKeyNavigation} from './web/utils/firefoxUtils'; const useClientEffect = typeof window === 'undefined' ? useEffect : useLayoutEffect; @@ -526,6 +528,15 @@ const MarkdownTextInput = React.forwardRef { + const div = document.createElement('div') as unknown as MarkdownTextInputElement; + div.value = value; + return div; +}; + +jest.mock('../cursorUtils', () => ({ + ...jest.requireActual('../cursorUtils'), + getCurrentCursorPosition: jest.fn(), + setCursorPosition: jest.fn(), +})); + +describe('handleFirefoxArrowKeyNavigation', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + it('should do nothing if no cursor in target', () => { + const target = createMockTarget('test'); + (CursorUtils.getCurrentCursorPosition as jest.Mock).mockReturnValue(null); + + handleFirefoxRightArrowKeyNavigation(target); + expect(CursorUtils.setCursorPosition).not.toHaveBeenCalled(); + }); + + it('should move cursor to next grapheme boundary with regular text', () => { + const target = createMockTarget('hello world'); + (CursorUtils.getCurrentCursorPosition as jest.Mock).mockReturnValue({start: 5, end: 5}); + + handleFirefoxRightArrowKeyNavigation(target); + expect(CursorUtils.setCursorPosition).toHaveBeenCalledWith(target, 6, 6); + }); + + it('should move cursor correctly when inside emoji', () => { + const target = createMockTarget('๐Ÿ˜€text'); + (CursorUtils.getCurrentCursorPosition as jest.Mock).mockReturnValue({start: 1, end: 1}); + + handleFirefoxRightArrowKeyNavigation(target); + expect(CursorUtils.setCursorPosition).toHaveBeenCalledWith(target, 2, 2); + }); + + it('should not move cursor beyond text length', () => { + const target = createMockTarget('test'); + + (CursorUtils.getCurrentCursorPosition as jest.Mock).mockReturnValue({start: 4, end: 4}); + handleFirefoxRightArrowKeyNavigation(target); + expect(CursorUtils.setCursorPosition).toHaveBeenCalledWith(target, 4, 4); + }); + + it('should handle multiple emojis correctly', () => { + const target = createMockTarget('๐Ÿ˜€๐Ÿ˜€text'); + (CursorUtils.getCurrentCursorPosition as jest.Mock).mockReturnValueOnce({start: 0, end: 0}).mockReturnValueOnce({start: 2, end: 2}); + + handleFirefoxRightArrowKeyNavigation(target); + expect(CursorUtils.setCursorPosition).toHaveBeenCalledWith(target, 2, 2); + handleFirefoxRightArrowKeyNavigation(target); + expect(CursorUtils.setCursorPosition).toHaveBeenCalledWith(target, 4, 4); + }); + + it('should handle emoji selection', () => { + const target = createMockTarget('๐Ÿ˜€๐Ÿ˜€text'); + (CursorUtils.getCurrentCursorPosition as jest.Mock).mockReturnValueOnce({start: 0, end: 0}).mockReturnValueOnce({start: 2, end: 2}); + + handleFirefoxRightArrowKeyNavigation(target, true); + expect(CursorUtils.setCursorPosition).toHaveBeenCalledWith(target, 0, 2); + handleFirefoxRightArrowKeyNavigation(target); + expect(CursorUtils.setCursorPosition).toHaveBeenCalledWith(target, 4, 4); + }); +}); diff --git a/src/web/utils/firefoxUtils.ts b/src/web/utils/firefoxUtils.ts new file mode 100644 index 000000000..07ad68266 --- /dev/null +++ b/src/web/utils/firefoxUtils.ts @@ -0,0 +1,26 @@ +import type {MarkdownTextInputElement} from '../../MarkdownTextInput.web'; +import {getCurrentCursorPosition, setCursorPosition} from './cursorUtils'; + +function handleFirefoxRightArrowKeyNavigation(target: MarkdownTextInputElement, isSelectionEvent = false): void { + const currentSelection = getCurrentCursorPosition(target); + if (!currentSelection) { + return; + } + + const text = target.value; + const cursorPos = currentSelection.end; + + const segmenter = new Intl.Segmenter('en', {granularity: 'grapheme'}); + const graphemes = Array.from(segmenter.segment(text)); + + const nextGrapheme = graphemes.find(({index, segment}) => { + const segmentEnd = index + segment.length; + return cursorPos < segmentEnd; + }); + + const newCursorPos = nextGrapheme ? nextGrapheme.index + nextGrapheme.segment.length : text.length; + + setCursorPosition(target, isSelectionEvent ? currentSelection.start : newCursorPos, newCursorPos); +} +// eslint-disable-next-line import/prefer-default-export +export {handleFirefoxRightArrowKeyNavigation}; From 2bef014977539b11ff0513b5b7e602f059a633ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Musia=C5=82?= Date: Tue, 27 May 2025 16:06:43 +0200 Subject: [PATCH 2/4] code style changes --- src/MarkdownTextInput.web.tsx | 4 ++-- src/web/utils/__tests__/firefoxUtils.test.ts | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/MarkdownTextInput.web.tsx b/src/MarkdownTextInput.web.tsx index 3109b79a4..7a2b25d3d 100644 --- a/src/MarkdownTextInput.web.tsx +++ b/src/MarkdownTextInput.web.tsx @@ -528,8 +528,8 @@ const MarkdownTextInput = React.forwardRef { beforeEach(() => { jest.clearAllMocks(); }); + it('should do nothing if no cursor in target', () => { const target = createMockTarget('test'); (CursorUtils.getCurrentCursorPosition as jest.Mock).mockReturnValue(null); From bd2f992acdc22d7a92eb78eed94ef6b530b38a16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Musia=C5=82?= Date: Tue, 27 May 2025 16:36:49 +0200 Subject: [PATCH 3/4] add handling of left arrow navigation --- src/MarkdownTextInput.web.tsx | 8 ++-- src/web/utils/__tests__/firefoxUtils.test.ts | 39 +++++++++++++++----- src/web/utils/firefoxUtils.ts | 32 +++++++++++----- 3 files changed, 56 insertions(+), 23 deletions(-) diff --git a/src/MarkdownTextInput.web.tsx b/src/MarkdownTextInput.web.tsx index 7a2b25d3d..300c2950f 100644 --- a/src/MarkdownTextInput.web.tsx +++ b/src/MarkdownTextInput.web.tsx @@ -25,7 +25,7 @@ import {idGenerator, parseToReactDOMStyle, processMarkdownStyle} from './web/uti import {forceRefreshAllImages} from './web/inputElements/inlineImage'; import type {MarkdownRange, InlineImagesInputProps} from './commonTypes'; import BrowserUtils from './web/utils/browserUtils'; -import {handleFirefoxRightArrowKeyNavigation} from './web/utils/firefoxUtils'; +import {handleFirefoxArrowKeyNavigation} from './web/utils/firefoxUtils'; const useClientEffect = typeof window === 'undefined' ? useEffect : useLayoutEffect; @@ -528,13 +528,13 @@ const MarkdownTextInput = React.forwardRef { const div = document.createElement('div') as unknown as MarkdownTextInputElement; @@ -23,7 +23,7 @@ describe('handleFirefoxArrowKeyNavigation', () => { const target = createMockTarget('test'); (CursorUtils.getCurrentCursorPosition as jest.Mock).mockReturnValue(null); - handleFirefoxRightArrowKeyNavigation(target); + handleFirefoxArrowKeyNavigation(target); expect(CursorUtils.setCursorPosition).not.toHaveBeenCalled(); }); @@ -31,7 +31,7 @@ describe('handleFirefoxArrowKeyNavigation', () => { const target = createMockTarget('hello world'); (CursorUtils.getCurrentCursorPosition as jest.Mock).mockReturnValue({start: 5, end: 5}); - handleFirefoxRightArrowKeyNavigation(target); + handleFirefoxArrowKeyNavigation(target); expect(CursorUtils.setCursorPosition).toHaveBeenCalledWith(target, 6, 6); }); @@ -39,7 +39,7 @@ describe('handleFirefoxArrowKeyNavigation', () => { const target = createMockTarget('๐Ÿ˜€text'); (CursorUtils.getCurrentCursorPosition as jest.Mock).mockReturnValue({start: 1, end: 1}); - handleFirefoxRightArrowKeyNavigation(target); + handleFirefoxArrowKeyNavigation(target); expect(CursorUtils.setCursorPosition).toHaveBeenCalledWith(target, 2, 2); }); @@ -47,27 +47,46 @@ describe('handleFirefoxArrowKeyNavigation', () => { const target = createMockTarget('test'); (CursorUtils.getCurrentCursorPosition as jest.Mock).mockReturnValue({start: 4, end: 4}); - handleFirefoxRightArrowKeyNavigation(target); + handleFirefoxArrowKeyNavigation(target); expect(CursorUtils.setCursorPosition).toHaveBeenCalledWith(target, 4, 4); }); - it('should handle multiple emojis correctly', () => { + it('should handle multiple emojis', () => { const target = createMockTarget('๐Ÿ˜€๐Ÿ˜€text'); (CursorUtils.getCurrentCursorPosition as jest.Mock).mockReturnValueOnce({start: 0, end: 0}).mockReturnValueOnce({start: 2, end: 2}); - handleFirefoxRightArrowKeyNavigation(target); + handleFirefoxArrowKeyNavigation(target); expect(CursorUtils.setCursorPosition).toHaveBeenCalledWith(target, 2, 2); - handleFirefoxRightArrowKeyNavigation(target); + handleFirefoxArrowKeyNavigation(target); expect(CursorUtils.setCursorPosition).toHaveBeenCalledWith(target, 4, 4); }); + it('should handle multiple emojis backward navigation', () => { + const target = createMockTarget('๐Ÿ˜€๐Ÿ˜€text'); + (CursorUtils.getCurrentCursorPosition as jest.Mock).mockReturnValueOnce({start: 4, end: 4}).mockReturnValueOnce({start: 2, end: 2}); + + handleFirefoxArrowKeyNavigation(target, false, 'left'); + expect(CursorUtils.setCursorPosition).toHaveBeenCalledWith(target, 2, 2); + handleFirefoxArrowKeyNavigation(target, false, 'left'); + expect(CursorUtils.setCursorPosition).toHaveBeenCalledWith(target, 2, 2); + }); + it('should handle emoji selection', () => { const target = createMockTarget('๐Ÿ˜€๐Ÿ˜€text'); (CursorUtils.getCurrentCursorPosition as jest.Mock).mockReturnValueOnce({start: 0, end: 0}).mockReturnValueOnce({start: 2, end: 2}); - handleFirefoxRightArrowKeyNavigation(target, true); + handleFirefoxArrowKeyNavigation(target, true); expect(CursorUtils.setCursorPosition).toHaveBeenCalledWith(target, 0, 2); - handleFirefoxRightArrowKeyNavigation(target); + handleFirefoxArrowKeyNavigation(target); expect(CursorUtils.setCursorPosition).toHaveBeenCalledWith(target, 4, 4); }); + it('should handle emoji selection backwards', () => { + const target = createMockTarget('๐Ÿ˜€๐Ÿ˜€text'); + (CursorUtils.getCurrentCursorPosition as jest.Mock).mockReturnValueOnce({start: 4, end: 4}).mockReturnValueOnce({start: 2, end: 4}); + + handleFirefoxArrowKeyNavigation(target, true, 'left'); + expect(CursorUtils.setCursorPosition).toHaveBeenCalledWith(target, 2, 4); + handleFirefoxArrowKeyNavigation(target, true, 'left'); + expect(CursorUtils.setCursorPosition).toHaveBeenCalledWith(target, 0, 4); + }); }); diff --git a/src/web/utils/firefoxUtils.ts b/src/web/utils/firefoxUtils.ts index 07ad68266..109ac4ed5 100644 --- a/src/web/utils/firefoxUtils.ts +++ b/src/web/utils/firefoxUtils.ts @@ -1,26 +1,40 @@ import type {MarkdownTextInputElement} from '../../MarkdownTextInput.web'; import {getCurrentCursorPosition, setCursorPosition} from './cursorUtils'; -function handleFirefoxRightArrowKeyNavigation(target: MarkdownTextInputElement, isSelectionEvent = false): void { +/** + * Ensures consistent arrow navigation across grapheme clusters (like emojis) + */ +function handleFirefoxArrowKeyNavigation(target: MarkdownTextInputElement, isSelectionEvent = false, direction: 'right' | 'left' = 'right'): void { const currentSelection = getCurrentCursorPosition(target); if (!currentSelection) { return; } const text = target.value; - const cursorPos = currentSelection.end; const segmenter = new Intl.Segmenter('en', {granularity: 'grapheme'}); const graphemes = Array.from(segmenter.segment(text)); - const nextGrapheme = graphemes.find(({index, segment}) => { - const segmentEnd = index + segment.length; - return cursorPos < segmentEnd; - }); + if (direction === 'right') { + const cursorPos = currentSelection.end; + const nextGrapheme = graphemes.find(({index, segment}) => { + const segmentEnd = index + segment.length; + return cursorPos < segmentEnd; + }); - const newCursorPos = nextGrapheme ? nextGrapheme.index + nextGrapheme.segment.length : text.length; + const newCursorPos = nextGrapheme ? nextGrapheme.index + nextGrapheme.segment.length : text.length; + setCursorPosition(target, isSelectionEvent ? currentSelection.start : newCursorPos, newCursorPos); + } else { + const cursorPos = currentSelection.start; + const prevGrapheme = graphemes.findLast(({index, segment}) => { + const segmentEnd = index + segment.length; + return segmentEnd < cursorPos; + }); - setCursorPosition(target, isSelectionEvent ? currentSelection.start : newCursorPos, newCursorPos); + const newCursorPos = prevGrapheme ? prevGrapheme.index + prevGrapheme.segment.length : 0; + setCursorPosition(target, newCursorPos, isSelectionEvent ? currentSelection.end : newCursorPos); + } } + // eslint-disable-next-line import/prefer-default-export -export {handleFirefoxRightArrowKeyNavigation}; +export {handleFirefoxArrowKeyNavigation}; From 0b732d81800ab17644f7b073c910b42b2aab2fc6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Musia=C5=82?= Date: Thu, 5 Jun 2025 12:42:41 +0200 Subject: [PATCH 4/4] optimize implementation of firefox arrow nav --- src/web/utils/firefoxUtils.ts | 36 +++++++++++++++++++++-------------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/src/web/utils/firefoxUtils.ts b/src/web/utils/firefoxUtils.ts index 109ac4ed5..9ae63ce96 100644 --- a/src/web/utils/firefoxUtils.ts +++ b/src/web/utils/firefoxUtils.ts @@ -13,27 +13,35 @@ function handleFirefoxArrowKeyNavigation(target: MarkdownTextInputElement, isSel const text = target.value; const segmenter = new Intl.Segmenter('en', {granularity: 'grapheme'}); - const graphemes = Array.from(segmenter.segment(text)); + const segments = segmenter.segment(text); if (direction === 'right') { const cursorPos = currentSelection.end; - const nextGrapheme = graphemes.find(({index, segment}) => { - const segmentEnd = index + segment.length; - return cursorPos < segmentEnd; - }); + let newCursorPos = text.length; - const newCursorPos = nextGrapheme ? nextGrapheme.index + nextGrapheme.segment.length : text.length; - setCursorPosition(target, isSelectionEvent ? currentSelection.start : newCursorPos, newCursorPos); - } else { - const cursorPos = currentSelection.start; - const prevGrapheme = graphemes.findLast(({index, segment}) => { + // eslint-disable-next-line no-restricted-syntax + for (const {index, segment} of segments) { const segmentEnd = index + segment.length; - return segmentEnd < cursorPos; - }); + if (cursorPos < segmentEnd) { + newCursorPos = segmentEnd; + break; + } + } + setCursorPosition(target, isSelectionEvent ? currentSelection.start : newCursorPos, newCursorPos); + return; + } + const cursorPos = currentSelection.start; + let newCursorPos = 0; - const newCursorPos = prevGrapheme ? prevGrapheme.index + prevGrapheme.segment.length : 0; - setCursorPosition(target, newCursorPos, isSelectionEvent ? currentSelection.end : newCursorPos); + // eslint-disable-next-line no-restricted-syntax + for (const {index, segment} of segments) { + const segmentEnd = index + segment.length; + if (segmentEnd < cursorPos) { + newCursorPos = segmentEnd; + } } + + setCursorPosition(target, newCursorPos, isSelectionEvent ? currentSelection.end : newCursorPos); } // eslint-disable-next-line import/prefer-default-export