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 #804 - Fixes Wrong Fragment selection #1230

Merged
merged 1 commit into from
Dec 16, 2019
Merged

Fix #804 - Fixes Wrong Fragment selection #1230

merged 1 commit into from
Dec 16, 2019

Conversation

OHassini
Copy link
Contributor

Fixes #804

Please Add Screenshots If there are any UI changes.
video2gif_20191212_115515

Please make sure these boxes are checked before submitting your pull request - thanks!

  • Apply the MifosStyle.xml style template to your code in Android Studio.

  • Run the unit tests with ./gradlew check to make sure you didn't break anything

  • If you have multiple commits please combine them into one commit by squashing them.

Copy link

@PiyushSharma99 PiyushSharma99 left a comment

Choose a reason for hiding this comment

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

LGTM

@OHassini
Copy link
Contributor Author

@luckyman20 @moksh-mahajan Hi, It's been more than a day since I've pushed this PR please have a look !!!

@moksh-mahajan
Copy link
Collaborator

@OussEmaDevCode Sorry for the delay.
On it.

@manigedit
Copy link

@moksh-mahajan I hope I am not needed to intervene.

Copy link
Collaborator

@moksh-mahajan moksh-mahajan left a comment

Choose a reason for hiding this comment

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

Good job @OussEmaDevCode . The changes look fine. Just replace the code with switch case as @laxyapahuja suggested to make code a bit clean.

@OHassini
Copy link
Contributor Author

@moksh-mahajan @luckyman20 Hi, It's simply impossible to use switch statement with instanceof operator because the switch statement can only contain case statements which are compile time constants and which evaluate to an integer up to Java 6 and a string in Java 7. And Since I didn't want to deceive you I did this solution with involves the class names instead of the actual classes. Hope it's fine !!!!

@OHassini
Copy link
Contributor Author

OHassini commented Dec 16, 2019

@moksh-mahajan That will create a style error like this 'the line exceeds 100 characters' and the build will fail.

@moksh-mahajan
Copy link
Collaborator

moksh-mahajan commented Dec 16, 2019

@OussEmaDevCode I mean to say line no. 245 can be merged with line no. 244

@OHassini
Copy link
Contributor Author

@moksh-mahajan Done !!!

@OHassini
Copy link
Contributor Author

@moksh-mahajan Done !!

Copy link
Collaborator

@moksh-mahajan moksh-mahajan left a comment

Choose a reason for hiding this comment

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

Good job @OussEmaDevCode

@OHassini
Copy link
Contributor Author

@moksh-mahajan is it going to be merged ?

@luckyman20 luckyman20 merged commit 714bdd5 into openMF:master Dec 16, 2019
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.

Wrong Fragment selection in Navigation Pane
5 participants