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

Fix issue of don't keep activities #108

Merged
merged 9 commits into from
Feb 11, 2016

Conversation

sys1yagi
Copy link

@sys1yagi sys1yagi commented Feb 9, 2016

Related issue

#95

Problem

When I opened a detail screen of a talk and go back to the "All Sessions" screen with the back button, the app crashed and logged below stack trace.

Cause

Crash point is SessionsFragment.java#L175.

SessionsFragment#onActivityResult() is unnecessary. FragmentActivity calls onActivityResult() of the child Fragment automatically. So, you should using the Fragment#startActivityForResult().

Solution

  • 044e13c Initialize a fragment only when savedInstanceState of the Activity is null.
  • 965a857 Use Fragment#startActivityForResult(). I added the method for Fragment#startActivityForResult() to ActivityNavigator.
  • e8b3245 Remove unnecessary codes.

Tests

Verified the following:

  • With "Don't keep activities" on, I opened a session detail and added it to my schedule and return back to the All Sessions to see the session was checked
  • With "Don't keep activities" off, I opened a session detail and added it to my schedule and return back to the All Sessions to see the session was checked

@sys1yagi
Copy link
Author

sys1yagi commented Feb 9, 2016

@hkurokawa Could you review this when you have time? 🙏

@konifar
Copy link
Owner

konifar commented Feb 9, 2016

👀

@hkurokawa
Copy link
Collaborator

@sys1yagi Thank you for your fix. It is great.

By the way, I feel like SearchActivity need to do a similar behaviour at onActivityResult() as it is currently does not use REQ_DETAIL any other place than where it calls activityNavigator.showSessionDetail() when an item is pressed. And I verified the below steps caused a problem. How do you think?

  1. Click the magnifying glass to open search screen
  2. Input any text to show the search result
  3. Open a session
  4. Click the star icon to add the session to my schedule
  5. Press Back button to return to the search result
  6. Open the same session again
  7. The star is not turned on

@sys1yagi
Copy link
Author

ic, I'll fix it as well. 😉

@konifar konifar added this to the 1.0.0 milestone Feb 10, 2016
# Conflicts:
#	app/src/main/java/io/github/droidkaigi/confsched/activity/ActivityNavigator.java
#	app/src/main/java/io/github/droidkaigi/confsched/activity/MainActivity.java
@sys1yagi
Copy link
Author

SearchActivity issue is complicated and it has nothing to do with this PR.
I'll correct it separately. Do you mind?

@hkurokawa
Copy link
Collaborator

@sys1yagi Oh, sorry for that. No, don't mind. Please create another PR and let me know when this PR is ready to merge. I will merge this PR.

@konifar
Copy link
Owner

konifar commented Feb 11, 2016

@sys1yagi Great thanks!!

@konifar
Copy link
Owner

konifar commented Feb 11, 2016

LGTM!
Thanks for your fix! I will merge now because this PR might be conflicted.

konifar added a commit that referenced this pull request Feb 11, 2016
@konifar konifar merged commit 91ab77e into konifar:master Feb 11, 2016
@sys1yagi
Copy link
Author

thanks! I'm writing detail of this change now.

@sys1yagi
Copy link
Author

Detail of this changes. (Japanese) https://gist.github.com/sys1yagi/b0596187917501331707

@konifar
Copy link
Owner

konifar commented Feb 11, 2016

@sys1yagi Thanks! I'll read it 👍

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

Successfully merging this pull request may close these issues.

None yet

3 participants