Code review comment for lp:~elopio/autopilot/fix1257055-slow_drag

Revision history for this message
Leo Arias (elopio) wrote :

> If you're adding (or removing, or changing) a parameter to a public method, please document it in the docstrings.

I did my best. Please check it.

> In general this MP makes me feel rather uneasy. You're introducing a new parameter, and some code changes, and I can't see any justification for it.

The justification is on the linked bug. We need a way to slow down the drags. If we do them with the current hard-coded rate, when we try to make a new item visible on a list, we end up on the bottom of the list instead.

> If this is really needed, I think it might be worth moving the algorithm to a common location, and making both the X11 and uinput backends be able to use it. That way, you can test it without needing to mock out anything.

That's not actually possible at the moment, because the X11 relies on the move method, and so, it uses the current position of the pointer. On Touch, we don't have a current position, so there's no clean way to do a slow drag. Of course, I plan to keep cleaning things up, and that's something I will try in the following weeks.

But for now, I am losing my head with the number of workarounds I have to do on every place where the drag fails. This is an improvement from the current status, that has a hard-coded value of 100 steps per drag on the touch and 10 pixels per iteration on the mouse, which is inconsistent and comes without tests.

Lets talk tomorrow on IRC.

For now, I'll update the tests to use the new testability that comes from the branch I've added as a prerequisite.

« Back to merge proposal