Skip to content

Conversation

@aitorvs
Copy link
Collaborator

@aitorvs aitorvs commented Aug 4, 2021

Task/Issue URL: https://app.asana.com/0/414730916066338/1200701185339109/f
Tech Design URL:
CC:

Description:
This PR fixes a crash that happened when submitting negative feedback.
In MainReasonNegativeFeedbackFragment and SubReasonNegativeFeedbackFragment we were doing some logic in onCreateView and then returning binding.root. But Android does not allow to use view binding inside onCreateView that should return an inflated view, which at that point it is not.

This PR moves the logic performed in the onCreateView method to the onActivityCreated method--which is the next lifecycle method to be called--so that we can continue using view binding

Steps to test this PR:

  1. install from develop and launch the app
  2. go to settings -> share feedback
  3. click in sad face
  4. BOOM! app crashes
  5. install from this branch and launch the app
  6. go to settings -> share feedback
  7. click in sad face
  8. verify app does not crash
  9. select a sub-reason
  10. verify app does not crash
  11. complete the feedback and submit
  12. verify app does not crash

Internal references:

Software Engineering Expectations
Technical Design Template

@aitorvs aitorvs force-pushed the fix/aitor/require_view_crash branch from db4c471 to 7f31bb7 Compare August 4, 2021 09:38
Copy link
Contributor

@cmonfortep cmonfortep left a comment

Choose a reason for hiding this comment

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

LGTM 👍 All good! Thanks for fixing it so quick.

override fun onActivityCreated(savedInstanceState: Bundle?) {
super.onActivityCreated(savedInstanceState)

recyclerAdapter = MainReasonAdapter(object : (FeedbackTypeMainReasonDisplay) -> Unit {
Copy link
Contributor

Choose a reason for hiding this comment

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

We could even create the adapter using by lazy directly in the var declaration if we like more that style. In any case 👍

@cmonfortep cmonfortep self-assigned this Aug 4, 2021
@aitorvs aitorvs merged commit 7719506 into develop Aug 4, 2021
@aitorvs aitorvs deleted the fix/aitor/require_view_crash branch August 4, 2021 10:28
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.

2 participants