Skip to content

Conversation

@anoopaneesh
Copy link
Contributor

Addressing issue: #945

I have a added a snackbar when user deletes a bookmark or fireproof website.The snackbar contains a undo option which can be used to cancel the deletion.I have used string resources instead of hardcoded string .
Screenshot_1601553190
Screenshot_1601552863

@anoopaneesh anoopaneesh closed this Oct 1, 2020
@anoopaneesh anoopaneesh reopened this Oct 1, 2020
@cmonfortep
Copy link
Contributor

@anoopaneesh Thanks for the contribution!

This is failing because our ktlint detected some codestyle issues:

/bitrise/src/app/src/main/java/com/duckduckgo/app/bookmarks/ui/BookmarksActivity.kt:143:61: Missing spacing before "{"
/bitrise/src/app/src/main/java/com/duckduckgo/app/bookmarks/ui/BookmarksViewModel.kt:34:1: Unused import
/bitrise/src/app/src/main/java/com/duckduckgo/app/bookmarks/ui/BookmarksViewModel.kt:108:41: Missing spacing before "{"
/bitrise/src/app/src/main/java/com/duckduckgo/app/fire/fireproofwebsite/ui/FireproofWebsitesActivity.kt:30:1: Unused import
/bitrise/src/app/src/main/java/com/duckduckgo/app/fire/fireproofwebsite/ui/FireproofWebsitesActivity.kt:83:61: Missing spacing before "{"
/bitrise/src/app/src/main/java/com/duckduckgo/app/fire/fireproofwebsite/ui/FireproofWebsitesViewModel.kt:75:64: Missing spacing before "{"
/bitrise/src/app/src/main/java/com/duckduckgo/app/fire/fireproofwebsite/ui/FireproofWebsitesViewModel.kt:77:55: Missing spacing before "{"

You can fix them manually, or configure your Android studio with our setup.

@malmstein
Copy link
Contributor

Thanks for the contribution @anoopaneesh! I'll take a look at the implementation during this week.

Copy link
Contributor

@malmstein malmstein left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution @anoopaneesh, a few comments before I do some manual testing:

  • Can you add tests for the new methods added to the ViewModel?
  • This project is translated to several languages, but you have only changed the text for English. Can you add two new entries to strings-untranslated with the new snackbars messages? Also remove the old bookmarkDeleteConfirmMessage and fireproofWebsiteDeleteConfirmMessage from all the other string files.

@anoopaneesh
Copy link
Contributor Author

added two new entries to strings-untranslated with the new snackbars messages. Also removed the old bookmarkDeleteConfirmMessage and fireproofWebsiteDeleteConfirmMessage from all the other string files.

Copy link
Contributor

@malmstein malmstein left a comment

Choose a reason for hiding this comment

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

Thanks for updating the strings @anoopaneesh, once the tests are added to the ViewModel we can close this one.

@cmonfortep
Copy link
Contributor

@anoopaneesh Do you have plans to address the comments in this PR? Thanks.

@anoopaneesh
Copy link
Contributor Author

anoopaneesh commented Jan 7, 2021 via email

@malmstein
Copy link
Contributor

I've created #1056 to add the missing tests and fix the conflicts shown here

@malmstein malmstein closed this Jan 8, 2021
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.

3 participants