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

Gesture migration #87

Merged
merged 25 commits into from
Jun 23, 2017
Merged

Gesture migration #87

merged 25 commits into from
Jun 23, 2017

Conversation

mortoray
Copy link
Contributor

@mortoray mortoray commented Jun 2, 2017

This migrates several of the gesture and behaviors to use the gesture system. It allows these behaviours to overlap and behave together better.

This PR contains:

  • Changelog
  • Documentation
  • Tests

CHANGELOG.md Outdated
## Gesture
- The experimental `IGesture` interface has changed.
* The `Significance`, `Priority` and `PriotityAdjustment` have been merged into the single `GetPriority` function.
* `OnCapture` is changed to `OnCaptureChanged` and provides the previous capture state
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other multi-level lists use two spaces for indent, please use that in the future instead...

Copy link
Contributor

@kusma kusma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty good to me, but seems to miss a couple of details addressed...

if (Ended != null)
Ended();
}
_begun = false;
_trackingKeyboard = false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be wrapped in a try-finally block, so we know that _trackingKeyboard becomes false even if an Ended-handler throws?

Thinking about it a bit more, events kinda feel a bit crappy for this, as one event-handler throwing is going to ruin the fun for all the others, leaving them in a limbo-state... Why does programming have to be so awful?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll reorder. In general I want to call the event handlers last. I don't know if the entire handling is exception safe though, Gesture as a whole might be an issue, let me check a bit there.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did something minimal, but looking at the overall code I can't really "fix" it here.

https://github.com/fusetools/fuselibs/issues/4236

/**
The priority of the swiping gesture when competing with other gestures.

The default is Higher.
The default is Lower.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change of behavior should be mentioned in the changelog.

@kusma kusma merged commit 32cbbaf into fuse-open:master Jun 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants