-
Notifications
You must be signed in to change notification settings - Fork 0
Conversation
* @param textId id of the text in the dialog | ||
* @param name of the dialog | ||
*/ | ||
private fun showTextDialog(title: String, hint: String, id: Int, textId: Int, name: String) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Method showTextDialog
has 5 arguments (exceeds 4 allowed). Consider refactoring.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes look good to me! Thank you for the separation of adapter functions. From the UI point of view, I believe it would be better to display the user's profile (the on clicked in the search) as a popup with a button close in the left upper corner. Creating a new activity to display user's profile is a little heavy. But we can change it in the next pr!
|
||
private fun createMockImageGetter() { | ||
every {mockImageGetter.fetchImage(any(), any())} answers { | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
} | |
} | ||
|
||
private fun createMockImageGetter() { | ||
every {mockImageGetter.fetchImage(any(), any())} answers { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
every {mockImageGetter.fetchImage(any(), any())} answers { | |
every {mockImageGetter.fetchImage(any(), any())} answers {} |
} | ||
every {mockUsersRepo.getCurrentUser()} answers { | ||
null | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
} | |
secondArg<(User) -> Unit>().invoke(mockProfile) | ||
} | ||
every {mockUsersRepo.getCurrentUser()} answers { | ||
null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
null | |
every { mockUsersRepo.getUserData(any(), any()) } answers { | ||
secondArg<(User) -> Unit>().invoke(mockProfile) | ||
} | ||
every {mockUsersRepo.getCurrentUser()} answers { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
every {mockUsersRepo.getCurrentUser()} answers { | |
every {mockUsersRepo.getCurrentUser()} answers { null } |
Also, the other reason to make it as a popup, is cause now every time the profile is open, and return button is pressed, it goes back to the main actvity (not SearchUserActivity or ScoreBoardActivity) |
I didn't think about using a popup, but that seems to be a good idea, we can discuss that in detail during the next meeting. |
That's true, but I thought the red return button is designed to go back to the main, we can go back to the scoreboard using the default back button of the device. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work ! 👍
Just found somewhere with missing docs
|
||
class AdapterHelper { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
class AdapterHelper { | |
/** | |
* Missing Docs | |
*/ | |
class AdapterHelper { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also missing in functions below
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Code Climate has analyzed commit b940e98 and detected 3 issues on this pull request. Here's the issue category breakdown:
The test coverage on the diff in this pull request is 79.4% (80% is the threshold). This pull request will bring the total coverage in the repository to 82.4%. View more on Code Climate. |
Now if the user clicks on a player in scoreboard/search user, he can see the player's profile.
(similar to user profile except it cannot be edited)
The scores are displayed by default if he goes from the scoreboard.
Similar things will happen for search users in my next PR.
The coverage is only 79%, but I tested all the new functions I added, so it should not decrease the total much.