Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add timetable detail screen Base #102

Merged
merged 10 commits into from
Sep 3, 2022
Merged

Conversation

cOnigashima
Copy link
Contributor

@cOnigashima cOnigashima commented Sep 2, 2022

Issue

  • close #ISSUE_NUMBER

Overview (Required)

  • add transition from sessions screen on item clicked
  • add timetable detail screen

Not yet

//TODO comment Added

  • design
  • ViewModel injection
  • connection data layer
  • module dependency problem
  • Bottom Navigation View
  • Back to sessions by navigation click

Links

Screenshot

After

@@ -118,6 +122,12 @@ class KaigiAppScaffoldState @OptIn(ExperimentalMaterial3Api::class) constructor(
}
}

fun onTimeTableClick(timetableId : TimetableItemId){
Copy link

Choose a reason for hiding this comment

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

[spotless] reported by reviewdog 🐶

Suggested change
fun onTimeTableClick(timetableId : TimetableItemId){
fun onTimeTableClick(timetableId: TimetableItemId) {

import kotlinx.datetime.TimeZone
import kotlinx.datetime.toInstant


Copy link

Choose a reason for hiding this comment

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

[spotless] reported by reviewdog 🐶

Suggested change

}
}

fun fakeTimetableDetail() = Session(
Copy link

Choose a reason for hiding this comment

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

[spotless] reported by reviewdog 🐶

Suggested change
fun fakeTimetableDetail() = Session(
fun fakeTimetableDetail() = Session(

"INTERMEDIATE",
).toPersistentList(),
)

Copy link

Choose a reason for hiding this comment

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

[spotless] reported by reviewdog 🐶

Suggested change

import androidx.compose.ui.layout.ContentScale
import androidx.compose.ui.tooling.preview.Preview
import androidx.compose.ui.unit.dp
import io.github.droidkaigi.confsched2022.designsystem.theme.KaigiScaffold
Copy link

Choose a reason for hiding this comment

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

[spotless] reported by reviewdog 🐶

Suggested change
import io.github.droidkaigi.confsched2022.designsystem.theme.KaigiScaffold
import coil.compose.AsyncImage
import io.github.droidkaigi.confsched2022.designsystem.theme.KaigiScaffold

Comment on lines 75 to 77
TimetableDetailDescription(
description = item.description
)
Copy link

Choose a reason for hiding this comment

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

[spotless] reported by reviewdog 🐶

Suggested change
TimetableDetailDescription(
description = item.description
)
TimetableDetailDescription(
description = item.description
)

Comment on lines 102 to 103
language : String,
levels : PersistentList<String>,
Copy link

Choose a reason for hiding this comment

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

[spotless] reported by reviewdog 🐶

Suggested change
language : String,
levels : PersistentList<String>,
language: String,
levels: PersistentList<String>,


Text(
modifier = modifier,
text = "$startsAt $endsAt" ,
Copy link

Choose a reason for hiding this comment

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

[spotless] reported by reviewdog 🐶

Suggested change
text = "$startsAt $endsAt" ,
text = "$startsAt $endsAt",

Spacer(
modifier = modifier.padding(horizontal = 16.dp),
)
//TODO ClickableにしてSpeaker詳細へ遷移
Copy link

Choose a reason for hiding this comment

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

[spotless] reported by reviewdog 🐶

Suggested change
//TODO ClickableにしてSpeaker詳細へ遷移
// TODO ClickableにしてSpeaker詳細へ遷移

uiModel = TimeTableDetailUiModel(Loaded(fakeTimetableDetail()))
)
}

Copy link

Choose a reason for hiding this comment

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

[spotless] reported by reviewdog 🐶

Suggested change

@@ -43,6 +43,7 @@ dependencies {
implementation(projects.coreData)
implementation(projects.coreDesignsystem)
implementation(projects.coreUi)
implementation(projects.coreModel)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this dependency is bad ... but I don't know what to do

Copy link
Member

Choose a reason for hiding this comment

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

Is this for TimetableItemId? I think this is not so bad. Because the model is used everywhere in this project.

Copy link
Contributor Author

@cOnigashima cOnigashima Sep 3, 2022

Choose a reason for hiding this comment

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

Is this for TimetableItemId?

That's right.

the model is used everywhere in this project.

I see. Thank you very much!!

composable(route = SessionsNavGraph.sessionRoute) {
SessionsScreenRoot(onNavigationIconClick = onNavigationIconClick)
SessionsScreenRoot(
Copy link
Contributor Author

@cOnigashima cOnigashima Sep 2, 2022

Choose a reason for hiding this comment

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

[Q]
Should I make another NavGraph Class , and move them ?

Copy link
Member

Choose a reason for hiding this comment

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

I think it is ok. 👍

}

@Composable
fun TimetableDetailDescription(
Copy link
Member

Choose a reason for hiding this comment

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

👍

"INTERMEDIATE",
).toPersistentList(),
)
}
Copy link

Choose a reason for hiding this comment

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

[spotless] reported by reviewdog 🐶

Suggested change
}
}

Copy link
Member

@takahirom takahirom left a comment

Choose a reason for hiding this comment

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

Let's merge first

@cOnigashima
Copy link
Contributor Author

Thanks for fixing!

@cOnigashima cOnigashima merged commit 90729dd into main Sep 3, 2022
@cOnigashima cOnigashima deleted the add_timetable_detail_screen branch September 3, 2022 11:42
@takahirom
Copy link
Member

🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants