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

fix: left/right key works wrong around hr (fix: 543) #545

Merged
merged 3 commits into from
Jun 19, 2019

Conversation

sohee-lee7
Copy link
Contributor

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

fix: #543

구분선 주변에서 좌우 키로 이동 시 커서(range)가 <hr> 테그 양옆에 위치하게 되고 (실제 랜더링은 css의 영향으로 <hr> 바로 아래에 렌더링 된다.) 그 순간 텍스트를 입력하면 구분선 위아래에 텍스트가 입력됩니다.

이를 수정하기 위해 구분선을 아래와 같은 형태의 돔으로 운영하도록 변경하였습니다.

<div contenteditable="false">
  <hr contenteditable="false">
</div>

추가로 스콰이어에 의해서 보통의 텍스트를 나타내는 <div><div contenteditable="false">로 머지될 소지가 있으며, delete키와 backspace 키로 구분선을 지울 때 <div contenteditable="false"> 안에 있는 <hr contenteditable="false"> 만 지우는 문제가 있는데, 이는 스콰이어 쪽에서 contenteditable="false"에 대해 예외처리하도록 수정하였습니다. (sohee-lee7/Squire#7)


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

Sohee Lee added 2 commits June 18, 2019 20:04
So hr should be wrapped by div that has contenteditable as false.
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.

수고하셨어요

* @memberof WwHrManager
* @private
*/
_unwrapDivOnHr() {
_changeHRContenteditableFalse() {
Copy link
Member

Choose a reason for hiding this comment

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

contenteditable을 false로 바꾸는건 domUtils.createHorizontalRule가 하니까 이 메서드의 이름은 한 뎊스 더 추상화된 이름이 되어야할것 같아요..contenteditable을 false로 두는게 해결방법이 아니라 다른 방법으로 바뀌어도 이메서드의 이름은 수정할 필요가 없게 말이죠.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

아하 넵! 반영하겠습니다.

Copy link
Member

@junghwan-park junghwan-park left a comment

Choose a reason for hiding this comment

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

리뷰 완료입니다~~! 고생 많으셨어요 ㅎㅎ

@sohee-lee7 sohee-lee7 merged commit f14d984 into master Jun 19, 2019
@sohee-lee7 sohee-lee7 deleted the fix/cursor-wrong-rendering-around-hr branch June 19, 2019 07:30
kishorethecoder pushed a commit to kishorethecoder/tui.editor that referenced this pull request Jul 11, 2019
* fix: left/right key works wrong around hr

So hr should be wrapped by div that has contenteditable as false.

* chore: squire update for contenteditable=false

* refactor: reflect code review
kishorethecoder added a commit to kishorethecoder/tui.editor that referenced this pull request Dec 4, 2019
kishorethecoder added a commit to kishorethecoder/tui.editor that referenced this pull request Dec 4, 2019
seonim-ryu pushed a commit that referenced this pull request Jan 2, 2020
* fix: left/right key works wrong around hr

So hr should be wrapped by div that has contenteditable as false.

* chore: squire update for contenteditable=false

* refactor: reflect code review
seonim-ryu pushed a commit that referenced this pull request Jan 6, 2020
* fix: left/right key works wrong around hr

So hr should be wrapped by div that has contenteditable as false.

* chore: squire update for contenteditable=false

* refactor: reflect code review
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Left and right key works wrong around hr in wysiwyg
3 participants