Skip to content
This repository has been archived by the owner on Jan 31, 2024. It is now read-only.

Maximezmt/UI #124

Merged
merged 23 commits into from
Mar 31, 2022
Merged

Maximezmt/UI #124

merged 23 commits into from
Mar 31, 2022

Conversation

MaximeZmt
Copy link
Owner

@MaximeZmt MaximeZmt commented Mar 29, 2022

Coherent UI

This is related to issue #116

This Pull Requests contains:

ch.sdp.vibester.activity: GameEndingActivity: Update UI
ch.sdp.vibester.activity: GameSetupActivity: Update UI + add submenu
ch.sdp.vibester.activity: GamescreenActivity: Update UI
ch.sdp.vibester.activity: IncorrectSongActivity: Update UI
ch.sdp.vibester.activity: ScoreboardActivity: Update UI
ch.sdp.vibester.activity: ProfileActivity: Update UI + Correct Image
ch.sdp.vibester.activity: SettingsActivity: Update UI
Readme: has been modified
Logo: Has been refactored

In addition the code is tested with a coverage of: [100]%

Notes for reviewers

  • NONE

@MaximeZmt MaximeZmt self-assigned this Mar 29, 2022
@MaximeZmt MaximeZmt marked this pull request as ready for review March 30, 2022 22:25
Copy link
Collaborator

@jiabaow jiabaow left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -85,4 +95,15 @@ class GameSetupActivity : AppCompatActivity(), AdapterView.OnItemSelectedListene
intent.putExtra("Player Names", pNameArray)
startActivity(intent)
}

private fun choosingGameSetupListener(){
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
private fun choosingGameSetupListener(){
private fun chooseGameSetupListener(){

Copy link
Owner Author

Choose a reason for hiding this comment

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

👍

@@ -123,6 +135,15 @@ class ProfileActivity : AppCompatActivity() {
findViewById<TextView>(R.id.ranking).text = user.ranking.toString()
/* TODO: add functionality to display the image (may be using )
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
/* TODO: add functionality to display the image (may be using )

Copy link
Owner Author

Choose a reason for hiding this comment

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

👍

@@ -123,6 +135,15 @@ class ProfileActivity : AppCompatActivity() {
findViewById<TextView>(R.id.ranking).text = user.ranking.toString()
/* TODO: add functionality to display the image (may be using )
findViewById<ImageView>(R.id.avatar).loadImg(user.image)*/
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
findViewById<ImageView>(R.id.avatar).loadImg(user.image)*/

Copy link
Owner Author

Choose a reason for hiding this comment

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

👍

Copy link
Collaborator

@kamilababayeva kamilababayeva left a comment

Choose a reason for hiding this comment

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

The app looks pretty now! Thank you for your work!

Github does not allow me to view all the changed files, so I will leave some small comments there. I think we can fix them in the next PR.

WelcomeActivity:button play music is not working and the app crashes. We need to remove switchToListen function from there.
GameSetupActivity: I liked it more when there was a small arrow in the dropdown for number of players. Now it is not clear if it is clickable or not.
[Question] is it okay that github build badge says failing?

@@ -123,6 +135,15 @@ class ProfileActivity : AppCompatActivity() {
findViewById<TextView>(R.id.ranking).text = user.ranking.toString()
/* TODO: add functionality to display the image (may be using )
findViewById<ImageView>(R.id.avatar).loadImg(user.image)*/
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you remove these comment? thanks!

Copy link
Owner Author

Choose a reason for hiding this comment

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

👍

val bm = task.await()
findViewById<ImageView>(R.id.avatar).setImageBitmap(bm)
}
findViewById<ImageView>(R.id.avatar).foregroundGravity= Gravity.LEFT
Copy link
Collaborator

Choose a reason for hiding this comment

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

We have to do the same logic for loading images in the scoreboard. Right now, Glide library is used, but it is not working. I think it will take some time to redo, so lets leave it for future tasks.

Copy link
Collaborator

Choose a reason for hiding this comment

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

yep, now the glide is failing:
W/Glide: Load failed for https://images.app.goo.gl/UpXEDTFLbcTL5mTJ8 with size [197x197]
class com.bumptech.glide.load.engine.GlideException: Failed to load resource
There were 8 root causes:
java.lang.RuntimeException(setDataSourceCallback failed: status = 0x80000000)
java.lang.RuntimeException(setDataSourceCallback failed: status = 0x80000000)

Copy link
Owner Author

Choose a reason for hiding this comment

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

👍


requestWindowFeature(Window.FEATURE_NO_TITLE)
supportActionBar?.hide()

Copy link
Collaborator

Choose a reason for hiding this comment

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

now these functions appear in most of the activities... do you think it would be better to do it from xml? Or maybe have one function that will be called in all these activities, or it would be too much?

Copy link
Owner Author

Choose a reason for hiding this comment

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

I don't know if it is doable from the XML, maybe we should look for it next week :)

val bm = task.await()
findViewById<ImageView>(R.id.avatar).setImageBitmap(bm)
}
findViewById<ImageView>(R.id.avatar).foregroundGravity= Gravity.LEFT
Copy link
Collaborator

@kamilababayeva kamilababayeva Mar 31, 2022

Choose a reason for hiding this comment

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

why dont you move Gravity stuff it to xml? is it possible?

Copy link
Owner Author

Choose a reason for hiding this comment

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

yep will do it

## Scrumboard & Weekly Meetings Logs <a name="devLogs"></a>

- [Our Scrumboard](https://github.com/MaximeZmt/SDP_2022-Vibester/projects/1)
- [Our Weekly Meeting](https://github.com/MaximeZmt/SDP_2022-Vibester/wiki)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also in the beginning we can remove [To Complete: Abstract]

Copy link
Owner Author

Choose a reason for hiding this comment

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

yes

@MaximeZmt
Copy link
Owner Author

The app looks pretty now! Thank you for your work!

Github does not allow me to view all the changed files, so I will leave some small comments there. I think we can fix them in the next PR.

WelcomeActivity:button play music is not working and the app crashes. We need to remove switchToListen function from there. GameSetupActivity: I liked it more when there was a small arrow in the dropdown for number of players. Now it is not clear if it is clickable or not. [Question] is it okay that github build badge says failing?

Concerning the badges that fails, it is normal -> CI has failed and therefore it shows fail, just need a rebuild (would be done once merged)
Yes I will hide the button, I've keeped it temporarily if needed in the future
Yes I will see How I can change it :)

@codeclimate
Copy link

codeclimate bot commented Mar 31, 2022

Code Climate has analyzed commit 0817172 and detected 1 issue on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 1

The test coverage on the diff in this pull request is 100.0% (80% is the threshold).

This pull request will bring the total coverage in the repository to 91.3%.

View more on Code Climate.

@MaximeZmt MaximeZmt merged commit 4bc6aed into main Mar 31, 2022
@MaximeZmt MaximeZmt deleted the maximezmt/ui branch March 31, 2022 08:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor main UI from the app + design logo (based on current draft) (#115)
3 participants