Beautify show detail page #62
Conversation
|
Dear @u3frajaeian, |
|
And we do do not need play button or navigating to player from shows detail page. You can remove it. (Episodes should have play button not Show details) |
Hi it's already updated |
Hi, I think we must have an intro for the series, and this button should play something like a trailer or intro. |
But the conflicts are not resolved yet.
Yes, we could consider having a "Trailers" button, But Using a play icon without accompanying text for different functionalities can be confusing. The button's label should clearly indicate its purpose so that users can easily understand its function. Also that is outside the scope of this issue. and if we want to add that button, it should navigate to another screen since Shows and movies usually have more than one trailers. |
Dear Reza, It appears that everything is already updated. After fetching the updated branches and attempting to merge remote/dev with my branch, I received a message stating: "Already up to date." |
Yes, you are right. I completely misunderstood the function of this button. I assumed the play button was responsible for showing trailers because Filmtime isn't a streaming application. However, based on your description, I will remove it. |
Hi @u3frajaeian, The message indicates that the branch is updated because it is being compared with your dev branch on your repository. When you create a PR against the original repository you forked from, you must sync your branch with the latest changes before creating the PR. Currently, there are many conflicts, as shown in the screenshot I sent you earlier. To have a better overview of what I am telling, take a look at this screenshot: 
SolutionI suggest you follow one of the options from this document and sync your repo. |
There was a problem hiding this comment.
Dear @u3frajaeian,
Thank you for your contribution. It appears that syncing your repository with moallemi/Film-Time did not go smoothly, resulting in some of our changes being reverted.
Considering the significant effort and time you've put into this PR, I will manually merge it this time. However, for future contributions, please ensure your PR is synced with the latest version of this repository to avoid such conflicts.
Best regards,
There was a problem hiding this comment.
We already have this file in movie-detail module
| import io.filmtime.data.model.VideoDetail | ||
|
|
||
| data class ShowDetailState( | ||
| val isLoading: Boolean = false, | ||
| val videoDetail: VideoDetail? = null, | ||
| val message: String? = null, | ||
| val streamInfo: StreamInfo? = null, |
There was a problem hiding this comment.
show does not need streamInfo since the show itself is not playable.
| }, | ||
| ) | ||
| } | ||
| } |
| @@ -10,6 +10,10 @@ import io.filmtime.core.ui.common.DestinationRoute | |||
|
|
|||
| fun NavGraphBuilder.showDetailScreen( | |||
| rootRoute: DestinationRoute, | |||
| onStreamReady: (DestinationRoute, streamUrl: String) -> Unit, | |||
There was a problem hiding this comment.
We do not need streamReady for shows
| Text( | ||
| modifier = Modifier.padding(horizontal = 16.dp), | ||
| style = TextStyle( | ||
| fontWeight = FontWeight.Bold, | ||
| fontSize = 16.sp, | ||
| color = Color.Black, | ||
| ), | ||
| text = "Similar", |
There was a problem hiding this comment.
It seems that you have trouble with fixing merge conflicts. Because using VideoSectionRow is the new approach we used for displaying similar items. Please do not revert the changes to previous state.
Add two sections in the show detail page to represent similar, casts