Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: convert list copied from ms office to standard list element in wysiwyg #1153

Merged
merged 8 commits into from
Aug 24, 2020

Conversation

seonim-ryu
Copy link
Member

@seonim-ryu seonim-ryu commented Aug 19, 2020

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

Bug Fix

If you copy and paste the list from MS Office in WYSIWYG, it is pasted as plain text as follows.

wrong

In Chrome, Edge, and IE10, it is pasted as above. However, in IE11, the list is pasted as it is, but the list item with children is shown as newline as follows.

ie11-wrong

When pasting, the clipboard data copied from MS Office must be parsed and processed into a list element of the WYSIWIG editor. In MS Office, the copied list is created as a paragraph, and in this case, the list is created with the attribute value of a specific class (MsoListParagraph ) or style (mso-list).

In MS Office, the clipboard data is as follows when copied and pasted into the WYSIWIG editor.

<p class="MsoListParagraph" style="margin-left:40.0pt;mso-para-margin-left:0gd;
text-indent:-20.0pt;mso-list:l0 level1 lfo1">
    <span class="font" style="font-family:Wingdings">
        <span style="mso-list:Ignore">l
            <span class="font" style="font-family:&quot;Times New Roman&quot;">
                <span class="size" style="font-size:7pt">&nbsp; </span>
            </span>
        </span>
    </span>
    <span lang="KO">foo</span>
</p>
<p class="MsoListParagraph" style="margin-left:60.0pt;mso-para-margin-left:0gd;
text-indent:-20.0pt;mso-list:l0 level2 lfo1">
    <span class="font" style="font-family:Wingdings">
        <span style="mso-list:Ignore">n
            <span class="font" style="font-family:&quot;Times New Roman&quot;">
                <span class="size" style="font-size:7pt">&nbsp; </span>
            </span>
        </span>
    </span>
    <span lang="KO">bar</span>
</p>

List elements created after processing are as follows. Note that the WYSIWIG editor uses Squire. Because of the list structure created at this time, the child list (li > ul) is created as the next element, not as a child of the list item.

<ul>
    <li>foo</li>
    <ul>
        <li>bar</li>
    </ul>
</ul>

Result

Finally, the list should be pasted in the following format.

Chrome

chrome-correct

Edge

edge-correct

IE11

ie11-correct

IE10

ie10-correct


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

// In our viewer case, <p>aaa</p><p>bbb<p> have empty line becuase P tags have margin.
// In MS Office case, <p>aaa</p><p>bbb<p> do not have empty line becuase P tags means just one line.
if (!this._isFromMs(clipboardContainer.innerText)) {
if (isCopiedFromMso(clipboardContainer.innerHTML)) {
Copy link
Member Author

@seonim-ryu seonim-ryu Aug 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

참조 : 기존에 innerText로 체크했는데 이 경우 텍스트를 검사하는 것이기 때문에 태그에 포함된 mso- 프리픽스 값을 체크할 수 없어서 innerHTML로 수정함

Comment on lines 158 to 162
_listElementAid(container) {
const wwListManager = this.wwe.componentManager.getManager('list');

container.innerHTML = wwListManager.convertToArbitraryNestingList(container.innerHTML);
}
Copy link
Member Author

@seonim-ryu seonim-ryu Aug 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

참조 : IE11에서 MS Office의 리스트를 붙여넣기 하면 다른 브라우저와 다르게 리스트 엘리먼트로 데이터가 들어온다. 이 경우 Squire의 리스트 구조에 맞게 자식 리스트의 위치를 변경해줘야 한다. 이를 처리하기 위한 로직으로 추가함

  • 표준 리스트 마크업 구조
<ul>
    <li>
        foo
        <ul>
            <li>bar</li>
        </ul>
    </li>
    <li>baz</li>
</ul>
  • Squire 리스트 마크업 구조
<ul>
    <li>foo</li>
    <ul>
        <li>bar</li>
    </ul>
    <li>baz</li>
</ul>

Comment on lines -167 to -169
if ( !hasImage ) {
items = null;
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

참조 : 이 로직으로 인해서 Edge에서 type === 'text/html'의 클립보드 데이터를 가져오지 못한다. 주석 내용에 따르면 구형 Edge를 대응할 목적으로 보이는데 이 부분은 Squire 이슈로 등록하여 관리할 예정임

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 부분은 두레이 task에 같이 히스토리로 남겨놓는게 좋을 것 같습니다~!

Copy link
Member Author

@seonim-ryu seonim-ryu Aug 20, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

넵~ Squire에 이슈 등록하고 같이 업데이트 해둘게요~

Squire 등록 이슈 : fastmail/Squire#389

@seonim-ryu seonim-ryu added Feature New features to implement Mode: WYSIWYG Bug NHN and removed Feature New features to implement labels Aug 20, 2020
@seonim-ryu seonim-ryu changed the title feat: convert list paragraph copied from ms office to list element feat: convert list paragraph copied from ms office to list element in wysiwyg Aug 20, 2020
@seonim-ryu seonim-ryu changed the title feat: convert list paragraph copied from ms office to list element in wysiwyg feat: convert list copied from ms office to standard list element in wysiwyg Aug 20, 2020
Copy link
Contributor

@lja1018 lja1018 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[08/20] 리뷰완료
고생하셨습니다!

apps/editor/src/js/utils/wwPasteMsoList.js Outdated Show resolved Hide resolved
@seonim-ryu seonim-ryu added this to the 2.4.0 milestone Aug 20, 2020
Copy link
Member

@shiren shiren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

수고하셨어요

apps/editor/src/js/utils/wwPasteMsoList.js Outdated Show resolved Hide resolved
apps/editor/src/js/utils/wwPasteMsoList.js Outdated Show resolved Hide resolved
Copy link
Contributor

@js87zz js87zz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생하셨습니다!

@seonim-ryu seonim-ryu merged commit be8f7b3 into master Aug 24, 2020
@seonim-ryu seonim-ryu deleted the feat/convert-mso-list branch August 26, 2020 11:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants