DirectlyManipulable plan
AbandonedPublic

Authored by appsforartists on Aug 22 2016, 2:46 PM.

Details

Summary

Add Pinchable and Rotatable to build DirectlyManipulable

Rename to DirectManipulation family. Closes #10
Dont add gesture recognizer to target in GesturePerformer. Closes #9
Remove BlockGesturable. Closes #7
Remove Plan conformity for Gesturable. Closes #5

Diff Detail

Repository
rMDMDIRECTMANIPULATIONSWIFT material-motion/direct-manipulation-swift
Branch
develop
Lint
Lint OK
Unit
Unit Tests OK
rcameron retitled this revision from to Rename to DirectManipulation family. Closes #10 dont add gesture recognizer to target in GesturePerformer. Closes #9 Remove BlockGesturable. Closes #7 Remove Plan conformity for Gesturable. Closes #5.Aug 22 2016, 2:46 PM
rcameron updated this object.
rcameron edited the test plan for this revision. (Show Details)
Restricted Application added a project: Material Motion. · View Herald Transcript
rcameron planned changes to this revision.Aug 22 2016, 2:58 PM
rcameron retitled this revision from Rename to DirectManipulation family. Closes #10 dont add gesture recognizer to target in GesturePerformer. Closes #9 Remove BlockGesturable. Closes #7 Remove Plan conformity for Gesturable. Closes #5 to Internal cleanup.Aug 22 2016, 5:08 PM
rcameron updated this object.
rcameron updated this revision to Diff 6389.Aug 22 2016, 8:10 PM

Plans for Pinchable, Rotatable and DirectlyManipulable

rcameron retitled this revision from Internal cleanup to DirectlyManipulable plan .Aug 22 2016, 8:12 PM
rcameron updated this object.

This probably should've been broken up into multiple diffs.

Definitely :) Is up to you whether you'd still like to split it up; will increase the throughput of individual changes.

featherless requested changes to this revision.Aug 23 2016, 10:34 AM
featherless added a reviewer: featherless.
featherless added inline comments.
src/DirectManipulationMotionFamily.swift
23

Does this need to be implicitly unwrapped? If not, can we make it a non-optional type?

29

What required that this class be declared open?

91

If we can get type enforcement on the setter for these specific plans I think that would be a preferable "clean" API. If we can't use generics to do so then perhaps it's sensible to not have a common base class here. In other words, each plan is an NSObject that declares its gesture recognizer type explicitly.

103

Link to a github issue.

202

I don't think we'd ever considered the idea of performers directly creating other performers. This is interesting and likely needs to be thought about more.

https://github.com/material-motion/material-motion-starmap/issues/36

There are benefits to using composition however:

  1. The scheduler will only ever create one instance of a given performer type per target.
  2. Inspection tooling will better be able to represent the different plans/performers registered to the scheduler if everything is managed via the scheduler.
  3. If someone registers a DirectlyManipulable + a Pinchable plan, two PinchablePerformers will start affecting the target. This is likely not the desired behavior by the creator of the plans.

Concrete recommendation here is to convert this to the composition APIs. I've just landed D1476 which simplifies the API for emitting new plans.

This revision now requires changes to proceed.Aug 23 2016, 10:34 AM
rcameron added inline comments.Aug 23 2016, 1:31 PM
src/DirectManipulationMotionFamily.swift
23

I made it implicitly unwrapped to prevent needing to add an init() to Gesturable. Gesturable feels to me like it should be an abstract class, so I may try converting it to a protocol.

29

My thought was that third parties would subclass GesturePerformer down the line. open is now the correct access control level to allow subclassing outside of the module.

91

Agreed. I'll try to refactor using generics.

202

I'll switch over to using emitters.

rcameron updated this revision to Diff 6457.Aug 25 2016, 6:39 PM
rcameron retitled this revision from DirectlyManipulable plan to DirectlyManipulable plan.

Breaking this up into multiple diffs

rcameron planned changes to this revision.Aug 25 2016, 6:39 PM
appsforartists commandeered this revision.Mar 24 2017, 12:00 AM
appsforartists added a reviewer: rcameron.
Restricted Application added a reviewer: Material Motion. · View Herald TranscriptMar 24 2017, 12:00 AM
appsforartists abandoned this revision.Mar 24 2017, 12:00 AM