JSAB2-75 Leaderboard #16
Conversation
|
Suggestion: add scrollView to leaderboard part. So no matter the size of the screen, user can still check all 10 first players |
frontend/src/Leaderboard/index.jsx
Outdated
|
|
||
| function Leaderboard() { | ||
| return ( | ||
| <> |
There was a problem hiding this comment.
Maybe we can use instead of <> ---> Just advice
| }, | ||
| }); | ||
|
|
||
| function RowTopTen({ |
There was a problem hiding this comment.
RowTopTen is not a lucky name, it's not fixed that we want to render the first 10 users, it can change. The component has nothing to do with how many do we want to render. Now we only render 3 of them. Please rename it to something more general.
frontend/src/Leaderboard/index.jsx
Outdated
| function Leaderboard() { | ||
| return ( | ||
| <ScrollView> | ||
| <RowTopTen rank={1} username="fdas" gold={20} crown={10} /> |
There was a problem hiding this comment.
please don't hardcode ranking, username, gold, and crown (which should be kingdoms, or amountOfGold and amountOfKingdoms)
create a mocked user list which contains users with the amount of gold, and kingdoms, and map through that array
(crown is just the icon, it represents the amount of kingdoms)
you don't have to deal with the logic of sorting, you can create an already sorted mocked list and just map through it.
ranking (1,2,3,4,...) should not be hardcoded, calculate it somehow please
| <RowTopTen rank={1} username="fdas" gold={20} crown={10} /> | |
| // add something like this with a few users and map through it | |
| let users = [ | |
| { | |
| username: 'user1', | |
| gold: 100, | |
| kingdoms: 1 | |
| }, | |
| { | |
| username: 'user2', | |
| gold: 200, | |
| kingdoms: 2 | |
| }, | |
| { | |
| username: 'user with a very long username', | |
| gold: 300, | |
| kingdoms: 3 | |
| }, | |
| ] |
| <View style={styles.rowFlex}> | ||
| <Text style={styles.text}>{rank}</Text> | ||
| <Image style={styles.userAvatar} source={userAvatar} /> | ||
| <Text style={styles.text}>{username}</Text> |
There was a problem hiding this comment.
if the username is too long, the amount of gold won't be seen. Please fix this. For this you'll need to change on the styling just a little bit, give the text a width, and because of this you can't use the text styling for all of the texts here.
Handling too long text is really easy in React Native, it's only adding one attribute to the Text:) let me know if you need help
| </View> | ||
| <View style={styles.userInfo}> | ||
| <Image source={crownIcon} style={styles.icon} /> | ||
| <Text style={styles.text}>{crown}</Text> |
There was a problem hiding this comment.
| <Text style={styles.text}>{crown}</Text> | |
| <Text style={styles.text}>{kingdom}</Text> |
| }); | ||
|
|
||
| function RowTopTen({ | ||
| rank, username, gold, crown, |
There was a problem hiding this comment.
crown is the icon name, but it represents the kingdom, please rename
There was a problem hiding this comment.
@GuJialu naming --> amountOfGold, amountOfKingdoms, or something similar
Co-Authored-By: evasimonyi <30794349+evasimonyi@users.noreply.github.com>
Co-Authored-By: evasimonyi <30794349+evasimonyi@users.noreply.github.com>
Co-Authored-By: evasimonyi <30794349+evasimonyi@users.noreply.github.com>
Co-Authored-By: evasimonyi <30794349+evasimonyi@users.noreply.github.com>
No description provided.