fix: different behavior of blockquote between markdown and wysiwyg (#325) #326

sohee-lee7 · 2018-11-12T02:11:51Z

Please check if the PR fulfills these requirements

It's the right issue type on the title
When resolving a specific issue, it's referenced in the PR's title (e.g. fix #xxx[,#xxx], where "xxx" is the issue number)
The commit message follows our guidelines
Tests for the changes have been added (for bug fixes/features)
Docs have been added/updated (for bug fixes/features)
It does not introduce a breaking change or has a description of the breaking change

Description

#325
인용문구 툴바 버튼으로 인용문구 추가 시에 이미 인용문구가 있으면 인용문구 제거, 없을 시에만 추가하는 기능입니다.
만약 여러라인을 선택하여 추가 시에 하나의 라인이라도 인용문구가 없으면 선택한 라인 전체에 인용문구 추가합니다.

Thank you for your contribution to TOAST UI product. 🎉 😘 ✨

kyuwoo-choi

리뷰 완료입니다. 고생했습니다.

다른분들 이슈 이해를 돕기 위해, 설명을 살짝 추가해주면 좋겠어요.

kyuwoo-choi · 2018-11-13T01:13:34Z

src/js/markdownCommands/blockquote.js

@@ -35,11 +37,13 @@ const Blockquote = CommandManager.command('markdown', /** @lends Blockquote */{
    };

    const textToModify = doc.getRange(from, to);
-    const textLinesToModify = textToModify.split('\n');
-    const lineLength = textLinesToModify.length;
+    let textLinesToModify = textToModify.split('\n');


가급적 const를 유지하고, 수정전/후 문자열 배열은 각기 더 알맞는 이름의 변수에 넣는 것이 읽기 좋겠습니다.

기본적으로 const 를 사용하고 중간 수정이 필요한 데이터만 let을 사용하는 것을 원칙으로 하면 될것 같아요

변경에 참고가 되는 데이터인 textLinesToModify는 const로 유지하고, 중간 수정이 필요한 데이터를 let으로 사용하도록 하여 변경하겠습니다.

kyuwoo-choi · 2018-11-13T01:23:23Z

src/js/markdownCommands/blockquote.js

+  },
+
+  addBlockquote(textArr) {
+    return textArr.map(text => `>${text}`);


BlockquoteStr 상수가 선언되어 있습니다.
그것을 사용하던가, 안쓰이는 상수를 삭제해도 좋겠습니다.

상수를 사용하도록 수정하겠습니다

kyuwoo-choi · 2018-11-13T01:26:33Z

src/js/markdownCommands/blockquote.js

+  },
+
+  removeBlockquote(textArr) {
+    return textArr.map(text => text.slice(1, text.length));


to-mark에서는 blockquote를 > text 처럼 >과 text 사이에 스페이스 하나가 들어가 있습니다.

commonmark 표준에도 >text, > text 둘 다 허용하고 있습니다.
2가지 케이스 모두 고려해서 불필요한 공백이 생기지 않도록 수정해야합니다.

https://spec.commonmark.org/0.28/#block-quotes

인용문구 추가 시에는 > 공백 포함하여 추가하고, 제거 시에는 > 혹은 >을 제거하도록 수정하겠습니다.

kyuwoo-choi · 2018-11-13T01:32:00Z

test/unit/markdownCommands/blockquote.spec.js

@@ -63,4 +63,34 @@ describe('Blockquote', () => {
      expect(cm.getValue()).toEqual(['>mytext1', '>', '>mytext2', 'mytext3'].join('\n'));
    });
  });
+
+  describe('removing quote in one line', () => {


아래 2개의 TC에 공백이 들어간 케이스도 추가해주세요.

seonim-ryu · 2018-11-13T03:13:33Z

src/js/markdownCommands/blockquote.js

@@ -49,6 +53,24 @@ const Blockquote = CommandManager.command('markdown', /** @lends Blockquote */{
    doc.setCursor(range.to);

    cm.focus();
+  },
+
+  haveBlockquote(textArr) {


이 메서드는 외부 모듈에서 호출되는 메서드인가요?
프라이빗 메서드로 보입니다.
프라이빗은 _[methodName] 컨벤션입니다~

컨벤션에 맞게 수정하겠습니다

seonim-ryu · 2018-11-13T03:15:18Z

[11/13] 리뷰 완료하였습니다. 첫 에디터 리뷰네요ㅎㅎ 고생하셨어요~

shiren

수고하셨어요~

shiren · 2018-11-13T03:18:41Z

src/js/markdownCommands/blockquote.js

@@ -35,11 +37,13 @@ const Blockquote = CommandManager.command('markdown', /** @lends Blockquote */{
    };

    const textToModify = doc.getRange(from, to);
-    const textLinesToModify = textToModify.split('\n');
-    const lineLength = textLinesToModify.length;
+    let textLinesToModify = textToModify.split('\n');


기본적으로 const 를 사용하고 중간 수정이 필요한 데이터만 let을 사용하는 것을 원칙으로 하면 될것 같아요

) (#326) fix: different behavior of blockquote between markdown and wysiwyg (#325)

Fixes #326

* Release v1.8.13 * Fix documentation error * Update built version from latest source * Release v1.8.14 * Make createRange method public * Parse url and create anchor with queryparams * Build lib * Simplify regex for matching link query params * Make check for link protocol case-insensitive Resolves #308. * Allow class names to be configured * Add config.willCutCopy option Is an optional function that transforms the HTML being cut/copied before placing it on the clipboard. * Make TreeWalker filter argument optional * Make shift-enter always just add <br> * Add support for <pre>/<code> formatting * Convert adjacent space to nbsp when extracting range This resolves the issue where if you selected a word and then typed to replace it, the following space would be deleted as well. Fixes #331 * Escape <a> on space even if in nested tags Fixes #326 * Make detect-link regular expression customisable Resolves #313 * Update readme * Fix Kana-Kanji input on Mac Safari Resolves #332 * Release v1.9.0 * fix: event.key does not supported IE * Revert "Preserve block style if pasting on blank line" This reverts commit 50fb7c7c53ccb9b9fe03866ac02416d4a6989e66. * fix: merge td or tr when merge block and container (#1) * fix: blocking when merge td with div In tui.editor, when div is inserted using TextObject in the table, TD is merge div. So should allow td merge other tags. * fix: remove tag when pasting heading, list, blockquote (#2) * fix: HR is set to inline element HR is block element so HR is removed in inlineNodeNames and inserted in allowedBlock. * fix: fixcursor insert br into hr * enhance: handle element that has contenteditable as falase (close: #7) (#8) * Revert "enhance: handle element that has contenteditable as falase (close: #7) (#8)" This reverts commit f164d13a40b6d2f1a664f3c941f94ed48451755d. * fix: backspace/delete key remove contenteditable false block (fix: #7) * fix: merge div that has contenteditable false (fix: #7) * fix: focus() moves scroll position (fix: #1) * fix: the <img /> tag is not considered when checking line breaks (fix #2) (#3) * fix: setActive error, revert notWSTextNode fix: setActive error, revert notWSTextNode * fix: line break works correctly between <br> to <img> (fix #2) (#5) * feat: add allowedBlocks option (close #6) (#7) * Revert "feat: add allowedBlocks option (close #6) (#7)" (#8) This reverts commit 154bd6a0c0707f46f2a2435d8886a9068b3930c3. * feat: add allowedBlocks option (close #6) (#9) * feat: add allowedBlocks option (close #6) * feat: make build file * 환경셋팅 저장 * change bower, package informations * donRunner첫작업 완료 resolve #2 * 룰 적용 구조 작업 resolve #3 * 정리 * fixed render comma rule bug * add getNodeText to domRunner * basicRender Heading resolve #4 * 개행 프로세스 추가, css스타일의 셀렉터 필터 추가 resolve #6, #5 * 베이직 렌더러 - 링크 resolve #8 * 베이직 렌더러 - 이미지 resolve #7 * add list converter to renderer resolve #9 * 디펜던시 모듈들 업데이트 * change rule apply code * nested li resolve #12 * separate test case for toMark, strong converter added resolve #15 * code converter resolve #13 * li-heading convert inline html * add horizontal rule converter to renderer resolve #11 * add blockquote converter to renderer resolve #10 * code block converter resolve #14 * entry point filename change * build test and create demo test page resolve #16 * string trim resolve #17 * stop track rule over html root, and some fixed #21 * renderer getConverter refactor resolved #23 * added trimming process to toDom #19 * markdown backslash escapes resolve #24 * 화이트 스페이스에 대한 처리 resolve #19 * Convert with dom node resolve #25 * dom demo page resolve #26 * ol li number increase for li count resolve #27 * etc * add jsdoc resolve #31 * some fixed * some fixed: * refactor regex codes * refactor regex codes 2 * apply code review resolve #33 * text node in code tag not escape resolve #34 * gfm code block resolve #30 * if rule converter returns falsy renderer.converter returns empty string resolve #36 * gfm checkbox resolve #28 * gfm table resolve #29 * gfm del resolve #37 * some fixed * some fixed * add options * edit json files * add some table related code and edit demo html file * fixed problem in firefox * some fix after ie test, some code refactor * some code fixed * some demo code fixed * table header text fixed #46 * added space related codes resolve #51 * fixed p tags resolved #43 * b, s tag support resolve #48 * fix some tc * amd considered entry point * change p tag converter * change connect port to 8081 * change p tag converter resolve #54 * some convertors returns fixed, non-standard tag converter fixed * renderer.getSpaceControlled for non-standard inline tag * collapse duplicated returns made by <br /> and block element fixed #57 * remove div converter * some code fixed related <br> and return * some fixed * empty list resolve #58 * fixed gfm input code resolve #60 * some code fixed * ready to bower * v0.5.0 * some fixed from finalize code * dist * add code for empty inline element resolve #62 * build * some fixed #62 * build * some code fixed empty inline element related #62 * some code fixed * p in p is not valid html * fix empty p tag * add testcase about empty p tag * build * some tc added, gfm br added * build * fix gfm br * build * process br in end of li * add some tc * build * code fixed related p in li * build process added, prepare bower publish * change bower.json * bower fixed * escape text only contain markdown syntax resolve #63 * build * finalize multiple empty line fixed #67 * empty list render fixed #65 * empty list render2 fixed #65 * build * multiple block in p related #67 * build * some code fixed related remove returns * build * enhance about return resolve #69, resolve #68 * finalize refactoring resolve #72 * remove unused codes * build * blockquote multiple br convert to one br * build * some code fixed * build * all backticks need escape resolve #74 * apply diferent syntax on strong and em resolve #64 * build * control gfm block returns resolved #76 * build * return outerHTML if there is no converter and ie7,8 tc enhance fixed #77, #78 * resolve some tc problem about pre-code in ie78 fixed #79 * build * inline html should meged with subcontent resolve #80 * build * remove td, th when they have not text content * link enhanced about how to get href fixed #82 * build 0.0.1 * version 0.0.1 * dont remove empty td or th resolved #83 * inline tags need get space control with siblings resolved #84 * some code fixed * build 0.0.2 * preseve reurns in code block * build * release version 0.0.3 * escape markdown blockquote resolved #85 * tc fixed for cross browser test resolved #86 * build 0.0.4 * Apply inputless task resolved #90 * Add characters that should be escaped resolved #89, #88 * Should render link title attribute correctly fixed #87 * Enhance cross-browsing * Update 0.0.5 * Update bower.json, package.json * Enhance P in Lists fixed #92 * Package update & apply webpack fixed #92 * Update 0.0.6 * Add `-+` into list regexp fixes #98 * Modify _isNeedEscape() regexp related #98 * Merge pull request #97 from sungho-kim/vertical-bar-escape Add vertical bar regexp for escape fixes #96 * Update 0.0.7 * Rebuild 0.0.7 * Fix T/C fail * Merge pull request #100 from sungho-kim/task-convert Empty task converting without text content * Update 0.0.8 * Merge pull request #102 from junghwan-park/img-attr-path2 Preserve IMG element's relative path URI fixes #103 * Update 0.0.9 * Merge pull request #104 from sungho-kim/tomark-pass Convert method returns plain HTML on specific attribute * Update to 0.0.10 * feat: Renderer, basicRenderer and gfmRenderer are publicly available * fix: Fix change for anchor href * fix(escape): escape html in text node #2 * fix: escape 3-tildes code block * chore: version update to 0.0.13 * Merge pull request #4 from nhnent/hotfix/normalize_text_node normalize text nodes * version up date to 0.0.14 * fix: wrong childNodes length on tracker #5 * chore: add karma saucelab * chore: remove dist files * chore: version update to 0.0.16-pre * Merge branch 'hotfix/root_children_text_normalize_for_ie' * chore: karma config update run TC on safari & edge * chore: version update to 0.0.17-pre * feat: print number of backticks by 'data-backticks' attr (#7) * feat: print number of backticks by 'data-backticks' attr * refactor: apply code review (refs #7) * chore: version update to 0.0.17 * fix: wrong inline text conversion ,;:$&+= (fix #9) * chore: version update to 0.0.18 * chore: karma webdriver launcher change * chore: update ne, sl, local karma setting * chore: add npm dev command * feat: add npm support * chore: version update to 0.0.19 * chore: ready to publish on npm * chore: rename toMark to to-mark according to npm naming policy (ref #10) * feat: rename toMark to to-mark according to npm naming policy - change copyright - update webpack as to-mark can not be a namespace(toMark) * chore: karma version update to 2 * feat: code block with data-backticks property (close: #11, #12) * chore: version update to 1.1.0 * fix: breaking link, image markdown with [,(,< (close #14) (#15) * fix: breaking link, image markdown with [,(,< (close #14) * refactor: apply code review (ref #15) * chore: update dependencies * chore: version update to 1.1.1 * fix: newline in table breaks syntax (close #16)(#17) * chore: version update to 1.1.2 * fix: escape html + md syntax mixed text (close #19) (#20) * chore: version update to 1.1.3 * fix: underscore(_) is not allowed inside words * fix: test case about emphasis change from _ to * * chore: version update to 1.1.4 * fix: check strong, em, strike syntax (close: #23) * chore: version update to 1.1.5 * fix: list can contain heading (fix #25) (#26) * fix: list can contain heading (fix #25) * refactor: reflect code review * chore: version update to 1.1.6 * fix: backslash as text is removed * fix: not handle start attribute of OL * Merge pull request #31 from nhn/fix/escape-all-emphasis-characters fix: change logic to escape emphasis characters (fix #30) * chore: update config files for test (close #32) (#33) * feat: escape backticks (fix #34) (#35) * fix: convert child element of data-tomark-pass (#39) * fix: prevent image inside link from being escaped (#40) * fix: prevent image inside link from being escaped * refactor: remove unnecessary null comparision * fix: regexp test state reset (#41) * chore: update version to v1.1.8 * fix: escape emphasis characters twice (fix #42) (#43) * chore: update version to v1.1.9 Co-authored-by: Neil Jenkins <neil@nmjenkins.com> Co-authored-by: dhoko <dhoko@users.noreply.github.com> Co-authored-by: 이소희 <42666492+sohee-lee7@users.noreply.github.com> Co-authored-by: 김동우 <dongwoo.kim@nhnent.com> Co-authored-by: Sungho Kim <shirenbeat@gmail.com> Co-authored-by: 박정환 <junghwan-park@users.noreply.github.com> Co-authored-by: 강지웅 <trkang79@gmail.com> Co-authored-by: KyuWoo Choi <kyuwoo.choi@gmail.com>

Sohee Lee added 2 commits November 12, 2018 11:05

fix: change blockquote command to toggle blockquote state (#325)

66910a2

test: add blockquote command test

e28310b

kyuwoo-choi reviewed Nov 13, 2018

View reviewed changes

seonim-ryu reviewed Nov 13, 2018

View reviewed changes

shiren approved these changes Nov 13, 2018

View reviewed changes

Sohee Lee added 2 commits November 13, 2018 12:42

fix: reflect codereview

1e7a8ca

fix: check quotations using regex

ea86e52

sohee-lee7 changed the title ~~fix: change blockquote command to toggle blockquote state (#325)~~ fix: different behavior of blockquote between markdown and wysiwyg (#325) Nov 13, 2018

sohee-lee7 merged commit ae4e27b into master Nov 13, 2018

sohee-lee7 deleted the fix/blockquote branch November 13, 2018 11:09

seonim-ryu pushed a commit that referenced this pull request Jan 2, 2020

post process for tasks after paste or cut fixed #326

5439ff9

seonim-ryu pushed a commit that referenced this pull request Jan 2, 2020

fix: different behavior of blockquote between markdown and wysiwyg (#325

c4c7d90

) (#326) fix: different behavior of blockquote between markdown and wysiwyg (#325)

seonim-ryu pushed a commit that referenced this pull request Jan 6, 2020

fix: different behavior of blockquote between markdown and wysiwyg (#325

6ecb9b9

) (#326) fix: different behavior of blockquote between markdown and wysiwyg (#325)

seonim-ryu pushed a commit that referenced this pull request Feb 5, 2020

Escape <a> on space even if in nested tags

507fe3e

Fixes #326

seonim-ryu pushed a commit that referenced this pull request Feb 5, 2020

Escape <a> on space even if in nested tags

6fe16cf

Fixes #326

js87zz pushed a commit that referenced this pull request Jun 17, 2021

post process for tasks after paste or cut fixed #326

4e2434c

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: different behavior of blockquote between markdown and wysiwyg (#325) #326

fix: different behavior of blockquote between markdown and wysiwyg (#325) #326

sohee-lee7 commented Nov 12, 2018 •

edited

Loading

kyuwoo-choi left a comment •

edited

Loading

kyuwoo-choi Nov 13, 2018

shiren Nov 13, 2018

sohee-lee7 Nov 13, 2018

kyuwoo-choi Nov 13, 2018

sohee-lee7 Nov 13, 2018

kyuwoo-choi Nov 13, 2018

sohee-lee7 Nov 13, 2018

kyuwoo-choi Nov 13, 2018

seonim-ryu Nov 13, 2018

sohee-lee7 Nov 13, 2018

seonim-ryu commented Nov 13, 2018

shiren left a comment

shiren Nov 13, 2018

fix: different behavior of blockquote between markdown and wysiwyg (#325) #326

fix: different behavior of blockquote between markdown and wysiwyg (#325) #326

Conversation

sohee-lee7 commented Nov 12, 2018 • edited Loading

Please check if the PR fulfills these requirements

Description

kyuwoo-choi left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seonim-ryu commented Nov 13, 2018

shiren left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sohee-lee7 commented Nov 12, 2018 •

edited

Loading

kyuwoo-choi left a comment •

edited

Loading