From 85178e85e2ce8d4ccff0b07d73cc14ee6aedb3e9 Mon Sep 17 00:00:00 2001 From: Brett Walker <bwalker@gitlab.com> Date: Wed, 16 Feb 2022 04:51:06 +0000 Subject: [PATCH] Continue a markdown list on enter/return When creating a list in markdown, when typing return or enter at the end of a list item, create a new empty list item. --- .../javascripts/lib/utils/text_markdown.js | 65 ++++++++++++++-- app/controllers/projects/issues_controller.rb | 9 ++- .../projects/merge_requests_controller.rb | 19 ++--- .../development/markdown_continue_lists.yml | 8 ++ .../design_notes/design_reply_form_spec.js | 4 + .../components/fields/description_spec.js | 1 + spec/frontend/lib/utils/text_markdown_spec.js | 74 +++++++++++++++++++ .../notes/components/note_form_spec.js | 2 + .../components/issuable_edit_form_spec.js | 1 + spec/frontend/zen_mode_spec.js | 2 + ...r_previews_wiki_changes_shared_examples.rb | 4 +- 11 files changed, 170 insertions(+), 19 deletions(-) create mode 100644 config/feature_flags/development/markdown_continue_lists.yml diff --git a/app/assets/javascripts/lib/utils/text_markdown.js b/app/assets/javascripts/lib/utils/text_markdown.js index 40dd29bea76e..ec6789d81ecb 100644 --- a/app/assets/javascripts/lib/utils/text_markdown.js +++ b/app/assets/javascripts/lib/utils/text_markdown.js @@ -5,6 +5,12 @@ import { insertText } from '~/lib/utils/common_utils'; const LINK_TAG_PATTERN = '[{text}](url)'; +// at the start of a line, find any amount of whitespace followed by +// a bullet point character (*+-) and an optional checkbox ([ ] [x]) +// OR a number with a . after it and an optional checkbox ([ ] [x]) +// followed by one or more whitespace characters +const LIST_LINE_HEAD_PATTERN = /^(?<indent>\s*)(?<leader>((?<isOl>[*+-])|(?<isUl>\d+\.))( \[([x ])\])?\s)(?<content>.)?/; + function selectedText(text, textarea) { return text.substring(textarea.selectionStart, textarea.selectionEnd); } @@ -13,8 +19,15 @@ function addBlockTags(blockTag, selected) { return `${blockTag}\n${selected}\n${blockTag}`; } -function lineBefore(text, textarea) { - const split = text.substring(0, textarea.selectionStart).trim().split('\n'); +function lineBefore(text, textarea, trimNewlines = true) { + let split = text.substring(0, textarea.selectionStart); + + if (trimNewlines) { + split = split.trim(); + } + + split = split.split('\n'); + return split[split.length - 1]; } @@ -284,9 +297,9 @@ function updateText({ textArea, tag, cursorOffset, blockTag, wrap, select, tagCo } /* eslint-disable @gitlab/require-i18n-strings */ -export function keypressNoteText(e) { +function handleSurroundSelectedText(e, textArea) { if (!gon.markdown_surround_selection) return; - if (this.selectionStart === this.selectionEnd) return; + if (textArea.selectionStart === textArea.selectionEnd) return; const keys = { '*': '**{text}**', // wraps with bold character @@ -306,7 +319,7 @@ export function keypressNoteText(e) { updateText({ tag, - textArea: this, + textArea, blockTag: '', wrap: true, select: '', @@ -316,6 +329,48 @@ export function keypressNoteText(e) { } /* eslint-enable @gitlab/require-i18n-strings */ +function handleContinueList(e, textArea) { + if (!gon.features?.markdownContinueLists) return; + if (!(e.key === 'Enter')) return; + if (e.altKey || e.ctrlKey || e.metaKey || e.shiftKey) return; + if (textArea.selectionStart !== textArea.selectionEnd) return; + + const currentLine = lineBefore(textArea.value, textArea, false); + const result = currentLine.match(LIST_LINE_HEAD_PATTERN); + + if (result) { + const { indent, content, leader } = result.groups; + const prevLineEmpty = !content; + + if (prevLineEmpty) { + // erase previous empty list item - select the text and allow the + // natural line feed erase the text + textArea.selectionStart = textArea.selectionStart - result[0].length; + return; + } + + const itemInsert = `${indent}${leader}`; + + e.preventDefault(); + + updateText({ + tag: itemInsert, + textArea, + blockTag: '', + wrap: false, + select: '', + tagContent: '', + }); + } +} + +export function keypressNoteText(e) { + const textArea = this; + + handleContinueList(e, textArea); + handleSurroundSelectedText(e, textArea); +} + export function updateTextForToolbarBtn($toolbarBtn) { return updateText({ textArea: $toolbarBtn.closest('.md-area').find('textarea'), diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index 281e21b3ab0a..1b98810b09b1 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -46,14 +46,15 @@ class Projects::IssuesController < Projects::ApplicationController push_frontend_feature_flag(:vue_issues_list, project&.group, default_enabled: :yaml) push_frontend_feature_flag(:iteration_cadences, project&.group, default_enabled: :yaml) push_frontend_feature_flag(:contacts_autocomplete, project&.group, default_enabled: :yaml) + push_frontend_feature_flag(:markdown_continue_lists, project, default_enabled: :yaml) end before_action only: :show do - push_frontend_feature_flag(:real_time_issue_sidebar, @project, default_enabled: :yaml) + push_frontend_feature_flag(:real_time_issue_sidebar, project, default_enabled: :yaml) push_frontend_feature_flag(:confidential_notes, project&.group, default_enabled: :yaml) - push_frontend_feature_flag(:issue_assignees_widget, @project, default_enabled: :yaml) - push_frontend_feature_flag(:paginated_issue_discussions, @project, default_enabled: :yaml) - push_frontend_feature_flag(:fix_comment_scroll, @project, default_enabled: :yaml) + push_frontend_feature_flag(:issue_assignees_widget, project, default_enabled: :yaml) + push_frontend_feature_flag(:paginated_issue_discussions, project, default_enabled: :yaml) + push_frontend_feature_flag(:fix_comment_scroll, project, default_enabled: :yaml) push_frontend_feature_flag(:work_items, project, default_enabled: :yaml) end diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 05452ef65786..6445f920db59 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -36,20 +36,21 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo before_action only: [:show] do push_frontend_feature_flag(:file_identifier_hash) - push_frontend_feature_flag(:merge_request_widget_graphql, @project, default_enabled: :yaml) - push_frontend_feature_flag(:default_merge_ref_for_diffs, @project, default_enabled: :yaml) - push_frontend_feature_flag(:core_security_mr_widget_counts, @project) - push_frontend_feature_flag(:paginated_notes, @project, default_enabled: :yaml) - push_frontend_feature_flag(:confidential_notes, @project, default_enabled: :yaml) + push_frontend_feature_flag(:merge_request_widget_graphql, project, default_enabled: :yaml) + push_frontend_feature_flag(:default_merge_ref_for_diffs, project, default_enabled: :yaml) + push_frontend_feature_flag(:core_security_mr_widget_counts, project) + push_frontend_feature_flag(:paginated_notes, project, default_enabled: :yaml) + push_frontend_feature_flag(:confidential_notes, project, default_enabled: :yaml) push_frontend_feature_flag(:improved_emoji_picker, project, default_enabled: :yaml) push_frontend_feature_flag(:restructured_mr_widget, project, default_enabled: :yaml) - push_frontend_feature_flag(:refactor_mr_widgets_extensions, @project, default_enabled: :yaml) - push_frontend_feature_flag(:rebase_without_ci_ui, @project, default_enabled: :yaml) + push_frontend_feature_flag(:refactor_mr_widgets_extensions, project, default_enabled: :yaml) + push_frontend_feature_flag(:rebase_without_ci_ui, project, default_enabled: :yaml) push_frontend_feature_flag(:rearrange_pipelines_table, project, default_enabled: :yaml) + push_frontend_feature_flag(:markdown_continue_lists, project, default_enabled: :yaml) # Usage data feature flags - push_frontend_feature_flag(:users_expanding_widgets_usage_data, @project, default_enabled: :yaml) + push_frontend_feature_flag(:users_expanding_widgets_usage_data, project, default_enabled: :yaml) push_frontend_feature_flag(:diff_settings_usage_data, default_enabled: :yaml) - push_frontend_feature_flag(:usage_data_diff_searches, @project, default_enabled: :yaml) + push_frontend_feature_flag(:usage_data_diff_searches, project, default_enabled: :yaml) end before_action do diff --git a/config/feature_flags/development/markdown_continue_lists.yml b/config/feature_flags/development/markdown_continue_lists.yml new file mode 100644 index 000000000000..8be9a3008da7 --- /dev/null +++ b/config/feature_flags/development/markdown_continue_lists.yml @@ -0,0 +1,8 @@ +--- +name: markdown_continue_lists +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/79161 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/351386 +milestone: '14.8' +type: development +group: group::project management +default_enabled: false diff --git a/spec/frontend/design_management/components/design_notes/design_reply_form_spec.js b/spec/frontend/design_management/components/design_notes/design_reply_form_spec.js index d2d1fe6b2d88..0cef18c60de2 100644 --- a/spec/frontend/design_management/components/design_notes/design_reply_form_spec.js +++ b/spec/frontend/design_management/components/design_notes/design_reply_form_spec.js @@ -31,6 +31,10 @@ describe('Design reply form component', () => { }); } + beforeEach(() => { + gon.features = { markdownContinueLists: true }; + }); + afterEach(() => { wrapper.destroy(); }); diff --git a/spec/frontend/issues/show/components/fields/description_spec.js b/spec/frontend/issues/show/components/fields/description_spec.js index 3043c4c36735..dd511c3945c3 100644 --- a/spec/frontend/issues/show/components/fields/description_spec.js +++ b/spec/frontend/issues/show/components/fields/description_spec.js @@ -25,6 +25,7 @@ describe('Description field component', () => { beforeEach(() => { jest.spyOn(eventHub, '$emit'); + gon.features = { markdownContinueLists: true }; }); afterEach(() => { diff --git a/spec/frontend/lib/utils/text_markdown_spec.js b/spec/frontend/lib/utils/text_markdown_spec.js index ab81ec47b64d..dded32cc890f 100644 --- a/spec/frontend/lib/utils/text_markdown_spec.js +++ b/spec/frontend/lib/utils/text_markdown_spec.js @@ -165,6 +165,80 @@ describe('init markdown', () => { // cursor placement should be between tags expect(textArea.selectionStart).toBe(start.length + tag.length); }); + + describe('Continuing markdown lists', () => { + const enterEvent = new KeyboardEvent('keydown', { key: 'Enter' }); + + beforeEach(() => { + gon.features = { markdownContinueLists: true }; + }); + + it.each` + text | expected + ${'- item'} | ${'- item\n- '} + ${'- [ ] item'} | ${'- [ ] item\n- [ ] '} + ${'- [x] item'} | ${'- [x] item\n- [x] '} + ${'- item\n - second'} | ${'- item\n - second\n - '} + ${'1. item'} | ${'1. item\n1. '} + ${'1. [ ] item'} | ${'1. [ ] item\n1. [ ] '} + ${'1. [x] item'} | ${'1. [x] item\n1. [x] '} + ${'108. item'} | ${'108. item\n108. '} + ${'108. item\n - second'} | ${'108. item\n - second\n - '} + ${'108. item\n 1. second'} | ${'108. item\n 1. second\n 1. '} + `('adds correct list continuation characters', ({ text, expected }) => { + textArea.value = text; + textArea.setSelectionRange(text.length, text.length); + + textArea.addEventListener('keydown', keypressNoteText); + textArea.dispatchEvent(enterEvent); + + expect(textArea.value).toEqual(expected); + expect(textArea.selectionStart).toBe(expected.length); + }); + + // test that when pressing Enter on an empty list item, the empty + // list item text is selected, so that when the Enter propagates, + // it's removed + it.each` + text | expected + ${'- item\n- '} | ${'- item\n'} + ${'- [ ] item\n- [ ] '} | ${'- [ ] item\n'} + ${'- [x] item\n- [x] '} | ${'- [x] item\n'} + ${'- item\n - second\n - '} | ${'- item\n - second\n'} + ${'1. item\n1. '} | ${'1. item\n'} + ${'1. [ ] item\n1. [ ] '} | ${'1. [ ] item\n'} + ${'1. [x] item\n1. [x] '} | ${'1. [x] item\n'} + ${'108. item\n108. '} | ${'108. item\n'} + ${'108. item\n - second\n - '} | ${'108. item\n - second\n'} + ${'108. item\n 1. second\n 1. '} | ${'108. item\n 1. second\n'} + `('adds correct list continuation characters', ({ text, expected }) => { + textArea.value = text; + textArea.setSelectionRange(text.length, text.length); + + textArea.addEventListener('keydown', keypressNoteText); + textArea.dispatchEvent(enterEvent); + + expect(textArea.value.substr(0, textArea.selectionStart)).toEqual(expected); + expect(textArea.selectionStart).toBe(expected.length); + expect(textArea.selectionEnd).toBe(text.length); + }); + + it('does nothing if feature flag disabled', () => { + gon.features = { markdownContinueLists: false }; + + const text = '- item'; + const expected = '- item'; + + textArea.value = text; + textArea.setSelectionRange(text.length, text.length); + + textArea.addEventListener('keydown', keypressNoteText); + textArea.dispatchEvent(enterEvent); + + expect(textArea.value).toEqual(expected); + expect(textArea.selectionStart).toBe(expected.length); + }); + }); }); describe('with selection', () => { diff --git a/spec/frontend/notes/components/note_form_spec.js b/spec/frontend/notes/components/note_form_spec.js index d3b5ab02f248..3e80b24f1283 100644 --- a/spec/frontend/notes/components/note_form_spec.js +++ b/spec/frontend/notes/components/note_form_spec.js @@ -45,6 +45,8 @@ describe('issue_note_form component', () => { noteBody: 'Magni suscipit eius consectetur enim et ex et commodi.', noteId: '545', }; + + gon.features = { markdownContinueLists: true }; }); afterEach(() => { diff --git a/spec/frontend/vue_shared/issuable/show/components/issuable_edit_form_spec.js b/spec/frontend/vue_shared/issuable/show/components/issuable_edit_form_spec.js index d3e484cf9130..b79dc0bf9768 100644 --- a/spec/frontend/vue_shared/issuable/show/components/issuable_edit_form_spec.js +++ b/spec/frontend/vue_shared/issuable/show/components/issuable_edit_form_spec.js @@ -36,6 +36,7 @@ describe('IssuableEditForm', () => { beforeEach(() => { wrapper = createComponent(); + gon.features = { markdownContinueLists: true }; }); afterEach(() => { diff --git a/spec/frontend/zen_mode_spec.js b/spec/frontend/zen_mode_spec.js index 13f221fd9d90..44684619faec 100644 --- a/spec/frontend/zen_mode_spec.js +++ b/spec/frontend/zen_mode_spec.js @@ -45,6 +45,8 @@ describe('ZenMode', () => { // Set this manually because we can't actually scroll the window zen.scroll_position = 456; + + gon.features = { markdownContinueLists: true }; }); describe('enabling dropzone', () => { diff --git a/spec/support/shared_examples/features/wiki/user_previews_wiki_changes_shared_examples.rb b/spec/support/shared_examples/features/wiki/user_previews_wiki_changes_shared_examples.rb index 1a981f42086f..2285d9a17e22 100644 --- a/spec/support/shared_examples/features/wiki/user_previews_wiki_changes_shared_examples.rb +++ b/spec/support/shared_examples/features/wiki/user_previews_wiki_changes_shared_examples.rb @@ -85,7 +85,9 @@ def relative_path(path) end it 'renders content with CommonMark' do - fill_in :wiki_content, with: "1. one\n - sublist\n" + # using two `\n` ensures we're sublist to it's own line due + # to list auto-continue + fill_in :wiki_content, with: "1. one\n\n - sublist\n" click_on "Preview" # the above generates two separate lists (not embedded) in CommonMark -- GitLab