[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
This commit is contained in:
Sondre Kjempekjenn 2023-10-20 14:23:30 +02:00 committed by GitHub
parent 745e5252ab
commit 5196a184a0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 166 additions and 12 deletions

View File

@ -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<Props, State> {
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<Props, State> {
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<Props, State> {
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<Props, State> {
e.preventDefault();
this.setShowPreview(!this.props.shouldShowPreview);
}
} else if (shiftAltCombo) {
} else if (shiftAltCombo && !caretIsWithinCodeBlock) {
if (Keyboard.isKeyPressed(e, KeyCodes.X)) {
e.stopPropagation();
e.preventDefault();

View File

@ -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<Props, State> {
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<Props, State> {
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<Props, State> {
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<Props, State> {
e.preventDefault();
this.setShowPreview(!this.props.shouldShowPreview);
}
} else if (shiftAltCombo) {
} else if (shiftAltCombo && !caretIsWithinCodeBlock) {
if (Keyboard.isKeyPressed(e, KeyCodes.X)) {
e.stopPropagation();
e.preventDefault();

View File

@ -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',

View File

@ -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'});

View File

@ -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);
}