Skip to content
This repository has been archived by the owner on Apr 25, 2023. It is now read-only.

fix: select not working after pinch event on ipad #290

Merged
merged 9 commits into from
Aug 4, 2022

Conversation

airslice
Copy link
Contributor

@airslice airslice commented Aug 3, 2022

Overview

Touch to select does not work after pinch event on iPad.

This bug comes from unproper handle of pinch event props.

The pinch events are added with mouse events as a nice-to-have feature. Since the data type definiation is same with the mouse event props i didn't pay additional attention on the data but it turns out to be different.

After discuss with Red we decide to remove pinch events, the reasons are:

  • Pinch events are not mouse events and should not be mix together.
  • Pinch events carry different props which is different campared with mouse events.
  • The main goal of mouse events API is to detect the target position or layer, which also can be done on touch device.
  • The pinch event actually do zoom or rotate which are different usecases.

Also if we got some requirements in detail about pinch events, we can add them back with certain design.

What I've done

  • Remove pinchstart pinchmove pinchend events.
  • Add some unit test (not related just for pass the codecov test).

What I haven't done

How I tested

Select layer -> zoom in/out -> select layer on iPad visiting the deploy preview site.

  • It is really dificult to dev / test for mobile device as can not visit local site through LAN.

Screenshot

Which point I want you to review particularly

Memo

@netlify
Copy link

netlify bot commented Aug 3, 2022

Deploy Preview for reearth-web ready!

Name Link
🔨 Latest commit 2638e9e
🔍 Latest deploy log https://app.netlify.com/sites/reearth-web/deploys/62ea79009044f60008f43553
😎 Deploy Preview https://deploy-preview-290--reearth-web.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@codecov
Copy link

codecov bot commented Aug 3, 2022

Codecov Report

Merging #290 (2638e9e) into main (50f2b68) will increase coverage by 0.07%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #290      +/-   ##
==========================================
+ Coverage   14.30%   14.37%   +0.07%     
==========================================
  Files         481      481              
  Lines       46159    46163       +4     
  Branches      392      402      +10     
==========================================
+ Hits         6602     6636      +34     
+ Misses      39543    39513      -30     
  Partials       14       14              
Impacted Files Coverage Δ
...onents/molecules/Visualizer/Engine/Cesium/hooks.ts 0.00% <ø> (ø)
...nents/molecules/Visualizer/Engine/Cesium/index.tsx 0.00% <ø> (ø)
...molecules/Visualizer/Engine/Cesium/useEngineRef.ts 65.43% <ø> (+2.78%) ⬆️
src/components/molecules/Visualizer/Engine/ref.ts 0.00% <ø> (ø)
...components/molecules/Visualizer/Plugin/context.tsx 32.12% <ø> (+0.38%) ⬆️
...rc/components/molecules/Visualizer/Plugin/hooks.ts 5.95% <ø> (+0.07%) ⬆️
...rc/components/molecules/Visualizer/Plugin/types.ts 0.00% <ø> (ø)
...les/Visualizer/Engine/Cesium/useEngineRef.test.tsx 100.00% <100.00%> (ø)

@airslice airslice marked this pull request as ready for review August 3, 2022 13:51
@airslice airslice changed the title fix: select not working after zoom on ipad fix: select not working after pinch event on ipad Aug 3, 2022
@airslice airslice merged commit 8215043 into main Aug 4, 2022
@airslice airslice deleted the fix/select-not-working-after-zoom-on-ipad branch August 4, 2022 05:10
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