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(VsModal): move esc eventListener from composable to store #284

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

smithoo
Copy link
Collaborator

@smithoo smithoo commented Oct 7, 2024

Type of PR (check all applicable)

  • Fix Bug (fix)

Summary

esc eventListener 중복 등록 이슈 버그 수정

Description

  • esc-composable mount, unmount 시점에 eventListener를 다루던 부분을 제거합니다 (이벤트 중복 등록)
  • 대신 esc-store에 event를 붙이는 부분이 추가 됩니다 (추후에 android back button 기능도 추가될 예정)

@smithoo smithoo requested a review from yeriiiii October 7, 2024 09:48
@smithoo smithoo self-assigned this Oct 7, 2024
push(id: string) {
this.idStack.push(id);
constructor() {
document.addEventListener('keydown', (event: KeyboardEvent) => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

딱 한 군데서만 event를 등록해야 하는데 그 지점이 store 밖에 잡히는 곳이 없어서 일단 여기에 작성했습니다ㅠ
Vlossom bootstrap 시점에 라이프사이클 함수를 파야할지 고민되는 지점입니다;;;
일단 버그 수정 위해서 여기 작성하고 설계를 고민해보는게 좋을 것 같아요

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.

1 participant