Split functionality of TouchZoomRotateHandler
Nobody has claimed this yet.
- Dominant language
- TypeScript
- Stars
- 12.4k
- Forks
- 2.4k
- PR merge metrics
- No merged PRs in 30d
Description
## Motivation
Currently the [TouchZoomRotateHandler](https://github.com/mapbox/mapbox-gl-js/blob/b9e10b939c6a3fe5d7ecac209f751b4871970ede/src/ui/handler/touch_zoom_rotate.js) handles two distinct, but related, types of interaction: touch rotate, and touch rotate + zoom. Touch rotate + zoom is configurable when creating a map by setting the property `touchZoomRotate`, but touch rotate is only accessible by invoking `map.touchZoomRotate.disableRotate()`. This results in a confusing an inconsistent API because touch rotate is distinct from touch zoom + rotate, yet is only configurable via an API that shares a name with the latter.
## Design Alternatives
There are two ways I could see fixing this API issue.
1. Update the initialization of `TouchZoomRotateHandler` to accept a parameter like `canRotate`. In [enable()](https://github.com/mapbox/mapbox-gl-js/blob/b9e10b939c6a3fe5d7ecac209f751b4871970ede/src/ui/handler/touch_zoom_rotate.js#L52) simply add a line to read `options.canRotate` and set `this._rotationDisabled`.
2. Split touch rotate and touch rotate + zoom into two different event handlers. Create a new handler named `TouchRotateHandler`, which could be an extension of `TouchZoomRotateHandler`.
## Design
While option 1 is the easiest to implement, option 2 would create a cleaner API in my mind, as it would make it easier to set a property like `touchRotate` while initializing a map. This language and behavior would be more consistent with the rest of Mapbox GL JS.
### Mock-Up
If the proposed change was made, disabling touch rotation on a map would occur by setting the property `touchRotate` to `false` during map creation, or by invoking `map.touchRotate.disable()`.
### Concepts
As previously mentioned, the primary motivation for this change is to create a more consistent terminology and API. The current behavior is a bit of an outlier and difficult to discover.
Is this a change the Mapbox GL JS team would be interested in making? If so, I could submit a pull request proposal.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by reading src/ui/handler/touch_zoom_rotate.js, especially enable(), and the map initialization options described in the issue. Resolve whether to add a separate TouchRotateHandler or a canRotate option, then verify that touch rotation can be configured consistently at initialization and through a dedicated API.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- frontend
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 35/100