Skip to content
This repository has been archived by the owner on Mar 16, 2021. It is now read-only.

Feature: restrict TiFragment api for subclasses #79

Merged
merged 5 commits into from
Apr 5, 2017

Conversation

ghost
Copy link

@ghost ghost commented Apr 4, 2017

This pull request restricts the TiFragment api by declaring some implemented methods from the DelegatedTiFragment-Interface as final so they cannot be overridden by subclasses.
It also adds the @CallSuper-Annotation to all important lifecycle methods that requires calling the super implementation because TiFragment uses this lifecycle methods to manage the TiFragmentDelegate instance.

This should help to prevent issues which are caused by extending TiFragment and not call its respective super implementations (see #29 for example).

@passsy suggested to use the FragmentManager.FragmentLifecycleCallbacks (https://developer.android.com/reference/android/support/v4/app/FragmentManager.FragmentLifecycleCallbacks.html) to enforce the calls on the managed TiFragmentDelegate instance but since we are trying to hit the 0.8.0 release i think this would be a rather drastic improvement.

Robert Berghegger added 2 commits April 4, 2017 17:48
…om the DelegatedTiFragment interface as final so they cannot be overridden by subclasses.
… requires calling the super implementation because TiFragment uses this lifecycle methods to manage the TiFragmentDelegate instance.
@ghost ghost added the Ready for Review label Apr 4, 2017
@ghost ghost changed the title Feature/restrict tifragment api for subclasses Feature: restrict TiFragment api for subclasses Apr 4, 2017
@ghost ghost requested review from passsy and StefMa April 4, 2017 16:16
@passsy
Copy link
Contributor

passsy commented Apr 4, 2017

Do the same for TiActivity as well as TiFragmentPlugin and TiActivityPlugin

@ghost
Copy link
Author

ghost commented Apr 4, 2017

@passsy I'm not sure whether or not onRetainCustomNonConfigurationInstance in the TiActivity-Class should be finalized or annotated with @CallSuper as well. 🤔

@passsy
Copy link
Contributor

passsy commented Apr 4, 2017

final would be wrong. Add @CallSuper

Copy link
Contributor

@StefMa StefMa left a comment

Choose a reason for hiding this comment

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

As @passsy mentioned. Please add the CallSuper to onRetainCustomNonConfigurationInstance inside the TiActivtiy... 👍
Rest looks good

@ghost
Copy link
Author

ghost commented Apr 5, 2017

I applied your suggestions to the TiActivity, TiActivityPlugin and the TiFragmentPlugin.

@passsy
Copy link
Contributor

passsy commented Apr 5, 2017

TiDialogFragment is missing 😉

@passsy passsy merged commit 972fd68 into master Apr 5, 2017
@passsy passsy deleted the feature/restrict_tifragment_api_for_subclasses branch April 5, 2017 10:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging this pull request may close these issues.

2 participants