From 5196a184a0a1023cd9b7cdbf1708375f9c4f6095 Mon Sep 17 00:00:00 2001 From: Sondre Kjempekjenn Date: Fri, 20 Oct 2023 14:23:30 +0200 Subject: [PATCH] [GH-22370] Disable markdown keybindings within codeblocks (#24558) * Disable markdown keybindings within codeblocks Addresses a usability issue where certain keyboard combinations, primarily used in nordic keyboard layouts, conflict with markdown shortcuts in the editor. This is especially problematic when trying to write characters like \, {, or }. Given that markdown rendering does not happen within code blocks, this commit disables markdown shortcuts in those regions to allow for normal text entry. Related Issue: #22370 * Added test suite * Add test to ensure isWithinCodeBlock is stateless * Disable markdown keybindings within codeblocks for advanced_create_comment * Fix linting issues --- .../advanced_create_comment.tsx | 9 +- .../advanced_create_post.tsx | 10 +- webapp/channels/src/utils/constants.tsx | 2 +- webapp/channels/src/utils/post_utils.test.tsx | 143 ++++++++++++++++++ webapp/channels/src/utils/post_utils.ts | 14 +- 5 files changed, 166 insertions(+), 12 deletions(-) diff --git a/webapp/channels/src/components/advanced_create_comment/advanced_create_comment.tsx b/webapp/channels/src/components/advanced_create_comment/advanced_create_comment.tsx index dfd7e5d992..c8cc8bf8f2 100644 --- a/webapp/channels/src/components/advanced_create_comment/advanced_create_comment.tsx +++ b/webapp/channels/src/components/advanced_create_comment/advanced_create_comment.tsx @@ -52,6 +52,7 @@ import { splitMessageBasedOnCaretPosition, groupsMentionedInText, mentionsMinusSpecialMentionsInText, + isWithinCodeBlock, } from 'utils/post_utils'; import * as UserAgent from 'utils/user_agent'; import * as Utils from 'utils/utils'; @@ -880,6 +881,8 @@ class AdvancedCreateComment extends React.PureComponent { const draft = this.state.draft!; const {message} = draft; + const {caretPosition} = this.state; + const caretIsWithinCodeBlock = caretPosition && isWithinCodeBlock(message, caretPosition); if (Keyboard.isKeyPressed(e, KeyCodes.ESCAPE)) { this.textboxRef.current?.blur(); @@ -910,7 +913,7 @@ class AdvancedCreateComment extends React.PureComponent { value, } = e.target as TextboxElement; - if (ctrlKeyCombo) { + if (ctrlKeyCombo && !caretIsWithinCodeBlock) { if (Keyboard.isKeyPressed(e, KeyCodes.UP)) { e.preventDefault(); this.props.onMoveHistoryIndexBack(); @@ -945,7 +948,7 @@ class AdvancedCreateComment extends React.PureComponent { message: value, }); } - } else if (ctrlAltCombo) { + } else if (ctrlAltCombo && !caretIsWithinCodeBlock) { if (Keyboard.isKeyPressed(e, KeyCodes.K)) { e.stopPropagation(); e.preventDefault(); @@ -977,7 +980,7 @@ class AdvancedCreateComment extends React.PureComponent { e.preventDefault(); this.setShowPreview(!this.props.shouldShowPreview); } - } else if (shiftAltCombo) { + } else if (shiftAltCombo && !caretIsWithinCodeBlock) { if (Keyboard.isKeyPressed(e, KeyCodes.X)) { e.stopPropagation(); e.preventDefault(); diff --git a/webapp/channels/src/components/advanced_create_post/advanced_create_post.tsx b/webapp/channels/src/components/advanced_create_post/advanced_create_post.tsx index 824146a5ac..6ad01812ab 100644 --- a/webapp/channels/src/components/advanced_create_post/advanced_create_post.tsx +++ b/webapp/channels/src/components/advanced_create_post/advanced_create_post.tsx @@ -67,6 +67,7 @@ import { groupsMentionedInText, mentionsMinusSpecialMentionsInText, hasRequestedPersistentNotifications, + isWithinCodeBlock, } from 'utils/post_utils'; import * as UserAgent from 'utils/user_agent'; import * as Utils from 'utils/utils'; @@ -1175,7 +1176,8 @@ class AdvancedCreatePost extends React.PureComponent { return; } - const {message} = this.state; + const {message, caretPosition} = this.state; + const caretIsWithinCodeBlock = isWithinCodeBlock(message, caretPosition); if (Keyboard.isKeyPressed(e, KeyCodes.ESCAPE)) { this.textboxRef.current?.blur(); @@ -1203,7 +1205,7 @@ class AdvancedCreatePost extends React.PureComponent { value, } = e.target as TextboxElement; - if (ctrlKeyCombo) { + if (ctrlKeyCombo && !caretIsWithinCodeBlock) { if (draftMessageIsEmpty && Keyboard.isKeyPressed(e, KeyCodes.UP)) { e.stopPropagation(); e.preventDefault(); @@ -1240,7 +1242,7 @@ class AdvancedCreatePost extends React.PureComponent { message: value, }); } - } else if (ctrlAltCombo) { + } else if (ctrlAltCombo && !caretIsWithinCodeBlock) { if (Keyboard.isKeyPressed(e, KeyCodes.K)) { e.stopPropagation(); e.preventDefault(); @@ -1272,7 +1274,7 @@ class AdvancedCreatePost extends React.PureComponent { e.preventDefault(); this.setShowPreview(!this.props.shouldShowPreview); } - } else if (shiftAltCombo) { + } else if (shiftAltCombo && !caretIsWithinCodeBlock) { if (Keyboard.isKeyPressed(e, KeyCodes.X)) { e.stopPropagation(); e.preventDefault(); diff --git a/webapp/channels/src/utils/constants.tsx b/webapp/channels/src/utils/constants.tsx index ad1e64a9ce..809cb182ea 100644 --- a/webapp/channels/src/utils/constants.tsx +++ b/webapp/channels/src/utils/constants.tsx @@ -2012,7 +2012,7 @@ export const Constants = { SEARCH_POST: 'searchpost', CHANNEL_ID_LENGTH: 26, TRANSPARENT_PIXEL: 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII=', - TRIPLE_BACK_TICKS: /```/g, + REGEX_CODE_BLOCK_OPTIONAL_LANGUAGE_TAG: /^```.*$/gm, MAX_ATTACHMENT_FOOTER_LENGTH: 300, ACCEPT_STATIC_IMAGE: '.jpeg,.jpg,.png,.bmp', ACCEPT_EMOJI_IMAGE: '.jpeg,.jpg,.png,.gif', diff --git a/webapp/channels/src/utils/post_utils.test.tsx b/webapp/channels/src/utils/post_utils.test.tsx index 47c236e98a..4c4b89d65e 100644 --- a/webapp/channels/src/utils/post_utils.test.tsx +++ b/webapp/channels/src/utils/post_utils.test.tsx @@ -1119,6 +1119,149 @@ describe('PostUtils.getPostURL', () => { }); }); +describe('PostUtils.isWithinCodeBlock', () => { + const CARET_MARKER = '•'; + const TRIPLE_BACKTICKS = '```'; + + const getCaretAndMsg = (textWithCaret: string): [number, string] => { + const normalizedText = textWithCaret.split('\n').map((line) => line.replace(/^\s*\|/, '')).join('\n'); + + return [normalizedText.indexOf(CARET_MARKER), normalizedText]; + }; + + it('should return true if caret is within a code block', () => { + const [caretPosition, message] = getCaretAndMsg(` + |This is a line of text + |${TRIPLE_BACKTICKS} + | fun main() { + | println("Hello Wo${CARET_MARKER}") + | } + |${TRIPLE_BACKTICKS} + |This is a line of text + `); + expect(PostUtils.isWithinCodeBlock(message, caretPosition)).toBe(true); + }); + + it('should return false if caret is not within a code block', () => { + const [caretPosition, message] = getCaretAndMsg(` + |This is a line of text + |${TRIPLE_BACKTICKS} + | fun main() { + | println("Hello World") + | } + |${TRIPLE_BACKTICKS} + |This is a line of t${CARET_MARKER} + `); + expect(PostUtils.isWithinCodeBlock(message, caretPosition)).toBe(false); + }); + + it('should handle code blocks with language tags', () => { + const [caretPosition, message] = getCaretAndMsg(` + |This is a line of text + |${TRIPLE_BACKTICKS}kotlin + | fun main() { + | println("Hello Wo${CARET_MARKER}") + | } + |${TRIPLE_BACKTICKS} + |This is a line of text + `); + expect(PostUtils.isWithinCodeBlock(message, caretPosition)).toBe(true); + }); + + it('should handle caret in front of code block', () => { + const [caretPosition, message] = getCaretAndMsg(` + |${CARET_MARKER}${TRIPLE_BACKTICKS}kotlin + | fun main() { + | println("Test") + | } + |${TRIPLE_BACKTICKS} + |This is text + `); + expect(PostUtils.isWithinCodeBlock(message, caretPosition)).toBe(false); + }); + + it('should handle caret behind code block', () => { + const [caretPosition, message] = getCaretAndMsg(` + |${TRIPLE_BACKTICKS}kotlin + | fun main() { + | println("Test") + | } + |${TRIPLE_BACKTICKS}${CARET_MARKER} + |This is text + `); + expect(PostUtils.isWithinCodeBlock(message, caretPosition)).toBe(false); + }); + + it('should handle multiple code blocks', () => { + const [caretPosition, message] = getCaretAndMsg(` + |${TRIPLE_BACKTICKS} + | Code Block 1 + |${TRIPLE_BACKTICKS} + |This is text + |${TRIPLE_BACKTICKS} + | Code Block 2 ${CARET_MARKER} + |${TRIPLE_BACKTICKS} + `); + expect(PostUtils.isWithinCodeBlock(message, caretPosition)).toBe(true); + }); + + it('should handle empty code blocks', () => { + const [caretPosition, message] = getCaretAndMsg(` + |${TRIPLE_BACKTICKS} + |${TRIPLE_BACKTICKS} + |${CARET_MARKER} + `); + expect(PostUtils.isWithinCodeBlock(message, caretPosition)).toBe(false); + }); + + it('should handle consecutive code blocks', () => { + const [caretPosition, message] = getCaretAndMsg(` + |${TRIPLE_BACKTICKS} + | Code Block 1 + |${TRIPLE_BACKTICKS} + |This is text + |${TRIPLE_BACKTICKS} + | Code Block 2 ${CARET_MARKER} + |${TRIPLE_BACKTICKS} + |${TRIPLE_BACKTICKS} + | Code Block 3 + |${TRIPLE_BACKTICKS} + |This is text + `); + expect(PostUtils.isWithinCodeBlock(message, caretPosition)).toBe(true); + }); + + it('should handle caret position at 0', () => { + const [caretPosition, message] = getCaretAndMsg(`${CARET_MARKER} + |This is text + `); + expect(PostUtils.isWithinCodeBlock(message, caretPosition)).toBe(false); + }); + + it('should handle whitespace within and around code blocks', () => { + const [caretPosition, message] = getCaretAndMsg(` + |${TRIPLE_BACKTICKS} + | Test text asd 1 + | ${CARET_MARKER} + |${TRIPLE_BACKTICKS} + `); + expect(PostUtils.isWithinCodeBlock(message, caretPosition)).toBe(true); + }); + + it('should produce consistent results when called multiple times', () => { + const [caretPosition, message] = getCaretAndMsg(` + |${TRIPLE_BACKTICKS} + | Test text asd 1 + | ${CARET_MARKER} + |${TRIPLE_BACKTICKS} + `); + + const results = Array.from({length: 10}, () => PostUtils.isWithinCodeBlock(message, caretPosition)); + + expect(results.every(Boolean)).toBe(true); + }); +}); + describe('PostUtils.getMentionDetails', () => { const user1 = TestHelper.getUserMock({username: 'user1'}); const user2 = TestHelper.getUserMock({username: 'user2'}); diff --git a/webapp/channels/src/utils/post_utils.ts b/webapp/channels/src/utils/post_utils.ts index 1fb1005daf..7fb7d7f39e 100644 --- a/webapp/channels/src/utils/post_utils.ts +++ b/webapp/channels/src/utils/post_utils.ts @@ -276,13 +276,19 @@ function canAutomaticallyCloseBackticks(message: string) { return {allowSending: true}; } +export function isWithinCodeBlock(message: string, caretPosition: number): boolean { + const match = message.substring(0, caretPosition).match(Constants.REGEX_CODE_BLOCK_OPTIONAL_LANGUAGE_TAG); + + return Boolean(match && match.length % 2 !== 0); +} + function sendOnCtrlEnter(message: string, ctrlOrMetaKeyPressed: boolean, isSendMessageOnCtrlEnter: boolean, caretPosition: number) { - const match = message.substring(0, caretPosition).match(Constants.TRIPLE_BACK_TICKS); - if (isSendMessageOnCtrlEnter && ctrlOrMetaKeyPressed && (!match || match.length % 2 === 0)) { + const inCodeBlock = isWithinCodeBlock(message, caretPosition); + if (isSendMessageOnCtrlEnter && ctrlOrMetaKeyPressed && !inCodeBlock) { return {allowSending: true}; - } else if (!isSendMessageOnCtrlEnter && (!match || match.length % 2 === 0)) { + } else if (!isSendMessageOnCtrlEnter && !inCodeBlock) { return {allowSending: true}; - } else if (ctrlOrMetaKeyPressed && match && match.length % 2 !== 0) { + } else if (ctrlOrMetaKeyPressed && inCodeBlock) { return canAutomaticallyCloseBackticks(message); }