• Changed Files
  • library/src/main/java/com/google/android/material/motion/operators/CommonOperators.java

Implement delayBy operator
ClosedPublic

Authored by markwei on Apr 6 2017, 2:30 PM.

Diff Detail

Repository
rMDMSTREAMSANDROID reactive-motion-android
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
markwei created this revision.Apr 6 2017, 2:30 PM
Restricted Application added a reviewer: O2: Material Motion. · View Herald TranscriptApr 6 2017, 2:30 PM
Restricted Application added a project: Material Motion. · View Herald Transcript
Restricted Application added a reviewer: O6: Material Android platform reviewers. · View Herald Transcript
Restricted Application added a reviewer: Material Motion. · View Herald Transcript
Restricted Application failed to build Restricted Buildable!Apr 6 2017, 2:32 PM
featherless requested changes to this revision.Apr 8 2017, 8:09 PM
featherless added a subscriber: featherless.
featherless added inline comments.
library/src/main/java/com/google/android/material/motion/operators/CommonOperators.java
220

Ah I think I just noticed a potential bug: if we connect, disconnect, and then connect again before the first connection's delayed event fired, it's possible that both connections will fire. We may need to correlate the active subscriptions with the delayed invocations in order to ensure that we're only receiving values for the most recent subscription.

This revision now requires changes to proceed.Apr 8 2017, 8:09 PM
markwei updated this revision to Diff 10728.Apr 11 2017, 9:39 PM
  1. Do not use connected boolean. Remove runnables from the handler instead.
  2. Handle the same operator instance being applied to multiple streams.
markwei added inline comments.Apr 11 2017, 9:40 PM
library/src/main/java/com/google/android/material/motion/operators/CommonOperators.java
220

ah, these stateful operators are harder to write than naively thought. There are 2 problems:

  1. Using connected boolean is flawed, as you said. Instead, these runnables must be removed from the handler upon disconnect.
  2. The same delayBy() operation instance can be reused with multiple streams. This is an issue that only exists on android and must be handled correctly. Filed https://github.com/material-motion/material-motion-android/issues/70
Restricted Application failed to build Restricted Buildable!Apr 11 2017, 9:41 PM
featherless accepted this revision.Apr 12 2017, 1:38 AM
This revision is now accepted and ready to land.Apr 12 2017, 1:38 AM
This revision was automatically updated to reflect the committed changes.