Pull to refresh indicators #297
Conversation
…od for code clarity
… event doesn't show up on the association's page
…ture/pull-to-refresh # Conflicts: # app/src/main/java/com/android/unio/ui/event/EventCreation.kt
… in a FirestoreReferenceList
…ture/pull-to-refresh # Conflicts: # app/src/androidTest/java/com/android/unio/end2end/EventCreationE2ETest.kt
…ture/pull-to-refresh # Conflicts: # app/src/androidTest/java/com/android/unio/end2end/EventCreationE2ETest.kt # app/src/main/java/com/android/unio/ui/home/Home.kt
|
AlexeiThornber
left a comment
AlexeiThornber
left a comment
There was a problem hiding this comment.
Overall these are great changes that will greatly improve the user experience when loading events, associations, ect.. I will refuse this pull request so that you can add some java doc to the methods found in the repositories. Aside that this is some great work. Well done 👾 !
| .onNodeWithText(event.description, substring = true) | ||
| .assertDisplayComponentInScroll() | ||
|
|
||
| Espresso.onView(ViewMatchers.isRoot()).perform(ViewActions.swipeUp()) |
There was a problem hiding this comment.
Amazing tests! Using Expresso, thinking about me every time oh, is a great way to test this functionality !
| script: firebase emulators:exec --import=firebase/emulator-data './gradlew connectedCheck --parallel --build-cache' | ||
|
|
||
| # Upload the test results to the artifacts | ||
| - name: Upload Test Results |
There was a problem hiding this comment.
Great Addition, maybe a bit late for the project, but better than never
| * fetched. | ||
| * @param onFailure [(Exception) -> Unit] : The callback to call when the fetch fails. | ||
| */ | ||
| override fun getAssociationWithId( |
There was a problem hiding this comment.
Good job separating the two types of databases calls. If we just use the snapchot listener, there will be way too many Firebase requests 🚀
There was a problem hiding this comment.
The only thing I can critisize is the lack of descriptions to the methods. If you can add a small javadoc that'll be great !
| }) | ||
| } | ||
|
|
||
| fun refreshAssociation() { |
There was a problem hiding this comment.
Maybe add a description to this method ?
| } | ||
| } | ||
|
|
||
| fun refreshEvent() { |
There was a problem hiding this comment.
Again maybe add a description and a toast
| _refreshState.value = false | ||
| }, | ||
| onFailure = { exception -> | ||
| Log.e("AssociationViewModel", "Failed to fetch association", exception) |
There was a problem hiding this comment.
I believe that a toast would be a good implementation in the onFailure to show the user what is going on
This pull request adds pull to refresh indicators just like the one in the user's profile in the following places:
This pull request also temporarily fixes the issue where a newly created event would not show up on the association's page. This fix should be refactored soon as it produces excessive requests. EDIT: This has been fixed.