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

Dispatching actions that have no reducers/middleware handling them #50

Closed
kentcb opened this issue May 15, 2018 · 5 comments
Closed

Dispatching actions that have no reducers/middleware handling them #50

kentcb opened this issue May 15, 2018 · 5 comments

Comments

@kentcb
Copy link

kentcb commented May 15, 2018

Hello,

This one is a doozy - strap yourself in!

I just found something the (super) hard way, and I want to record my thoughts before I leave work for the day. If necessary, I'm happy to put more effort in here and try to produce a succinct repro. Right now I need some validation/feedback on whether I'm doing something obviously wrong.

I was writing an automatic logout feature for my app, so if the user stops interacting for, say, 30 seconds, it logs them out. To achieve this, I wrote an InputDetector control like this:

class InputDetector extends StatefulWidget {
  final VoidCallback onInput;
  final Widget child;

  InputDetector({@required this.child, @required this.onInput})
      : assert(child != null),
        assert(onInput != null);

  @override
  State<StatefulWidget> createState() => new _InputDetectorState();
}

class _InputDetectorState extends State<InputDetector> {
  @override
  void initState() {
    super.initState();

    // Detect raw input from the keyboard (alas, only the hardware keyboard).
    RawKeyboard.instance.addListener(_onRawKeyEvent);
  }

  @override
  void dispose() {
    RawKeyboard.instance.removeListener(_onRawKeyEvent);

    super.dispose();
  }

  @override
  Widget build(BuildContext context) {
    var touchDetector = new Listener(
      behavior: HitTestBehavior.translucent,
      child: widget.child,
      onPointerDown: (_) => widget.onInput(),
      onPointerMove: (_) => widget.onInput(),
      onPointerCancel: (_) => widget.onInput(),
      onPointerUp: (_) => widget.onInput(),
    );

    return touchDetector;
  }

  void _onRawKeyEvent(RawKeyEvent event) => widget.onInput();
}

It works fine, apart from not being able to detect software keyboard input (but I'm chasing that up separately).

Anyway, I hook it into the root of my visual tree, and then dispatch an action whenever input is detected:

var inputDetector = new InputDetector(
  child: materialApp,
  onInput: () => store.dispatch(new DeferIdleLogout()),
);

My middleware then uses these actions to manage a timer, which will log the user out if it expires. I added a global flag to enable/disable that particular middleware, since it would be super annoying for developers to be logged out all the time whilst working on the app. The CI server substitutes in an appropriate value for the global prior to building, so there's no chance of developers accidentally disabling the feature.

This was all working wonderfully, except some of my widget tests started to fail. I dug in and found that all tests performing swipe interactions were failing. Weird. I tried running up the app and, sure enough, I couldn't swipe my Dismissible widgets any more. Wat.

After initially suspecting flutter was to blame - something to do with using a Listener above a Dismissible, I quickly dismissed this when an attempted repro worked fine. I had to whittle things down, control by control, line by line. It took quite a while, but I eventually narrowed it down to this code from above:

var inputDetector = new InputDetector(
  child: materialApp,
  onInput: () => store.dispatch(new DeferIdleLogout()),
);

Specifically, dispatching the action. If I commented out the dispatch and did nothing, it all worked fine.

Digging even further, I found that if I only forwarded onPointerCancel and onPointerUp, it worked. Forwarding either onPointerDown or onPointerMove exhibited the same problem. Now I started to suspect the frequency of the action dispatches or something. But then I tried only onPointerDown and it still didn't work. Wowsers. Now I started thinking maybe dispatching the action at the start of the interaction was somehow thwarting the interaction, even though nothing was handling that action.

Wait, I thought, nothing is handling the action. Maybe that's relevant (and maybe the issue title was a total spoiler). I enabled the middleware that dealt with this action (even though I don't ordinarily want it enabled for developers), and sure enough, it worked. Even with all input events forwarded, things were working fine.

So, um, sorry for the short story, but I'm really confused. I can see an easy way around this: instead of selectively registering the middleware, always register it and check the global value when handling the action. But I don't understand why this is happening. If an action has no reducer/middleware anywhere, would you expect Weird Things like this to occur?

Thanks for reading.

@brianegan
Copy link
Owner

Hey hey :) Sorry, back from a trip! Hrm, that's definitely an interesting case. Lemme see if I can write a couple test cases run into the same problem using that technique.

@brianegan
Copy link
Owner

Hey there -- I tried to reproduce this a couple ways on my end but couldn't quite get the bug to affect me.

I've pushed up an example of one attempt to this branch: https://github.com/brianegan/flutter_redux/blob/swipable-bug/example/counter/lib/main.dart

In this instance, I only have 1 reducer which handles a single increment action. In the onInput function of the InputDetector, I dispatch a String. The reducer needs to return something, so I return the previous state when the String is dispatched.

Even if I don't handle the action and return nothing from the reducer in that case, the dismiss gestures continue to work.

Could you please take a look at the example and see what changes might need to be made to reproduce the bug? If we can get a minimal test case that would really help narrow it down!

@brianegan
Copy link
Owner

@kentcb Hey hey -- sorry to bug ya, but has this continued to cause problems? Would be happy to make changes / fix it up, but couldn't repro myself and would love a bit more help if it's still causing pain :)

@kentcb
Copy link
Author

kentcb commented Jun 19, 2018

Hey @brianegan - no bother at all. Sorry, I got pulled onto another project so haven't been in flutter land for a bit. TBH, if it's not easy to reproduce I probably won't find the time any more to do so myself. I'll close this issue off and reopen later if it occurs and I can repro for you. Thanks.

@kentcb kentcb closed this as completed Jun 19, 2018
@brianegan
Copy link
Owner

Makes sense, thanks again for the report, and please do let me know if it bites ya again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants