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

When device orientation is changed no changes will happen #248

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ajay-prabhakar
Copy link
Contributor

@ajay-prabhakar ajay-prabhakar commented Apr 20, 2019

Closes #120

Description

What has been done to verify that this works as intended?

  • Checked by adding this code any new warning or crash are produced or not
  • Checked when selected forms get unselected when orientation changed
  • Checked when activity changed are things remain same or newly started
  • Checked sending forms working properly or not
  • Checked whether the bottom bar is updating as expecting or not

Why is this the best possible solution? Were any other approaches considered?

Stored the value of selectedForms/ selectedInstances in onSaveInstanceState and retrieved the values back when activity is recreated

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

When the device orientation is changed while sending forms then he will get the changes directly instead for selecting forms once again

GIF

ezgif com-video-to-gif

Checkings

Before submitting this PR, please make sure you have:

  • run ./gradlew check code and confirmed all checks still pass OR confirm CircleCI build passes
  • verified that any code or assets from external sources are properly credited in comments and/or in the about file.

@ajay-prabhakar
Copy link
Contributor Author

@shobhitagarwal1612 @lakshyagupta21 can you please review it

android:parentActivityName=".activities.MainActivity">
<activity
android:name=".activities.SendFormsActivity"
android:configChanges="orientation|screenSize"
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you should highlight this code as so many changes related to the code style, hard for reviewers to find what truly changed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is the reason I added the line in PR template

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure, making a self comment in the code base would be better.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@huangyz0918 like that I made self comments in PR #240 but @shobhitagarwal1612 asked to remove those comments due to code style will not make better

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean, making comment in Github review panel.

Copy link
Contributor

Choose a reason for hiding this comment

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

@Chromicle I don't know if its the best solution or not, that's the reason I didn't add it in the first place.
It would be helpful if you can go through the pros and cons of using it, and then update your PR description to back up the changes that you had done.
For ref. https://stackoverflow.com/a/7990543

@lakshyagupta21
Copy link
Contributor

@Chromicle Any updates?

@ajay-prabhakar
Copy link
Contributor Author

I checked it in all ways and updated in PR templete by adding that code there will be no issues which I detected

@lakshyagupta21
Copy link
Contributor

Please resolve the merge conflicts.

@ajay-prabhakar
Copy link
Contributor Author

Please resolve the merge conflicts.

Yeah, Done @lakshyagupta21

@ajay-prabhakar
Copy link
Contributor Author

@lakshyagupta21 can you please review this

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

Successfully merging this pull request may close these issues.

On screen rotation selected forms get unselected
3 participants