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

[RFC] LC 2.0 API. #48

Open
glung opened this issue Jun 10, 2016 · 7 comments
Open

[RFC] LC 2.0 API. #48

glung opened this issue Jun 10, 2016 · 7 comments

Comments

@glung
Copy link
Contributor

glung commented Jun 10, 2016

▲ ▲ ▲ /!\ This is a draft▲ ▲ ▲

🚧

Lately, @jenzz created this 3 PRs to dispatch more callbacks :

This exposes the fact that currently the LC API is not consistent. For example ActivityLightCycle provides onCreate, onOptionsItemSelected but not onPostCreate.

Context

Initially, LightCycle were meant to remove the boilerplate to hook up a presenter to an Activity/Fragment lifecycle. The library was basically handling the same callbacks than ActivityLifecycleCallbacks. But, as we used it more and more across the SoundCloud app and other companies apps, we ended adding others callbacks as we felt the need.

We even added boolean onOptionsItemSelected(T activity, MenuItem item);. On the other hand, we decided to not add onCreateOptionsMenu since it was decided it would be the responsibility of the activity to inflate layouts.

Let's rethink the API

Goals

  • Define a consistent API for LC

Proposals

Activity

// TODO

Fragment

// TODO

@glung
Copy link
Contributor Author

glung commented Sep 12, 2016

Hello,

We would love to collect your feedback here, that would help us to design the next version of LightCycles. Please comment here.

Non-exhaustive of things that would help :

  • If you use it :
    • In which context ?
    • What works for you ?
    • What does not work that well for you ?
  • If you don't, maybe mention if there is any blocker for you.
  • ...

Cheers

cc @vkotovv @doridori @ejohansson @MehdiChouag @farmazon3000 @guavabot @xrigau @janogonzalez

@michaelengland
Copy link
Contributor

  • Which context:
    • We use it for any activity/fragment 'mixin' that we'd like. (e.g. analytics for start/stop session, lifecycle providers
    • When we have classes that are controlling some state that then should also make use of the lifecycle (e.g. loading states).
  • Works for us:
    • The ease of use, especially together with Dagger2, and removal of a lot of boilerplate lifecycle-proxying is nice.
  • Doesn't work/could be improved for us:
    • We don't really need the activity/fragment in the calls, as we can use dagger for that.
    • Trying to get the types working can be a bit of a nightmare (e.g. creating a base class that doesn't extend one of the supported default activity/lifecycles produces generated code generics errors).
    • For us it would be useful to really have ALL of the lifecycle methods, as it's difficult for us to say what should/shouldn't be there. (e.g. we have a class that has the onCreateOptionsMenu & onOptionsItemSelected, which is used in a few activities. This would be great to also pull into a lightcycle controller.

@guavabot
Copy link

I was excited initially about the lib, but ended up not using it. The blocker is related to <T extends Activity> and <T extends Fragment> in the LightCycles.

As @michaelengland said, I don't need the Activity/Fragment in all the calls because they can be injected. I only need to know the stages in the lifecycle. I would prefer:

  • Non-generic LightCycles
  • A third type of LightCycle that only includes calls that are common to Activity and Fragment. I normally only use the calls that are common and this way it would be easy to switch an Activity to Fragment.

@glung
Copy link
Contributor Author

glung commented Nov 17, 2016

Hello, I finally took some time to summarize the feedbacks we received.

Lexicon

  • LightCycle : a representation of the Activity or Fragment's lifecycle.
  • Host : the instance of the Activity or Fragment's lifecycle as seen by the LightCycle.
  • LifeCycle : an Activity or a Fragment

Pseudo-code :

LightCycle {    
    - callback1 (host, <forwarded paramters if any>)
    - callback2 (host, <forwarded paramters if any>)
    [ ... ]
    - callbackN (host, <forwarded paramters if any>)
}

Needs

LightCycle

  • Add all callbacks (for instance, onPostCreate, onWindowFocusChanged, ... )
  • Provide a LightCycle of both Activity and Fragment (lifecycle's least common denominator)
  • Make the host optional to LightCycle callbacks

Pseudo-code :

LightCycle {    
    - callback1 (<forwarded paramters if any>)
    - callback2 (<forwarded paramters if any>)
    [ ... ]
    - callbackN (<forwarded paramters if any>)
}

LifeCycle (activity or fragment)

  • Ease of creating custom hosts (typically a base activity).

Host

  • The host can be an Activity, a Fragment or anything else (can be an interface like MyView).

@glung
Copy link
Contributor Author

glung commented Nov 17, 2016

Based on that, here are our requirements for the v2. cc @michaelengland

Must

  • (1) keep existing "features"
  • (2) make the host optional
  • (3) dispatch all callbacks
  • (4) introduce a LightCycle of both Activity and Fragment.
  • (5) streamline creation of custom hosts (typically a base activity).

Should

  • (6) allow exposing a subset of the host's API to the LightCycle (like an interface, MyView).
  • (7) be retro-compatble (if not, create a v2 package)

Won't

  • (8) decrease perfornamnces (LightCycle sits in the critical path of Android apps)

Does it make sense ?

@NikoYuwono
Copy link
Contributor

If I may add something, should we add automatic binding too? (If it's possible and doesn't decrease performances)

@glung
Copy link
Contributor Author

glung commented Nov 17, 2016

Good point ! We could do that before v2. Actually, I found this : #36

Pardon the poor description : ) !

@glung glung mentioned this issue Dec 14, 2016
4 tasks
glung added a commit that referenced this issue Dec 15, 2016
This mostly enable presenters (which are light cycles) to use views (implemented by Fragment, Activity, …). 

- Dispatcher : aware of the Android life cycle (Activity, SupportFragment, Fragment)
- Host : the activity/fragment as viewed by the light cycle. It can be a view.


#48
glung added a commit that referenced this issue Dec 15, 2016
…hich can be implemented by Fragment or Activity classes.

- Dispatcher: dispatch the Android life cycle (Activity, SupportFragment, Fragment) to LightCycles.
- Host: the activity/fragment as viewed by the light cycle. It can be a view.
- LightCycle: notified of the life cycle with the Host as a parameter.

#48
glung added a commit that referenced this issue Dec 20, 2016
…hich can be implemented by Fragment or Activity classes.

- Dispatcher: dispatch the Android life cycle (Activity, SupportFragment, Fragment) to LightCycles.
- Host: the activity/fragment as viewed by the light cycle. It can be a view.
- LightCycle: notified of the life cycle with the Host as a parameter.

#48
glung added a commit that referenced this issue Dec 20, 2016
…hich can be implemented by Fragment or Activity classes. (#87)

- Dispatcher: dispatch the Android life cycle (Activity, SupportFragment, Fragment) to LightCycles.
- Host: the activity/fragment as viewed by the light cycle. It can be a view.
- LightCycle: notified of the life cycle with the Host as a parameter.

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

No branches or pull requests

4 participants