IQBall
/
Application-Android
6
0
Fork 0
Code Issues 28 Pull Requests Packages Projects 1 Releases Wiki Activity
Labels Milestones

home page #30

Merged
maxime.batista merged 8 commits from home into master 1 year ago
Conversation 0 Commits 8 Files Changed 14
yanis.dahmane-bounoua commented 1 year ago
Owner

This pull request adds the home page.

Currently, you cannot visualize tactics when you click on them. There isn't the tactic's preview available because it's required the visualizer.

This pull request adds the home page. Currently, you cannot visualize tactics when you click on them. There isn't the tactic's preview available because it's required the visualizer.
mael.daim was assigned by yanis.dahmane-bounoua 1 year ago
maxime.batista was assigned by yanis.dahmane-bounoua 1 year ago
samuel.berion was assigned by yanis.dahmane-bounoua 1 year ago
vivien.dufour was assigned by yanis.dahmane-bounoua 1 year ago
yanis.dahmane-bounoua added 4 commits 1 year ago
maxime.batista requested changes 1 year ago
maxime.batista left a comment

Please always mind of formatting your code (ctrl + alt + L)

Please always mind of formatting your code (ctrl + alt + L)
app/src/main/java/com/iqball/app/MainActivity.kt Outdated
IQBallTheme {
maxime.batista commented 1 year ago 
- + IQBallTheme {
+ IQBallTheme(darkTheme = false, dynamicColor = false) {

Prone use of the dark theme because it can use it if the phone's system theme is set in night mode.
Don't bother with a night mode for now and just force it to use the light theme.

Also remove the dynamicColor feature as it appears to change colors based on the user's system theme preferences

```diff - + IQBallTheme { + IQBallTheme(darkTheme = false, dynamicColor = false) { ``` Prone use of the dark theme because it can use it if the phone's system theme is set in night mode. Don't bother with a night mode for now and just force it to use the light theme. Also remove the dynamicColor feature as it appears to change colors based on the user's system theme preferences
yanis.dahmane-bounoua marked this conversation as resolved
app/src/main/java/com/iqball/app/MainActivity.kt Outdated
val auth = result.getOrNull()!!
Log.d("test", "test")
service.getUserData(auth.token)
}
maxime.batista commented 1 year ago 
-	    runBlocking {
-        val result = service.login(AuthService.LoginRequest("yanis@mail.com", "123456"))
-
-        val auth = result.getOrNull()!!
-        Log.d("test", "test")
-        service.getUserData(auth.token)
-    }
```diff - runBlocking { - val result = service.login(AuthService.LoginRequest("yanis@mail.com", "123456")) - - val auth = result.getOrNull()!! - Log.d("test", "test") - service.getUserData(auth.token) - } ```
yanis.dahmane-bounoua marked this conversation as resolved
app/src/main/java/com/iqball/app/model/Team.kt Outdated
package com.iqball.app.model;
class Team public constructor(
maxime.batista commented 1 year ago

use a data class here as you done with Tactic

use a data class here as you done with `Tactic`
yanis.dahmane-bounoua marked this conversation as resolved
app/src/main/java/com/iqball/app/page/HomePage.kt Outdated
}
@Composable
fun Body(padding: PaddingValues, tactics: List<Tactic>, teams: List<Team>, invalid: Boolean) {
maxime.batista commented 1 year ago 
- fun Body(padding: PaddingValues, tactics: List<Tactic>, teams: List<Team>, invalid: Boolean) {
+ private fun Body(padding: PaddingValues, tactics: List<Tactic>, teams: List<Team>, invalid: Boolean) {
```diff - fun Body(padding: PaddingValues, tactics: List<Tactic>, teams: List<Team>, invalid: Boolean) { + private fun Body(padding: PaddingValues, tactics: List<Tactic>, teams: List<Team>, invalid: Boolean) { ```
yanis.dahmane-bounoua marked this conversation as resolved
app/src/main/java/com/iqball/app/page/HomePage.kt Outdated
}
@Composable
fun TabButtons(isTactics: Boolean, onToggle: (Boolean) -> Unit) {
maxime.batista commented 1 year ago 
- fun TabButtons(isTactics: Boolean, onToggle: (Boolean) -> Unit) {
+ private fun TabButtons(isTactics: Boolean, onToggle: (Boolean) -> Unit) {
```diff - fun TabButtons(isTactics: Boolean, onToggle: (Boolean) -> Unit) { + private fun TabButtons(isTactics: Boolean, onToggle: (Boolean) -> Unit) { ```
yanis.dahmane-bounoua marked this conversation as resolved
app/src/main/java/com/iqball/app/page/HomePage.kt Outdated
modifier = Modifier
.padding(top = 10.dp, start = 2.dp, end = 2.dp)
.fillMaxWidth()
.padding(top = 10.dp, bottom = 10.dp)
maxime.batista commented 1 year ago 
-            .padding(top = 10.dp, start = 2.dp, end = 2.dp)
+            .padding(top = 10.dp, start = 2.dp, end = 2.dp, bottom = 10.dp)
             .fillMaxWidth()
-            .padding(top = 10.dp, bottom = 10.dp)
```diff - .padding(top = 10.dp, start = 2.dp, end = 2.dp) + .padding(top = 10.dp, start = 2.dp, end = 2.dp, bottom = 10.dp) .fillMaxWidth() - .padding(top = 10.dp, bottom = 10.dp) ```
yanis.dahmane-bounoua marked this conversation as resolved
app/src/main/java/com/iqball/app/page/HomePage.kt Outdated
) {
val tacticTitle = "Espace personnel"
val teamTitle = "Mes équipes"
maxime.batista commented 1 year ago 
           val tacticsTabTitle = "Espace personnel"
           val teamsTabTitle = "Mes équipes"

You can also inline them but this is also OK. Just use the appropriate names if you want to be explicit on what those strings are for.
Best would be to place them in private consts

```diff val tacticsTabTitle = "Espace personnel" val teamsTabTitle = "Mes équipes" ``` You can also inline them but this is also OK. Just use the appropriate names if you want to be explicit on what those strings are for. Best would be to place them in private consts
yanis.dahmane-bounoua marked this conversation as resolved
app/src/main/java/com/iqball/app/page/HomePage.kt
val tacticTitle = "Espace personnel"
val teamTitle = "Mes équipes"
val spaceButton = 5.dp
if (isTactics) {
maxime.batista commented 1 year ago

ew. a component should be (in best cases) independant from the context of its usage. There are rare case where you can break this, but here you can reduce the code length and make it more understandable :
See the tabs component as a list of tabs :

@Composable
TabsSelector(tabsTitles: List<String>, selectedIndex: MutableState<Int>) {
	Row(...) {
		
        for ((idx, tab) in tabsTitles.withIndex()) {
            TabButton(
                tab,
                fill = idx == selectedIndex.value,
                onClick = { selectedIndex.value = idx }
            )
        }
        
	}
}

@Composable
fun TabButton(title: String, fill: Boolean, onClick: () -> Unit) {
    val scheme = MaterialTheme.colorScheme
    Button(
        border = BorderStroke(
            1.dp,
            color = scheme.tertiary
        ),
        colors = ButtonDefaults.buttonColors(
            containerColor = if (fill) scheme.tertiary else scheme.background,
            contentColor = if (fill) scheme.background else scheme.tertiary,
        ),
        onClick = onClick) {
        Text(title)
    }
}

You can then remove OutlinedButton and FilledButton components, and perform the changes in Body:

@Composable
fun Body(padding: PaddingValues, tactics: List<Tactic>, teams: List<Team>, invalid: Boolean) {
    Column(
        modifier = Modifier
            .padding(padding)
    ) {
        val selectedTab = remember { mutableIntStateOf(0) }
        val tabs = listOf<Pair<String, @Composable () -> Unit>>(
            Pair("Espace personnel") {
                ListTacticCard(tactics)
            },
            Pair("Mes Equipes") {
                ListTeamCard(teams)
            }
        )
        TabsSelector(tabsTitles = tabs.map { it.first }, selectedIndex = selectedTab)

        if (!invalid) {
            tabs[selectedTab.intValue].second()
            return 
        }
        
        TextCentered(
            text = "Erreur : Aucune connexion internet. Veillez activer votre connexion internet puis relancer l'application",
            fontSize = 20.sp
        )
    }
}
ew. a component should be (in best cases) independant from the context of its usage. There are rare case where you can break this, but here you can reduce the code length and make it more understandable : See the tabs component as a list of tabs : ```kt @Composable TabsSelector(tabsTitles: List<String>, selectedIndex: MutableState<Int>) { Row(...) { for ((idx, tab) in tabsTitles.withIndex()) { TabButton( tab, fill = idx == selectedIndex.value, onClick = { selectedIndex.value = idx } ) } } } @Composable fun TabButton(title: String, fill: Boolean, onClick: () -> Unit) { val scheme = MaterialTheme.colorScheme Button( border = BorderStroke( 1.dp, color = scheme.tertiary ), colors = ButtonDefaults.buttonColors( containerColor = if (fill) scheme.tertiary else scheme.background, contentColor = if (fill) scheme.background else scheme.tertiary, ), onClick = onClick) { Text(title) } } ``` You can then remove `OutlinedButton` and `FilledButton` components, and perform the changes in `Body`: ```kt @Composable fun Body(padding: PaddingValues, tactics: List<Tactic>, teams: List<Team>, invalid: Boolean) { Column( modifier = Modifier .padding(padding) ) { val selectedTab = remember { mutableIntStateOf(0) } val tabs = listOf<Pair<String, @Composable () -> Unit>>( Pair("Espace personnel") { ListTacticCard(tactics) }, Pair("Mes Equipes") { ListTeamCard(teams) } ) TabsSelector(tabsTitles = tabs.map { it.first }, selectedIndex = selectedTab) if (!invalid) { tabs[selectedTab.intValue].second() return } TextCentered( text = "Erreur : Aucune connexion internet. Veillez activer votre connexion internet puis relancer l'application", fontSize = 20.sp   ) } } ```
yanis.dahmane-bounoua marked this conversation as resolved
app/src/main/java/com/iqball/app/page/HomePage.kt
}
@Composable
fun ListTacticCard(tactics: List<Tactic>) {
maxime.batista commented 1 year ago 
- fun ListTacticCard(tactics: List<Tactic>) {
+ private fun ListTacticCard(tactics: List<Tactic>) {
```diff - fun ListTacticCard(tactics: List<Tactic>) { + private fun ListTacticCard(tactics: List<Tactic>) { ```
yanis.dahmane-bounoua marked this conversation as resolved
app/src/main/java/com/iqball/app/page/HomePage.kt Outdated
}
}
)
}
maxime.batista commented 1 year ago

DRY !

@Composable
fun ListTacticCard(tactics: List<Tactic>) {
    LazyVerticalStaggeredGrid(
        columns = StaggeredGridCells.Fixed(2),
        modifier = Modifier
            .padding(5.dp),
        content = {
            items(tactics) { tactic ->
                TacticCard(tactic)
            }
        }
    )
}

@Composable
fun ListTeamCard(teams: List<Team>) {
    LazyVerticalStaggeredGrid(
        columns = StaggeredGridCells.Fixed(2),
        modifier = Modifier
            .padding(5.dp),
        content = {
            items(teams) { team ->
                TeamCard(team)
            }
        }
    )
}

Can be factorized :

@Composable
fun <A> ComponentGrid(items: List<A>, component: @Composable (A) -> Unit) {
    LazyVerticalStaggeredGrid(
        columns = StaggeredGridCells.Fixed(2),
        modifier = Modifier
            .padding(5.dp),
        content = {
            items(items) { tactic ->
                component(tactic)
            }
        }
    )
}
DRY ! ```kt @Composable fun ListTacticCard(tactics: List<Tactic>) { LazyVerticalStaggeredGrid( columns = StaggeredGridCells.Fixed(2), modifier = Modifier .padding(5.dp), content = { items(tactics) { tactic -> TacticCard(tactic) } } ) } @Composable fun ListTeamCard(teams: List<Team>) { LazyVerticalStaggeredGrid( columns = StaggeredGridCells.Fixed(2), modifier = Modifier .padding(5.dp), content = { items(teams) { team -> TeamCard(team) } } ) } ``` Can be factorized : ```kt @Composable fun <A> ComponentGrid(items: List<A>, component: @Composable (A) -> Unit) { LazyVerticalStaggeredGrid( columns = StaggeredGridCells.Fixed(2), modifier = Modifier .padding(5.dp), content = { items(items) { tactic -> component(tactic) } } ) } ```
yanis.dahmane-bounoua marked this conversation as resolved
app/src/main/java/com/iqball/app/page/HomePage.kt
}
@Composable
fun TacticCard(tactic: Tactic) {
maxime.batista commented 1 year ago 
- fun TacticCard(tactic: Tactic) {
+ private fun TacticCard(tactic: Tactic) {
```diff - fun TacticCard(tactic: Tactic) { + private fun TacticCard(tactic: Tactic) { ```
yanis.dahmane-bounoua marked this conversation as resolved
app/src/main/java/com/iqball/app/page/HomePage.kt Outdated
)
.shadow(1.dp, shape = RoundedCornerShape(8.dp))
.background(
color = Color(0xFFFFFFFF)
maxime.batista commented 1 year ago

try to place the hardcoded colors in Colors.kt at least (in the theme could be better)
at line 195 and 200

try to place the hardcoded colors in `Colors.kt` at least (in the theme could be better) at line 195 and 200
yanis.dahmane-bounoua marked this conversation as resolved
app/src/main/java/com/iqball/app/page/HomePage.kt Outdated
}
@Composable
fun TeamCard(team: Team) {
maxime.batista commented 1 year ago 
- fun TeamCard(team: Team) {
+ private fun TeamCard(team: Team) {
```diff - fun TeamCard(team: Team) { + private fun TeamCard(team: Team) { ```
yanis.dahmane-bounoua marked this conversation as resolved
app/src/main/java/com/iqball/app/page/HomePage.kt
}
@Composable
fun TeamColorCard(text: String, color: Color, fraction : Float = 1f) {
maxime.batista commented 1 year ago 
- fun TeamColorCard(text: String, color: Color, fraction : Float = 1f) {
+ private fun TeamColorCard(text: String, color: Color, fraction : Float = 1f) {
```diff - fun TeamColorCard(text: String, color: Color, fraction : Float = 1f) { + private fun TeamColorCard(text: String, color: Color, fraction : Float = 1f) { ```
yanis.dahmane-bounoua marked this conversation as resolved
app/src/main/java/com/iqball/app/page/HomePage.kt
}
@Composable
fun FilledButton(text: String, onClick: () -> Unit) {
maxime.batista commented 1 year ago 
- fun FilledButton(text: String, onClick: () -> Unit) {
+ private fun FilledButton(text: String, onClick: () -> Unit) {
```diff - fun FilledButton(text: String, onClick: () -> Unit) { + private fun FilledButton(text: String, onClick: () -> Unit) { ```
yanis.dahmane-bounoua marked this conversation as resolved
app/src/main/java/com/iqball/app/page/HomePage.kt
}
@Composable
fun OutlinedButton(text: String, onClick: () -> Unit) {
maxime.batista commented 1 year ago 
- fun OutlinedButton(text: String, onClick: () -> Unit) {
+ private fun OutlinedButton(text: String, onClick: () -> Unit) {
```diff - fun OutlinedButton(text: String, onClick: () -> Unit) { + private fun OutlinedButton(text: String, onClick: () -> Unit) { ```
yanis.dahmane-bounoua marked this conversation as resolved
app/src/main/java/com/iqball/app/page/HomePage.kt
}
@Composable
fun TextCentered( modifier: Modifier = Modifier, text: String, fontSize: TextUnit = 18.sp, fontWeight: FontWeight? = null) {
maxime.batista commented 1 year ago 
- fun TextCentered( modifier: Modifier = Modifier, text: String, fontSize: TextUnit = 18.sp, fontWeight: FontWeight? = null) {
+ private fun TextCentered( modifier: Modifier = Modifier, text: String, fontSize: TextUnit = 18.sp, fontWeight: FontWeight? = null) {
```diff - fun TextCentered( modifier: Modifier = Modifier, text: String, fontSize: TextUnit = 18.sp, fontWeight: FontWeight? = null) { + private fun TextCentered( modifier: Modifier = Modifier, text: String, fontSize: TextUnit = 18.sp, fontWeight: FontWeight? = null) { ```
yanis.dahmane-bounoua marked this conversation as resolved
app/src/main/java/com/iqball/app/page/HomePage.kt Outdated
)
}
fun getDataFromApi(service: IQBallService) : UserService.UserDataResponse?{
maxime.batista commented 1 year ago 
- fun getDataFromApi(service: IQBallService) : UserService.UserDataResponse?{
+ private fun getDataFromApi(service: IQBallService) : UserService.UserDataResponse?{
```diff - fun getDataFromApi(service: IQBallService) : UserService.UserDataResponse?{ + private fun getDataFromApi(service: IQBallService) : UserService.UserDataResponse?{ ```
yanis.dahmane-bounoua marked this conversation as resolved
app/src/main/java/com/iqball/app/page/HomePage.kt Outdated
)
return tactics
}
maxime.batista commented 1 year ago

You should remove getStubTactic/getStubTeams functions as you now get the data from the API.

You should remove `getStubTactic/getStubTeams` functions as you now get the data from the API.
yanis.dahmane-bounoua commented 1 year ago
Poster
Owner

That can be usefull later for testing

That can be usefull later for testing
yanis.dahmane-bounoua commented 1 year ago
Poster
Owner

That can be usefull later for testing

That can be usefull later for testing
maxime.batista commented 1 year ago

You should move them in a stub file at least

You should move them in a stub file at least
maxime.batista reviewed 1 year ago
app/Home.kt
maxime.batista commented 1 year ago

Remove this file

Remove this file
yanis.dahmane-bounoua marked this conversation as resolved
yanis.dahmane-bounoua requested review from maxime.batista 1 year ago
yanis.dahmane-bounoua added 1 commit 1 year ago
maxime.batista requested changes 1 year ago
app/src/main/java/com/iqball/app/page/HomePage.kt Outdated
Commentaire
=======================================================
Gérer les listes à afficher avec des paires n'est pas forcément la meilleure chose à faire dans le contexte de composable.
maxime.batista commented 1 year ago

No french !!!!!

No french !!!!!
yanis.dahmane-bounoua added 1 commit 1 year ago
yanis.dahmane-bounoua added 1 commit 1 year ago
yanis.dahmane-bounoua added 1 commit 1 year ago
maxime.batista force-pushed home from 47311389bd to 7aab2a0d08 1 year ago
maxime.batista merged commit f813d5053c into master 1 year ago

Reviewers

maxime.batista requested changes 1 year ago
The pull request has been merged as f813d5053c.
Sign in to join this conversation.
Reviewers
Request review
No reviewers
maxime.batista
Labels
Apply labels
Clear labels
bug
Something is not working duplicate
This issue or pull request already exists enhancement
New feature help wanted
Need some help invalid
Something is wrong question
More information is needed wontfix
This won't be fixed
No Label bug duplicate enhancement help wanted invalid question wontfix
Milestone
Set milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Assign users
Clear assignees
No Assignees
mael.daim
samuel.berion
vivien.dufour
2 Participants
Notifications
Due Date

No due date set.

Dependencies

No dependencies set.

Reference: IQBall/Application-Android#30
Write Preview
Loading…
Cancel
Save
There is no content yet.
Delete Branch 'home'

Deleting a branch is permanent. It CANNOT be undone. Continue?

No
Yes