Enhance Movie Detail Screen Experience#176
Conversation
- Improve similar movie recommendations - Add favorite actors connection groundwork - Implement actor filmography sorting by rating - Add review filtering and spoiler detection - Add smart release status labels
There was a problem hiding this comment.
Code Review
This pull request introduces several enhancements to the MovieDB app, including sorting cast and crew movies, updating the share text format, filtering out adult content and missing posters from similar movies, and adding a spoiler warning system for reviews. It also updates the movie adapter to label releases as 'Upcoming' or 'New'. However, several critical issues were identified during the review: ReviewAdapter.kt contains compilation errors due to an unclosed post block and a duplicated ReviewDiffCallback class; HomeFragment.kt has a keyboard smash comment that breaks movie response handling and uses the wrong handler for upcoming TV shows; and NowPlayingMovieAdapter.kt has a logic bug where future releases less than 24 hours away are incorrectly labeled as 'New' instead of 'Upcoming'.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| // Set content with spoiler warning, max lines and click listener | ||
| val content = review.optString("content") | ||
| binding.textViewContent.text = content | ||
| binding.textViewContent.post { | ||
| if (binding.textViewContent.lineCount > 3) { | ||
| binding.textViewContent.maxLines = 3 | ||
| isContentExpanded = false | ||
| } else { | ||
| isContentExpanded = true | ||
| } | ||
| val isSpoiler = content.startsWith("⚠️ Possible spoiler") | ||
|
|
||
| if (isSpoiler) { | ||
| val cleanedContent = content | ||
| .replace("⚠️ Possible spoiler", "") | ||
| .trim() | ||
|
|
||
| binding.textViewContent.text = "⚠️ SPOILER WARNING\n\n$cleanedContent" | ||
| binding.textViewContent.setTypeface(null, Typeface.BOLD) | ||
| binding.textViewContent.alpha = 0.9f | ||
| } else { | ||
| binding.textViewContent.text = content | ||
| binding.textViewContent.setTypeface(null, Typeface.NORMAL) | ||
| binding.textViewContent.alpha = 1.0f | ||
| } | ||
|
|
||
| binding.textViewContent.post { | ||
|
|
||
| binding.textViewContent.setOnClickListener { |
There was a problem hiding this comment.
The post block on textViewContent is left unclosed, which causes a compilation error. Additionally, the original logic to initialize isContentExpanded and set maxLines based on the line count was removed. Let's restore the block and close it properly.
// Set content with spoiler warning, max lines and click listener
val content = review.optString("content")
val isSpoiler = content.startsWith("⚠️ Possible spoiler")
if (isSpoiler) {
val cleanedContent = content
.replace("⚠️ Possible spoiler", "")
.trim()
binding.textViewContent.text = "⚠️ SPOILER WARNING\n\n$cleanedContent"
binding.textViewContent.setTypeface(null, Typeface.BOLD)
binding.textViewContent.alpha = 0.9f
} else {
binding.textViewContent.text = content
binding.textViewContent.setTypeface(null, Typeface.NORMAL)
binding.textViewContent.alpha = 1.0f
}
binding.textViewContent.post {
if (binding.textViewContent.lineCount > 3) {
binding.textViewContent.maxLines = 3
isContentExpanded = false
} else {
isContentExpanded = true
}
}
binding.textViewContent.setOnClickListener {| return oldItem.getString("id") == newItem.getString("id") | ||
| } | ||
|
|
||
| override fun areContentsTheSame(oldItem: JSONObject, newItem: JSONObject): Boolean { | ||
| return oldItem.toString() == newItem.toString() | ||
| } | ||
| } | ||
| } | ||
|
|
||
| class ReviewDiffCallback : DiffUtil.ItemCallback<JSONObject>() { | ||
| override fun areItemsTheSame(oldItem: JSONObject, newItem: JSONObject): Boolean { | ||
| return oldItem.optString("id") == newItem.optString("id") | ||
| } | ||
|
|
||
| override fun areContentsTheSame(oldItem: JSONObject, newItem: JSONObject): Boolean { | ||
| return oldItem.toString() == newItem.toString() | ||
| } |
There was a problem hiding this comment.
There is a duplicated definition of ReviewDiffCallback and broken closing braces at the end of the file, which prevents compilation. Let's clean this up to have a single, correctly closed ReviewDiffCallback nested class.
| return oldItem.getString("id") == newItem.getString("id") | |
| } | |
| override fun areContentsTheSame(oldItem: JSONObject, newItem: JSONObject): Boolean { | |
| return oldItem.toString() == newItem.toString() | |
| } | |
| } | |
| } | |
| class ReviewDiffCallback : DiffUtil.ItemCallback<JSONObject>() { | |
| override fun areItemsTheSame(oldItem: JSONObject, newItem: JSONObject): Boolean { | |
| return oldItem.optString("id") == newItem.optString("id") | |
| } | |
| override fun areContentsTheSame(oldItem: JSONObject, newItem: JSONObject): Boolean { | |
| return oldItem.toString() == newItem.toString() | |
| } | |
| return oldItem.optString("id") == newItem.optString("id") | |
| } | |
| override fun areContentsTheSame(oldItem: JSONObject, newItem: JSONObject): Boolean { | |
| return oldItem.toString() == newItem.toString() | |
| } |
| withContext(Dispatchers.Main) { | ||
| if (isAdded && !response.isNullOrEmpty()) { | ||
| handleMovieResponse(response) | ||
| //SDFKLNHGJKSHKDJFGHSJKLGHDFJKFDGHKJFDHJKFGDHJGIDFK |
There was a problem hiding this comment.
The keyboard smash comment completely disables handling the movie response for now playing movies, leaving the UI in a permanent shimmer state when data is successfully fetched. Let's restore the correct call to handleMovieResponse(response).
| //SDFKLNHGJKSHKDJFGHSJKLGHDFJKFDGHKJFDHJKFGDHJGIDFK | |
| handleMovieResponse(response) |
| withContext(Dispatchers.Main) { | ||
| if (isAdded && !response.isNullOrEmpty()) { | ||
| handleUpcomingTVResponse(response) | ||
| handleMovieResponse(response) |
There was a problem hiding this comment.
In fetchUpcomingTVShows(), calling handleMovieResponse(response) instead of handleUpcomingTVResponse(response) causes TV show data to be loaded into the movie adapter/recycler view, overriding the now playing movies and failing to hide the shimmer for upcoming TV shows. Let's restore the correct handler.
| handleMovieResponse(response) | |
| handleUpcomingTVResponse(response) |
| holder.binding.date.text = when { | ||
| diffInDays > 0 -> "Upcoming • $formattedDate" | ||
| diffInDays in -30..0 -> "New • $formattedDate" | ||
| else -> formattedDate | ||
| } |
There was a problem hiding this comment.
Because of integer division and the time component of today, any future release date that is less than 24 hours away will result in diffInDays being 0, causing it to be incorrectly labeled as New instead of Upcoming. Using releaseDate.after(today) directly resolves this issue.
| holder.binding.date.text = when { | |
| diffInDays > 0 -> "Upcoming • $formattedDate" | |
| diffInDays in -30..0 -> "New • $formattedDate" | |
| else -> formattedDate | |
| } | |
| holder.binding.date.text = when { | |
| releaseDate.after(today) -> "Upcoming • $formattedDate" | |
| diffInDays >= -30 -> "New • $formattedDate" | |
| else -> formattedDate | |
| } |
Implemented the following improvements: