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 #899: Replaced Snackbar with Toast in Recent Transactions #914

Closed
wants to merge 1 commit into from
Closed

Fix #899: Replaced Snackbar with Toast in Recent Transactions #914

wants to merge 1 commit into from

Conversation

MigDinny
Copy link
Contributor

@MigDinny MigDinny commented Oct 25, 2018

Fixes #899

Removed "No more transactions available" snackbar from the app.

  • Apply the AndroidStyle.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.

Squashed them into 1 commit.

image

Awaiting response.

@MigDinny MigDinny changed the title Removed showMessage() Snackbar Fix #899 Oct 25, 2018
@miPlodder
Copy link
Collaborator

@MigDinny Instead of removing the Snackbar, add a Toast message and fix the conflict. Follow commit instructions and change the commit message accordingly.

@MigDinny
Copy link
Contributor Author

@miPlodder

I thought you asked in the issue to remove snackbar and not replace it. I’m confused now... should I replace it with a toast?

About the build errors, I think they are fixed in another PR created by another student. Should I do the same to mine or wait that his PR gets merged?

@miPlodder
Copy link
Collaborator

miPlodder commented Oct 26, 2018

@miPlodder

I thought you asked in the issue to remove snackbar and not replace it. I’m confused now... should I replace it with a toast?

About the build errors, I think they are fixed in another PR created by another student. Should I do the same to mine or wait that his PR gets merged?

Yes, I know that. I think that informing the user via Toast message will serve a purpose in future, whereas Snackbar will do the same, but the UX will be better in case of Toast.

Yes, pull the development branch, when it will get merged. Currently, it is being reviewed by other maintainers.

@MigDinny
Copy link
Contributor Author

Ok. Will do it when I get home. I’m in school right now.

@MigDinny
Copy link
Contributor Author

@miPlodder Created the toast. Updated it with the new merge.

@MigDinny MigDinny changed the title Snackbar Fix #899 Fix #899: Snackbar Oct 26, 2018
app/build.gradle Outdated
@@ -116,6 +116,9 @@ dependencies {
//mifos passcode
implementation "com.mifos.mobile:mifos-passcode:$mifosPasscodeVersion"

//multidex
Copy link
Collaborator

Choose a reason for hiding this comment

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

@MigDinny Remove this and pull the recent changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can't. That is from aa20e9f. It will break #909

Copy link
Collaborator

Choose a reason for hiding this comment

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

@MigDinny You have not pulled the changes properly. Check out this PR (https://github.com/openMF/mifos-mobile/pull/915/files or any other updated PR). It doesn't contain it.

@miPlodder
Copy link
Collaborator

miPlodder commented Oct 27, 2018

@MigDinny Change the commit message, Fix #899 : Replaced Snackbar with Toast in Recent Transaction.

@MigDinny MigDinny changed the title Fix #899: Snackbar Fix #899: Replaced Snackbar with Toast in Recent Transactions Oct 27, 2018
app/build.gradle Outdated
@@ -116,6 +116,9 @@ dependencies {
//mifos passcode
implementation "com.mifos.mobile:mifos-passcode:$mifosPasscodeVersion"

//multidex
Copy link
Collaborator

Choose a reason for hiding this comment

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

@MigDinny You have not pulled the changes properly. Check out this PR (https://github.com/openMF/mifos-mobile/pull/915/files or any other updated PR). It doesn't contain it.

* Shows a Toast message
* @param message
*/
public void showToastMessage(String message) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't create new method.

Use this method provided here.

.append(' ')
.append(integersOfDate.get(0));

if (integersOfDate != null) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

@MigDinny Why are you doing a null check here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not.

e6ef485

Copy link
Collaborator

Choose a reason for hiding this comment

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

@MigDinny Your are not pulling the changes properly. Green Colors shows the additions by your branch/PR. Red shows the line you have deleted. Analyze your code properly.

@MigDinny MigDinny closed this Oct 27, 2018
Replaced snackbar with toast
@MigDinny MigDinny reopened this Oct 27, 2018
@MigDinny
Copy link
Contributor Author

@miPlodder Fixed the PR. The gradle changes etc are fixed.

About the showToast() I managed to access the Activity and call BaseActivity.showToast() through RecentTransactionsFragment.

@MigDinny
Copy link
Contributor Author

@miPlodder I don't really know why bots are failing to build this. This PR is updated with development branch and I literally changed nothing but a few lines of code which have nothing to do with gradle.

@miPlodder
Copy link
Collaborator

miPlodder commented Oct 27, 2018

I don't really know why bots are failing to build this. This PR is updated with development branch and I literally changed nothing but a few lines of code which have nothing to do with gradle.

The number of lines of changes is less, it will be better you create a new branch and start fresh.

@MigDinny
Copy link
Contributor Author

@miPlodder Ok, will do it. What empty method were you talking about in GCI task?

@@ -42,6 +42,8 @@

void showMessage(String message);

void showToastMessage(String message);
Copy link
Collaborator

Choose a reason for hiding this comment

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

@MigDinny I meant to remove this and write code show message in showMessage()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't want to do it because it would probably ruin other showMessage() callings where a snackbar is the expected behavior.

I will close this PR and create a new one with a new branch

@MigDinny MigDinny closed this Oct 28, 2018
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