mattprecious / telescope

A simple tool to allow easy bug report capturing within your app.
Apache License 2.0
1.18k stars 116 forks source link

Foreground Service for native screenshot capturing #77

Closed julioz closed 4 years ago

julioz commented 4 years ago

Fixes https://github.com/mattprecious/telescope/issues/75

This branch was built on top of https://github.com/mattprecious/telescope/pull/76, to leverage the version bumps and also the crash reproducibility. Please consider merging that PR (it also means the diff for this one is smaller than it looks).

You can follow the commit history for easier understanding of the changes, but in summary:

Open question: Since the foreground service requires a notification, and thus a notification channel, I hardcoded some values for simplicity (like notification channel name, service ID, notification ID, and didn't define a notification icon). Is this something users of the library absolutely care about, and if so, would it be worth providing a public API via telescope's configuration methods to define them?

JakeWharton commented 4 years ago

Why move the actual media projection interaction to the service? The requirement is for a foreground service to be present (thus a notification), but not that the work is actually happening in the service. All we should need is an empty service and its notification that can be started and stopped by the existing code that interacts with media projection.

julioz commented 4 years ago

but not that the work is actually happening in the service

This is news to me, I might have misread the documentation.

Thanks for the clarification, I'll send out another PR as soon as I have the time.