-
Notifications
You must be signed in to change notification settings - Fork 0
Conversation
@@ -31,4 +32,9 @@ class MainActivity : AppCompatActivity() { | |||
startActivity(scoreboardIntent) | |||
} | |||
} | |||
|
|||
fun switchToWelcome(view: View) { |
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.
Similar blocks of code found in 6 locations. Consider refactoring.
setContentView(R.layout.activity_welcome_screen) | ||
} | ||
|
||
fun switchToPlay(view: View) { |
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.
Similar blocks of code found in 6 locations. Consider refactoring.
startActivity(intent) | ||
} | ||
|
||
fun switchToProfile(view: View) { //FILLER INTENT |
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.
Similar blocks of code found in 6 locations. Consider refactoring.
startActivity(intent) | ||
} | ||
|
||
fun switchToScoreboard(view: View) { |
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.
Similar blocks of code found in 6 locations. Consider refactoring.
startActivity(intent) | ||
} | ||
|
||
fun switchToListen(view: View) { //FILLER INTENT |
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.
Similar blocks of code found in 6 locations. Consider refactoring.
startActivity(intent) | ||
} | ||
|
||
fun switchToSettings(view: View) { //FILLER INTENT |
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.
Similar blocks of code found in 6 locations. 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.
Both UI and backend code seems clear to me, and I have checked the functionality. We should work on the colors and style though.
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.
Good job on the welcome screen and game setup screen!
Maybe try to reduce the code duplications indicated by the CI, use @string for the strings instead of hardcode them and remove the commented code blocks (or add some documentation explaining why they are in comment)
You can also put a Screenshot of the UI in the PR description. I might be too curious but it will be nice to see them ;)
intended(hasComponent(GameSetupScreen::class.java.name)) | ||
} | ||
|
||
/*@Test |
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.
why this test is in the comments?
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.
I removed a button that was causing problems, as it wasn't fitting in whatever emulator they are using for the CI tests. It all passed locally until then. This test is for that button, since it doesn't exist for now, I removed it, however am thinking of readding it in some form.
Code Climate has analyzed commit ee6882a and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 97.6% (80% is the threshold). This pull request will bring the total coverage in the repository to 99.2% (-0.7% change). View more on Code Climate. |
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.
LGTM!
Added the welcome screen and game setup screen which allows anywhere from one to four people to play together.