[FE] fix : 형광펜 위치,모바일 동작 오류 수정 #840

BadaHertz52 · 2024-10-12T11:33:55Z

resolves [FE] 형광펜 위치 오류 및 드래그와 클릭 삭제 버튼이 동시에 생기는 오류를 수정한다 #838

🚀 오류 수정

형광펜 버튼인 에디터 하단 영역을 벗어나거서 잘리거나 버튼 그림자가 자리는 오류 수정

드래그를 통한 삭제와 클릭을 통한 삭제가 mouseup, click으로 이루어져 두 이벤트 동작의 중첩으로 인해 드래그 삭제 버튼과 클릭 삭제 버튼이 같이 보이는 오류 수정

터치가 안되는 브라우저의 경우, 하이라이트된 영역을 길게 클릭해서 삭제 버튼(쓰레기통 아이콘 버튼)이 나오게 했습니다
터치가 되는 브라우저의 경우, 하이라이트 영역 클릭 시 글자가 같이 선택되는 이슈가 있어서 이때는 touchmove이벤트를 통해 슬라이드 동작을 통해 삭제 버튼이 나오게 처리했습니다

모바일에서 형광펜을 나타내는 동작이 잘 안되는 오류 수정

모바일 브라우저 기본 동작으로 인한 touch 이벤트 작동안하는 오류

모바일의 경우 touchstart, toucheend로 형광펜 버튼이 나타나도록 시도했지만 , 터치 기반 장치에서 텍스트 선택과 관련된 기본 동작이 같이 이루어지고 기본 동작인 텍스트 선택이 우선순위라 touchstart와 touchend가 제대로 작동하지 않았습니다
이를 해결하기 위해 selectionchange 이벤트를 사용해 버튼이 나타날 수 있게했습니다.

터치가 안되는 웹 브라우저에서 selectionchage 이벤트를 사용하지 않은 이유?

selectionchage는 브라우저에 선택 사항이 있을 때마다 일어나기 때문에, 드래그로 선택 영역을 바꿀 때 마다 버튼 위치가 변경되어서 레이아웃 시프트가 발생합니다.

모바일 브라우저에서는 해당 현상이 거스리지 않는데 웹 브라우저에서는 체감될 만큼의 화면 끊김이 발생했습니다.
관련해서 찾아보니, 모바일 브라우저는 터치 기반 상호작용과 같은 자주 발생하는 동작에 최적화되어서 레이아웃 시프트와 같은 UI 변화를 최대한 부드럽게 처리하도록 설계되어 있다고 합니다

그래서 웹 브라우저에서는 mouse이벤트, 모바일에서는 selectionchange 이벤트로 형광펜 버튼을 보이고 숨기도록 했습니다

모바일에서 글자 선택 시 나오는 브라우저 메뉴(복사,열기등등)과 선택 좌우에 나오는 아이콘(?)으로 버튼이 가려지는 오류 수정

브라우저 메뉴에 가려지는 오류

글자 선택 시 나오는 아이콘?에 가려기는 오류

✅ 수정된 모습

📝 어떤 부분에 집중해서 리뷰해야 할까요?

형광펜 기능을 웹,모바일에서 실행해보시고 오류나면 알려주세요. 하나씩 잡아서 기능 보완하겠습니다

📚 참고 자료, 할 말

형광펜 리팩토링은, 리뷰 작성 페이지 선택 답변 수정하고 (형광펜 리팩토링과 토스트 모달 작업 충돌을 막기 위해) 토스트 모달 사항 보면서 진행하겠습니다

- 터치 가능한 브라우저에서 글자 선택 시 나타나는 브라우저 메뉴와 드래그 끝 방향에 나오는 아이콘으로 인해, 드래그 시 버튼이 가려지는 오류가 발생해 이를 해결 - isTouchDevice를 유틸함수로 분리

- 그림자 너비 상수화, 버튼 높이 상수화를 사용하도록 파라미터 수정

chysis · 2024-10-15T05:00:45Z

frontend/src/components/highlight/HighlightEditor/index.tsx

+  const addHighlightEvent = () => {
+    document.addEventListener('mousedown', hideHighlightButton);
+    document.addEventListener('mouseup', showHighlightButton);
+    // NOTE: 터치가 가능한 기기에서는 touchstart, touchend 보다 selectionchange를 사요앟는 게 오류가 없음


사소한 오타 제보합니다..!

수정했습니다

chysis · 2024-10-15T08:10:25Z

frontend/src/utils/touchDevice.ts

+const isTouchDevice = () => {
+  return 'ontouchstart' in window || navigator.maxTouchPoints > 0 || window.matchMedia('(pointer: coarse)').matches;
+};


여러 방법으로 터치 지원 디바이스인지 확인할 수 있군요 😮

와우...... 바다 덕분에 많은 걸 배우네요 🤔

soosoo22

빠르게 버그를 찾아내서 수정하는 바다의 민첩함.... 꼼꼼함....
선.생.이라는 네임이 너무나 잘 어울리십니다.. 바선생. 소소하게 수정되었으면 하는 부분 코멘트로 남겼습니다! 확인 부탁드릴게요!

`HighlighterButton` 윗 부분이 잘리고 있어서 위치를 살짝만 아래로 내려도 괜찮을 것 같아요! 그리고 지우기 버튼과 삭제하기 버튼 위치가 동등한 라인에 있으면 좋을 것 같아요😊 어떨 땐 멀리 떨어졌다가 또 어떨 땐 겹쳐 보이더라구요.

soosoo22 · 2024-10-15T23:39:31Z

frontend/src/components/highlight/HighlightButton/index.tsx

  position: Position;
-  addHighlight: () => void;
+  addHighlightByDrag: () => void;


저희 컨벤션에 맞춰 handle- 로 함수명을 변경하는 건 어떨까요

함수명에 click,change등 이벤트가 들어갈 때
onClick-,onChange- 식으로 선언하면, onClick,onChange 속성과 헷갈리기 때문에
handle-을 쓰자고 한 것으로 기억합니다

컴포넌트명으로 역할을 판단할 수 있게 네이밍했지만, props로 주는 핸들러에서 다시 한번 더 역할을 선언적으로 알 수 있게 해줬어요 합성 컴포넌트라 더 역할이 구분가도록 했어요

ImxYJL

궁금한 점 코멘트로 남겼습니다! 추후 금방 어프룹할게요

ImxYJL · 2024-10-16T06:11:27Z

frontend/src/components/highlight/HighlightButton/style.ts

  padding: 0.5rem 0.8rem;

  background-color: ${({ theme }) => theme.colors.white};
  border-radius: ${({ theme }) => theme.borderRadius.basic};
-  -webkit-box-shadow: 0 0 1.4rem -0.2rem #343434;
-  box-shadow: 0 0 1.4rem -0.2rem #343434;
+  -webkit-box-shadow: 0 0 ${() => `${HIGHLIGHT_BUTTON_SIZE.shadow / 10}rem`} -0.2rem #343434b8;


상수화 굿굿!!

ImxYJL · 2024-10-16T06:20:08Z

frontend/src/mocks/handlers/index.ts

 import reviewHandler from './review';

-const handlers = [...reviewHandler, ...groupHandler, ...collectionHandler];
+const handlers = [...reviewHandler, ...groupHandler, ...highlightHandler];


변수명이 더 기능을 묘사하는 방식으로 바뀌었네요

처음에 만들때는 모아보기 페이지의 api도 collectionHandler에서 하지 않을까하고 열어놨어요.
모아보기 api 구현을 보니 review에서 추가하는 방향으로 진행해서 collectionHandler 이름을 사용할 이유가 없어졌고
리뷰와 연관된 api이기는 하지만 review api는 다른 리뷰어가 써준 데이터라면, 형광펜은 리뷰어가 만드는 데이터?라는 느낌이 들고 highlightHandler로 따로 빼도 될 것 같아서 핸들러명만 바꿨어요

바뀐 게 더 명확해서 좋아요~~!

ImxYJL · 2024-10-16T06:28:56Z

frontend/src/hooks/highlight/useLongPress.tsx

+  longPressDuration?: number;
+}
+
+const useLongPress = ({ handleLongPress, longPressDuration = 500 }: UseLongPressProps) => {


이렇게 단순 mouse/touch 이벤트만을 사용하지 않고 longPress를 따로 다루는 이유가, 모바일 환경에서 touchEvent의 기본 동작(모바일 브라우저의 메뉴 띄우기)과 형광펜을 위한 목록 띄우기를 분리하기 위해서인가요?
이벤트를 mouse와 touch 둘 다 받는 걸 보면 아닌 것 같긴 하지만요 😅

올리가 추측한게 맞아요
pr에 썼지만 글자를 선택하기 위한 터치 이벤트 시 브라우저는 터치보다 글자를 선택하는 이벤트를 우선시로 해서, 일반적인 터치와 다른 방식으로 하이라이트 영역을 지우는 버튼을 띄워야해요.

다만, 터치가 되는 모바일 브라우저의 경우 길게 영역을 누르면 글자 선택이 되기 때문에 touchMove라는 변칙 이벤트를 사용해 길게 누르되 일반적인 한번의 터치, 길게 누르는 터치와 구분될 수 있게 했어요. 또한 슬라이드 방식으로 버튼을 삭제하는데 모바일 사용자에게는 친숙한 액션이라 해당 방법을 사용했습니다.

아무래도 터치가 되냐에 따라 이벤트를 다르게 적용해서, 해당 훅의 이름이 걱정이기는 했어요.
touchMove의 경우도 삭제를 원하는 액션인지 판단하기 위해 일정 시간의 머무름이 필요하기 때문에
해당 훅 이름을 사용하기로 했어요,

모바일에서 contextmenu 이벤트를 사용하면 longPress 이벤트인지를 감지한다고 하는데, 손수 useLongPress를 구현한 이유가 궁금합니당
형광펜 기능을 위한 이벤트를 여러 종류 사용하는 것보다 최소한의 이벤트만을 남기고 커스텀하는 게 낫다고 생각하신 걸까요? 아니면 contextmenu 이벤트에 또다른 문제가 있는지 궁금해요

결론적으로 말하면 터치인 모바일 브라우저의의 글자 선택 방식때문이에요 .

모바일에서 글자 선택 방법

길게 화면을 누른다 (이게 많이 사용하는 방법이라고 생각해서, 이 기준으로 만들었어요)

더블 클릭 후 드래그

문제: 드래그를 통한 삭제와 하이라이트 된 영역 선택을 통한 삭제 기능 충돌

찾아본 결과, contextmenu는 브라우저의 컨텍스트 메뉴를 위한 것이지, longPress를 위한 것은 아니라고 이해했어요.
(웹 브라우저는 오른쪽 클릭으로 열리고, 모바일은 길게 터치 시 나옴)

contextmenu를 통해서 하이라이트 된 영역 삭제를 진행하면, 길게 터치 시 글자들도 같이 선택돼요. 모바일은 selectionChange를 통해서 글자 선택을 감지하고 있기 때문에 이 경우 드래그를 통한 삭제 버튼과 하이라이트 된 영역 선택을 통한 삭제 버튼이 동시에 활성화돼요 (현재 드래그를 통한 삭제거 간편한 삭제 방식이라 이를 더 많이 사용할 거라는 판단 해, 드래그 버튼 등장 시 선택을 통한 버튼은 언마운트 되게 처리 중 입니다)

즉, 길게 터치 시 글자들이 선택되는 모바일 특성 상 길게 누르는 동작은 두 개의 삭제버튼을 활성화하는 거죠
사용자 입장에서는 영역을 선택 해 삭제하고 싶은데, 글자들이 선택되어서 드래그 삭제 버튼이 활성화되는 불편함이 있어요.

영상을 보면 더 와닿을거에요 영상에서는 선택 시, 하이라이트 된 영역이 모두 선택되었지만 하이라이트 중 일부 영역의 글자만 선택되는 경우도 있었어요

Screen_Recording_20241016_235036_Chrome.mp4

추가 상황

contextmenu 이벤트를 사용해, 터치 브라우저에서 형광펜 에디터 활성화 시, 길게 터치하면 생기는 컨텍스트 메뉴가 나타나지 않도록 처리했습니다

- HighlightEditor 아래로 위치 변경

ImxYJL

형광펜 만지느라 넘 수고 많아요~~~!

…eview-me into fe/fix/838-highlighter-error

- 구현 이유 : api 오류 시, isEditable 상태가 초기화 됨, 상태가 초기화되더라도 이전 상태를 저장해 반영할 수 있도록 기능 구현

chysis

모바일 테스트는 오늘 안에 해보겠습니다

우선 Approve할게요👍

* fix : 토글 버튼 위치가 editor 하위 영역을 벗어날 경우 자리는 오류 수정 * refactor: 타입명 변경(EditorSelctionInfo -> SelectionInfo) * refactor: 변수명 변경(block,Block ->line,Line) * refactor: 드래스 시, 나오는 버튼명 변경(dragHighlightButton) * fix: 하이라이트 영역을 길게 클릭/터치해야 해당 하이라이트를 삭제할 수 있도록 변경 * fix:터치 가능한 브라우저에서 드래그 시 버튼 위치 수정 - 터치 가능한 브라우저에서 글자 선택 시 나타나는 브라우저 메뉴와 드래그 끝 방향에 나오는 아이콘으로 인해, 드래그 시 버튼이 가려지는 오류가 발생해 이를 해결 - isTouchDevice를 유틸함수로 분리 * fix: 버튼 그림자가 에디터 영역 밖으로 나가서 잘리는 오류 수정 - 그림자 너비 상수화, 버튼 높이 상수화를 사용하도록 파라미터 수정 * refactor: document에 형광펜 관련 이벤트 적용 및 삭제하는 함수 생성 * refactor: 형광펜에 관한 api 파일명을 collection -> highlight로 변경 * chore: 오타 수정 및 함수명에 Block -> Line으로 수정 * chore: 오타 수정 * chore: EditorTestPage 삭제 * refactor: 하이라이트 훅,컴포넌트 폴더 구조 변경 * refactor: 하이라이트 훅 폴더 위치 변경 - HighlightEditor 아래로 위치 변경 * feat: longPress 삭제버튼 span의 가운데 위치하도록 변경 * feat: 터치 브라우저에서 길게 누를 때 컨텍스트 메뉴 나오지 않게 이벤트 추가 * feat: 터치 관련 클릭 버튼 위치 변경 사항 삭제 * feat: useEditabelState훅 구현 및 isEditable 상태를 세션 스토리지에 저장/삭제하는 기능 추가 - 구현 이유 : api 오류 시, isEditable 상태가 초기화 됨, 상태가 초기화되더라도 이전 상태를 저장해 반영할 수 있도록 기능 구현 * fix: 하이라이트 훅 status 복원 * chore: 불필요한 코드 삭제 * refactor: useHighlightEventListener 훅 생성

BadaHertz52 added 8 commits October 12, 2024 16:50

fix : 토글 버튼 위치가 editor 하위 영역을 벗어날 경우 자리는 오류 수정

0c0e7d9

refactor: 타입명 변경(EditorSelctionInfo -> SelectionInfo)

ab5d3e7

refactor: 변수명 변경(block,Block ->line,Line)

ee8ba43

refactor: 드래스 시, 나오는 버튼명 변경(dragHighlightButton)

3502c2b

fix: 하이라이트 영역을 길게 클릭/터치해야 해당 하이라이트를 삭제할 수 있도록 변경

c69f0b5

fix: 버튼 그림자가 에디터 영역 밖으로 나가서 잘리는 오류 수정

00affbf

- 그림자 너비 상수화, 버튼 높이 상수화를 사용하도록 파라미터 수정

refactor: document에 형광펜 관련 이벤트 적용 및 삭제하는 함수 생성

04a4a13

BadaHertz52 added 🍀 FE 🐛 버그 🚀 기능 labels Oct 12, 2024

BadaHertz52 added this to the 6차 스프린트: 최종장 milestone Oct 12, 2024

BadaHertz52 requested review from chysis, soosoo22 and ImxYJL October 12, 2024 11:33

BadaHertz52 self-assigned this Oct 12, 2024

BadaHertz52 linked an issue Oct 12, 2024 that may be closed by this pull request

[FE] 형광펜 위치 오류 및 드래그와 클릭 삭제 버튼이 동시에 생기는 오류를 수정한다 #838

Closed

BadaHertz52 added 2 commits October 13, 2024 16:56

refactor: 형광펜에 관한 api 파일명을 collection -> highlight로 변경

7183ee7

chore: 오타 수정 및 함수명에 Block -> Line으로 수정

f436518

chysis reviewed Oct 15, 2024

View reviewed changes

soosoo22 approved these changes Oct 15, 2024

View reviewed changes

chore: 오타 수정

e1d8b27

ImxYJL reviewed Oct 16, 2024

View reviewed changes

BadaHertz52 requested review from chysis and ImxYJL October 16, 2024 07:18

BadaHertz52 added 5 commits October 16, 2024 23:19

chore: EditorTestPage 삭제

243686d

refactor: 하이라이트 훅,컴포넌트 폴더 구조 변경

459badb

refactor: 하이라이트 훅 폴더 위치 변경

7aa297b

- HighlightEditor 아래로 위치 변경

feat: longPress 삭제버튼 span의 가운데 위치하도록 변경

086a54d

feat: 터치 브라우저에서 길게 누를 때 컨텍스트 메뉴 나오지 않게 이벤트 추가

174e1f4

feat: 터치 관련 클릭 버튼 위치 변경 사항 삭제

dec284a

ImxYJL approved these changes Oct 17, 2024

View reviewed changes

BadaHertz52 added 4 commits October 17, 2024 16:47

Merge branch 'develop' of https://github.com/woowacourse-teams/2024-r…

9861618

…eview-me into fe/fix/838-highlighter-error

feat: useEditabelState훅 구현 및 isEditable 상태를 세션 스토리지에 저장/삭제하는 기능 추가

8398519

- 구현 이유 : api 오류 시, isEditable 상태가 초기화 됨, 상태가 초기화되더라도 이전 상태를 저장해 반영할 수 있도록 기능 구현

fix: 하이라이트 훅 status 복원

9767e7a

chore: 불필요한 코드 삭제

15aa2f0

chysis approved these changes Oct 17, 2024

View reviewed changes

refactor: useHighlightEventListener 훅 생성

8917096

BadaHertz52 merged commit b3bbe2b into develop Oct 17, 2024
1 check passed

donghoony deleted the fe/fix/838-highlighter-error branch November 17, 2024 08:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FE] fix : 형광펜 위치,모바일 동작 오류 수정 #840

[FE] fix : 형광펜 위치,모바일 동작 오류 수정 #840

BadaHertz52 commented Oct 12, 2024 •

edited

Loading

chysis Oct 15, 2024

BadaHertz52 Oct 16, 2024

chysis Oct 15, 2024

soosoo22 Oct 15, 2024

soosoo22 left a comment •

edited

Loading

soosoo22 Oct 15, 2024

BadaHertz52 Oct 15, 2024

ImxYJL left a comment

ImxYJL Oct 16, 2024

ImxYJL Oct 16, 2024

BadaHertz52 Oct 16, 2024 •

edited

Loading

ImxYJL Oct 16, 2024 •

edited

Loading

ImxYJL Oct 16, 2024

BadaHertz52 Oct 16, 2024

ImxYJL Oct 16, 2024

BadaHertz52 Oct 16, 2024 •

edited

Loading

ImxYJL left a comment

chysis left a comment

[FE] fix : 형광펜 위치,모바일 동작 오류 수정 #840

[FE] fix : 형광펜 위치,모바일 동작 오류 수정 #840

Conversation

BadaHertz52 commented Oct 12, 2024 • edited Loading

🚀 오류 수정

형광펜 버튼인 에디터 하단 영역을 벗어나거서 잘리거나 버튼 그림자가 자리는 오류 수정

드래그를 통한 삭제와 클릭을 통한 삭제가 mouseup, click으로 이루어져 두 이벤트 동작의 중첩으로 인해 드래그 삭제 버튼과 클릭 삭제 버튼이 같이 보이는 오류 수정

모바일에서 형광펜을 나타내는 동작이 잘 안되는 오류 수정

모바일 브라우저 기본 동작으로 인한 touch 이벤트 작동안하는 오류

터치가 안되는 웹 브라우저에서 selectionchage 이벤트를 사용하지 않은 이유?

모바일에서 글자 선택 시 나오는 브라우저 메뉴(복사,열기등등)과 선택 좌우에 나오는 아이콘(?)으로 버튼이 가려지는 오류 수정

📝 어떤 부분에 집중해서 리뷰해야 할까요?

📚 참고 자료, 할 말

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

soosoo22 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ImxYJL left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BadaHertz52 Oct 16, 2024 • edited Loading

Choose a reason for hiding this comment

ImxYJL Oct 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BadaHertz52 Oct 16, 2024 • edited Loading

Choose a reason for hiding this comment

모바일에서 글자 선택 방법

문제: 드래그를 통한 삭제와 하이라이트 된 영역 선택을 통한 삭제 기능 충돌

추가 상황

ImxYJL left a comment

Choose a reason for hiding this comment

chysis left a comment

Choose a reason for hiding this comment

BadaHertz52 commented Oct 12, 2024 •

edited

Loading

soosoo22 left a comment •

edited

Loading

BadaHertz52 Oct 16, 2024 •

edited

Loading

ImxYJL Oct 16, 2024 •

edited

Loading

BadaHertz52 Oct 16, 2024 •

edited

Loading