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

feat: zoom feature [#35, #95, #403] #520

Closed
wants to merge 8 commits into from
Closed

Conversation

lja1018
Copy link
Contributor

@lja1018 lja1018 commented Feb 3, 2021

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

  • 작업 내역

    • 확대/축소(줌) 기능 구현
    • 확대된 캔버스 드래그(핸드툴) 기능 구현
    • 확대된 캔버스의 가상 스크롤바 구현
  • 기능

    • 확대 시 드래그를 통해 영역을 지정하여 확대
      • 배율 값 기본 1.0에서 1씩 증가하며 최대 배율은 5.0으로 지정
      • 기본 배율 값이 아닌 상태에선 가상 스크롤바가 해당 캔버스의 크기 비율을 가지며 나타남
    • 축소 시 확대할 때의 기준점을 가져와 축소
      • 배율 값이 1씩 감소하며 최소 배율은 1.0으로 지정
    • 드래그 기능(핸드툴)을 이용하여 확대된 캔버스를 드래그 가능
      • 캔버스 이동 시 가상 스크롤바도 함께 이동
  • 확대 기능

    • 화면-기록-2021-02-04-오후-3 55 05
  • 핸드툴

    • 화면-기록-2021-02-04-오후-3 56 14 (1)

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

@dongsik-yoo
Copy link
Contributor

테스트해볼 수 있는 데모가 있으면 좋겠네요.
환경이 어려우면 스샷이라도 좋습니다.

@junghwan-park
Copy link
Member

@lja1018 저도 같은생각이에요. 브랜치 checkout 하고 나서 테스트 하려면 어떻게 해야 하는지 가이드 해주거나요.

@lja1018
Copy link
Contributor Author

lja1018 commented Feb 4, 2021

@dongsik-yoo @junghwan-park
gif 업로드하였습니다!

@lja1018 lja1018 changed the title feat: zoom feature feat: zoom feature [#35, #95, #403] Feb 4, 2021
Copy link
Contributor

@dongsik-yoo dongsik-yoo left a comment

Choose a reason for hiding this comment

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

리뷰 완료합니다. 줌이 생각보다 섬세하게 구현해야 하는데 잘하셨습니다. 수고하셨어요.

  • 스크롤바를 마우스로 움직이거나 마우스 휠로 스크롤이 이동되는 기능도 있으면 좋겠네요.

src/js/action.js Show resolved Hide resolved
src/js/component/zoom.js Outdated Show resolved Hide resolved
src/js/component/zoom.js Outdated Show resolved Hide resolved
src/js/component/zoom.js Outdated Show resolved Hide resolved
src/js/component/zoom.js Outdated Show resolved Hide resolved
src/js/graphics.js Show resolved Hide resolved
src/js/ui/zoom.js Show resolved Hide resolved
src/js/util.js Show resolved Hide resolved
src/js/interface/drawingMode.js Show resolved Hide resolved
test/zoom.spec.js Outdated Show resolved Hide resolved
@junghwan-park
Copy link
Member

image
버튼 디자인이 정상적으로 적용 된 뒤에 merge 되어야 할 것 같습니다.

src/js/action.js Outdated Show resolved Hide resolved
src/js/consts.js Outdated Show resolved Hide resolved
@junghwan-park
Copy link
Member

junghwan-park commented Feb 4, 2021

image

Zoom을 한 상황에서 무한정 같은 방향으로 갈 수 있네요.
이미지 영역 밖을 나가지 못하게 제한이 되어야 할 것 같아요.

@junghwan-park
Copy link
Member

다중 모드 버그

AS-IS

  1. Hand 더블클릭
  2. Zoom In 클릭
  3. Zoom In + Hand 모드 진입�

TO-BE

마지막에 누른 모드만 반영되어야함.

@junghwan-park
Copy link
Member

@lja1018
Zoom In은 영역 선택인데, Zoom Out은 바로 Zoom Out이 되어서 뭔가 맞지 않는 기분이에요.
Zoom by Area 버튼을 따로 추가하는게 어떨까요?

Copy link
Member

@junghwan-park junghwan-park left a comment

Choose a reason for hiding this comment

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

코드 리뷰 완료입니다!
Zoom 기능 기다린 분들이 많을텐데 얼른 버튼 디자인도 나와서 적용되면 좋겠네요 ㅎㅎ

사용성 측면에서는 조금 더 고민이 필요할 것 같습니다.
저는 Zoom을 할 Area를 정해서 zoom 하는게 맞을지 잘 모르겠네요.
Zoom In 단추를 누르면 전체가 확대된다고 생각했는데 그게 아니라서 약간 어색했어요
지금은 Mouse Scroll 지원도 빠져있어서...
이미지를 크게 확대 시킨 뒤에, 작은 화면을 Hand 툴만으로 조금씩 이동시키기엔 불편하기도 하고요.

@ricardojlrufino
Copy link

waiting for this feature....

@ricardojlrufino
Copy link

Pan and Zoom using mouse gestures:
https://ricardojlrufino.github.io/tui.image-editor-zoom/

No changes in sources, i use a separete class:
https://github.com/ricardojlrufino/tui.image-editor-zoom/blob/master/js/panZoom.js

Only need: call new IEditorPanZoom(imageEditor).enable();

I haven't tested it on iPhone yet ...

  • Desktop/Browser:
    • Zoom on Mouse Whell
    • Pan ou Middle button click
  • Mobile:
    • Pinch zoom gestures
  • TODO:
    • Pan on Mobile (TODO need a custom UI button)

Base automatically changed from feat/v3.13.0 to master March 5, 2021 03:07
@lja1018 lja1018 closed this Mar 8, 2021
@lja1018 lja1018 deleted the feat/zoom-feature branch March 8, 2021 06:54
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.

None yet

4 participants