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: editors with drop-down layer to follow scrolling #1542

Merged
merged 5 commits into from
Jan 7, 2022

Conversation

jajugoguma
Copy link
Contributor

Please check if the PR fulfills these requirements

  • It's submitted to right branch according to our branching model
  • It's right issue type on 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 description for the breaking change

Description

  • Fixed that where the position of the drop-down layer in the editor did not move when scrolling.
    • The drop-down layer moves as you scroll.
    • Hides the drop-down layer when the editing cell is moved out of the viewport by scrolling.

As-Is

To-Be


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

Copy link

@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.

리뷰 완료합니다.

packages/toast-ui.grid/cypress/integration/editor.spec.ts Outdated Show resolved Hide resolved
packages/toast-ui.grid/cypress/integration/editor.spec.ts Outdated Show resolved Hide resolved
packages/toast-ui.grid/src/helper/common.ts Outdated Show resolved Hide resolved
Copy link

@adhrinae adhrinae left a comment

Choose a reason for hiding this comment

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

리뷰 완료합니다.

packages/toast-ui.grid/cypress/integration/editor.spec.ts Outdated Show resolved Hide resolved
packages/toast-ui.grid/src/editor/dom.ts Outdated Show resolved Hide resolved
packages/toast-ui.grid/src/editor/dom.ts 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.

늦었지만 리뷰 완료입니다!

packages/toast-ui.grid/src/editor/dom.ts Outdated Show resolved Hide resolved
return start <= value && value <= end;
}

export function pixelToNumber(pixelString: string) {
Copy link
Contributor

Choose a reason for hiding this comment

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

string대신 템플릿 리터럴 타입쓰면 될 것 같은데..생각해보니 ts 버전이 낮아 안되겠네요..

packages/toast-ui.grid/src/view/editingLayer.tsx Outdated Show resolved Hide resolved
Comment on lines 177 to 187
if (this.editor?.moveDropdownLayer) {
this.editor.moveDropdownLayer({
bodyHeight,
bodyWidth,
bodyScrollTop,
bodyScrollLeft,
headerHeight,
leftSideWidth,
...this.initBodyScroll,
});
}
Copy link
Contributor

Choose a reason for hiding this comment

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

엔터키로 편집 종료하거나 탭키로 에디팅 레이어 이동할 때는 문제 없나요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

키보드 동작이 정상적으로 동작하지 않아 원인을 알아보니, visibility: hidden;이나 display: none;처럼 DOM 상에 보여지지 않는 요소에는 포커스를 줄 수 없다는 것을 알았습니다.
실제로 에디팅 중에 스크롤을 이용해 에디터를 보이지 않게 설정하면 editingLayer`가 포커스를 잃는 것을 확인했습니다.

이를 해결하기 위해서는 에디터를 DOM 상에서 보이지만, 실제로는 보이지 않게 해야했습니다. 이를 위해 드롭다운 레이어의topleft0으로 설정해 그리드 바디 좌측 상단에 위치하도록 하고 z-index-100을 할당해 그리드 셀 아래로 숨겨지도록 했습니다.

결과적으로 보여지는 화면은 같지만, 키보드 동작은 정상적으로 수행할 수 있었습니다. 이런 방법을 이용해도 괜찮을까요?

Copy link
Contributor

Choose a reason for hiding this comment

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

@jajugoguma
stack context나 위치 자체를 바꾸기 보다 opacity로 제어하는게 제일 나을 것 같은데 어떤가요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

처음에는 opacity를 이용해 제어했었습니다.

그러나 드롭다운 레이어가 보이지만 않을 뿐 실제로를 그 위치 그대로 있어서 위 그림과 같이 드롭다운 내 요소가 선택되어 값이 변하는 것을 볼 수 있었습니다. 그래서 stack context를 이용해 드롭다운 레이어가 선택되지 않도록 하고, 보이지 않는다면 레이어의 위치를 계속 이동시킬 필요가 없다고 생각해서 top: 0px; left: 0px;에 고정시키는 방법을 사용해보았습니다.

@jajugoguma jajugoguma merged commit 95eb78f into master Jan 7, 2022
@jajugoguma jajugoguma deleted the fix/editor-with-drop-down-layer branch January 7, 2022 10:25
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.

4 participants