Skip to content

fix: useFullScreen escKey가 적용안되는 문제 제거#287

Merged
Dave352 merged 2 commits intomainfrom
issues/286
Jul 5, 2023
Merged

fix: useFullScreen escKey가 적용안되는 문제 제거#287
Dave352 merged 2 commits intomainfrom
issues/286

Conversation

@Dave352
Copy link
Copy Markdown
Contributor

@Dave352 Dave352 commented Jul 5, 2023

escKey를 눌렀을 때, event를 감지할 수 없는 문제가 있어서 수정했습니다

#286

@Dave352 Dave352 self-assigned this Jul 5, 2023
setFullScreen((cur) => !cur)
}, [isFullScreen])

useEffect(() => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

useEffect로 풀스크린 감지 되던가용?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

useEffect가 감지한다기보다는 fullscreenchange 이벤트가 풀스크린을 감지합니다.

window.addEventListener('fullscreenchange', 콜백함수)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

의존성 배열이 비어있다면 이 훅을 다시 호출할 때 도는 거잖아용 그럼 esc눌렀을 때 이 훅을 다시 호출하는 걱나요?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

솔직히 말해서, useEffect안에 넣는 이유는 모르겠는데,
제 추측으로는 마운트 될 때, 딱 한번 이벤트 리스너에 등록하기 위함이 아닌가 싶습니다.
클린업을 작성한 이유는 중복으로 등록되는걸 방지하기 위함인거 같고요.
그래서 의존성 배열 안에 값을 넣으면 안됩니다. 중복으로 이벤트가 발생합니다.�

즉 window.addEvent~~~('fullscreenchange')가 전체모드 이벤트를 감지해서 알아서 해줍니다.
정확한 동작원리는 모르겠습니다. 정보가 별로 없네요.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

풀스크린 모드 설정, 해제가 사용자 이벤트에 의해 일어나야 하는 일이면 useEffect를 쓰지 않는게 좋을 듯 합니다.
그냥 풀스크린 설정시키는 함수, 해제 시키는 함수를 만들어서 제공해주는게 좋지 않을까요?

https://react.dev/learn/you-might-not-need-an-effect#how-to-remove-unnecessary-effects
여기보니 유저 이벤트를 핸들링 하기 위해 effects를 사용할 필요가 없다고 합니당

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

on, off 함수를 만들어서 풀스크린 모드 버튼 클릭시 on 메서드를 실행하고, 해제 버튼을 클릭하거나 esc키를 눌렀을 때는 off 메서드를 실행시키는식으로요!

@Dave352 Dave352 changed the title fix: useFullScreen escKey가 적용안돼는 문제 제거 fix: useFullScreen escKey가 적용안되는 문제 제거 Jul 5, 2023
@Dave352 Dave352 merged commit 4b36fe5 into main Jul 5, 2023
@Dave352 Dave352 deleted the issues/286 branch July 5, 2023 07:51
@github-actions github-actions Bot mentioned this pull request Jul 5, 2023
if (document.fullscreenElement) {
// 전체모드 해제
document.exitFullscreen()
setFullScreen(false)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

return 이 빠진 것 같습니다

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.

3 participants