Skip to content

Refactor container activities #374

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

Closed
wants to merge 90 commits into from
Closed

Refactor container activities #374

wants to merge 90 commits into from

Conversation

SUPERCILEX
Copy link
Collaborator

@SUPERCILEX SUPERCILEX commented Oct 22, 2016

@SUPERCILEX
Copy link
Collaborator Author

SUPERCILEX commented Oct 22, 2016

Again, #357 should be merged first to stay sane when reviewing this PR.

There's one major roadblock that's preventing me from proceeding: Twitter only lets you receive onActivityResults from an activity so there are three options:

  1. Make more breaking api changes 😞 that would require devs to send us the onActivityResult from their activity.
  2. Make two IdpSignInContainers: one that's an activity for SignInDelegate and one that's a fragment for AuthMethodPickerActivity. (This means that when people are restoring their phones they will see grossness because smart lock will probably be enabled, but other than that, normal sign-in will be pretty) Prefered option?
  3. Don't refactor IdpSignInContainerActivity at all (probably a bad idea)

Thoughts?

@SUPERCILEX
Copy link
Collaborator Author

Done refactoring EmailHintContainerActivity. It turned out kinda gross so any thoughts would be appreciated.

@SUPERCILEX SUPERCILEX changed the base branch from version-1.0.0-dev to master November 5, 2016 05:42
@samtstern samtstern mentioned this pull request Nov 21, 2016
10 tasks
@SUPERCILEX
Copy link
Collaborator Author

I just realized #418 replaces this PR.

@SUPERCILEX SUPERCILEX closed this Nov 24, 2016
@SUPERCILEX SUPERCILEX deleted the containers branch November 24, 2016 19:27
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.

1 participant