Skip to content
This repository was archived by the owner on Feb 9, 2023. It is now read-only.

Conversation

@josh-yun
Copy link

@josh-yun josh-yun commented Feb 28, 2019

Touch 영역을 생성할 때 touch 영역과 그영역에 해당하는 view에 같은 Tag를 달고, previewed state일 경우 tag를 체크하여 어떤 view를 기준으로 이동을 시킬것인지 확인하도록 바꿨습니다.

@josh-yun josh-yun requested a review from SeoJungHong February 28, 2019 07:43
touchView.setTag(tag);
mTouchViewMap.put(tag, touchView);
if (view instanceof HoverFrameLayout) {
((HoverFrameLayout) view).addOnPositionChangeListener(mOnLayoutChangeListener);

Choose a reason for hiding this comment

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

제가 이 과제 관련해서 조금 고민했던 것이 있는데요, 이번에 하신 버그 픽스와 직접 관련은 없지만 제가 발견한 개선 사항을 하나 제시하고 싶어요. 이 라인으로 인해 HoverFrameLayout 이 드래그 되면서 위치가 바뀌어도 그 위에 붙여뒀던 TouchView 또한 이를 따라오면서 변경이 가능하죠. 그런데 이로 인해 이벤트 전달이 좀 복잡합니다.

  1. Dragger 의 TouchView 에서 onTouch 호출
  2. DragListener 로 인해 FloatingTab 에 위치 이동 전달
  3. OnPositionChangeListener 를 통해 다시 Dragger의 TouchView로 전달
  4. moveTouchViewTo 메소드를 통해 TouchView 를 이동한 FloatingTab 위치로 이동시킴

기존 Hover 의 구현이 이렇게 되어 있었긴 하지만, 사실 TouchView 자체는 실시간으로 이동하지 않아도 FloatingTab 이 드래그 가능하게 만들기만 하면 충분한 기능 구현입니다. 어차피 한번 드래그 한 뒤로 Docking 로직이 호출되면 Dragger 를 reset 시키니까, 여기서 addOnPositionChangeListener 로 TouchView 에 리스너를 등록하지 않아도 정상적인 Dragger 로직을 구현할 수 있는 상황입니다. 그리고 이렇게 하면 위의 이벤트 처리하는 스텝이 반으로 줄어들어서 당장 눈으로도 훨씬 끊기지 않고 부드럽게 움직이네요. 이렇게 TouchView 의 position change listener 를 없애는 것에 대해서 어떻게 생각하시나요?

Copy link
Author

Choose a reason for hiding this comment

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

말씀하신대로 실시간으로 TouchView가 실시간으로 따라갈 필요는 없을 것 같네요. 하지만 OnPositionChangeListener를 그냥 제거하기만 하면, Landscape으로 변환되는 상황에서 TouchView가 업데이트가 안되는 문제가 있습니다. 그 부분은 onLayoutChangeListener를 등록하면 해결 될 것 같습니다.
view.addOnLayoutChangeListener(mOnLayoutChangeListener);

… 대한 touchView의 이동은 개별 view에서 관리하도록 수정
Copy link

@SeoJungHong SeoJungHong left a comment

Choose a reason for hiding this comment

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

LGTM!

@josh-yun josh-yun merged commit c5e72b2 into master Mar 4, 2019
@delete-merged-branch delete-merged-branch bot deleted the fix/preview_dragging_issue branch March 4, 2019 01:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants