Vis - #185
Conversation
There was a problem hiding this comment.
🟡 Not ready to approve
There are confirmed navigation/URL-construction bugs (invalid query strings, potential “no visible tabs” state, and redundant fragment transactions) that should be fixed before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
This PR extends the app’s settings-driven UI/network behavior by adding an “Include adult content” preference (propagated into TMDB requests) and expanding tab-visibility controls to allow hiding the Home tab, alongside a small UI action/menu reshuffle.
Changes:
- Added a settings toggle to include/exclude adult content and appended
include_adultto multiple TMDB request URLs. - Added a “Hide Home tab” preference and updated bottom navigation initialization/visibility handling in
MainActivity. - Swapped “Tags” vs “Stats” entry points between the FAB/menu, and introduced a new vector drawable icon.
File summaries
| File | Description |
|---|---|
| app/src/main/res/xml/preferences.xml | Adds “Include adult content” switch and a new “Hide Home tab” checkbox under tab visibility. |
| app/src/main/res/values/strings.xml | Adds strings for adult-content toggle and tab-visibility explanatory text + Home-tab label. |
| app/src/main/res/menu/database_menu.xml | Renames the menu item id/title from tags to stats. |
| app/src/main/res/drawable/ic_sell.xml | Adds a new vector drawable used as the secondary FAB icon. |
| app/src/main/java/com/wirelessalien/android/moviedb/pagingSource/ShowPagingSource.kt | Appends adult-content parameter to discover API calls. |
| app/src/main/java/com/wirelessalien/android/moviedb/pagingSource/SearchPersonPagingSource.kt | Appends adult-content parameter to person search calls. |
| app/src/main/java/com/wirelessalien/android/moviedb/pagingSource/SearchPagingSource.kt | Appends adult-content parameter to media search calls. |
| app/src/main/java/com/wirelessalien/android/moviedb/pagingSource/PersonPagingSource.kt | Appends adult-content parameter to popular people calls. |
| app/src/main/java/com/wirelessalien/android/moviedb/pagingSource/MultiSearchPagingSource.kt | Appends adult-content parameter to multi-search calls. |
| app/src/main/java/com/wirelessalien/android/moviedb/pagingSource/KeywordSearchPagingSource.kt | Appends adult-content parameter to keyword-discover calls. |
| app/src/main/java/com/wirelessalien/android/moviedb/fragment/ListFragment.kt | Changes FAB2 to open tagged lists; routes menu “stats” to watch summary dialog. |
| app/src/main/java/com/wirelessalien/android/moviedb/fragment/HomeFragment.kt | Appends adult-content parameter to several Home feed URLs. |
| app/src/main/java/com/wirelessalien/android/moviedb/adapter/SectionsPagerAdapter.kt | Adds the new hide-home preference key constant. |
| app/src/main/java/com/wirelessalien/android/moviedb/activity/MainActivity.kt | Applies Home tab visibility preference and selects an initial fragment based on visible tabs. |
| app/src/main/java/com/wirelessalien/android/moviedb/activity/BaseActivity.kt | Adds getAdultContentParameter() helper reading the new preference. |
Review details
- Files reviewed: 15/15 changed files
- Comments generated: 6
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
| val menu = binding.bottomNavigation.menu | ||
| menu.findItem(R.id.nav_home).isVisible = | ||
| !preferences.getBoolean(HIDE_HOME_PREFERENCE, false) | ||
| menu.findItem(R.id.nav_movie).isVisible = | ||
| !preferences.getBoolean(HIDE_MOVIES_PREFERENCE, false) |
| val url = URL("https://api.themoviedb.org/3/discover/$mediaType?with_keywords=${keywordId}&page=${page}" + | ||
| BaseActivity.getLanguageParameter2(context)) | ||
| BaseActivity.getLanguageParameter2(context) + BaseActivity.getAdultContentParameter(context)) |
| object SectionsPagerAdapter { | ||
| const val HIDE_HOME_PREFERENCE = "key_hide_home_tab" | ||
| const val HIDE_MOVIES_PREFERENCE = "key_hide_movies_tab" | ||
| const val HIDE_SERIES_PREFERENCE = "key_hide_series_tab" | ||
| const val HIDE_SAVED_PREFERENCE = "key_hide_saved_tab" |
| val url = URL("https://api.themoviedb.org/3/search/multi?query=${query}&page=${page}" + | ||
| BaseActivity.getLanguageParameter2(context)) | ||
| BaseActivity.getLanguageParameter2(context) + BaseActivity.getAdultContentParameter(context)) |
There was a problem hiding this comment.
🟡 Not ready to approve
The new tab-visibility behavior can hide the entire bottom navigation (contradicting the new settings text and leaving no navigation UI) and the new adult-parameter helper can crash due to context!! on a nullable parameter.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Suppressed comments (3)
app/src/main/java/com/wirelessalien/android/moviedb/activity/MainActivity.kt:280
- When all bottom-nav items are hidden via preferences, this code hides the entire BottomNavigationView (
View.GONE). That contradicts the new settings text that claims the Home tab will be displayed when all tabs are hidden, and it also leaves the user with no navigation UI. Consider forcingnav_homevisible when no items are visible so the app always has at least one tab.
val anyItemVisible = menu.findItem(R.id.nav_home).isVisible ||
menu.findItem(R.id.nav_movie).isVisible ||
menu.findItem(R.id.nav_series).isVisible ||
menu.findItem(R.id.nav_saved).isVisible ||
menu.findItem(R.id.nav_account).isVisible ||
app/src/main/java/com/wirelessalien/android/moviedb/activity/MainActivity.kt:336
- The preference-change listener repeats the same pattern of hiding the BottomNavigationView when all items are invisible. If the intent is to always show at least one tab (per the settings summary), apply the same "force Home visible when none are" logic here as well so runtime changes don't strand the user without tabs.
val anyItemVisible = menu1.findItem(R.id.nav_home).isVisible ||
menu1.findItem(R.id.nav_movie).isVisible ||
menu1.findItem(R.id.nav_series).isVisible ||
menu1.findItem(R.id.nav_saved).isVisible ||
menu1.findItem(R.id.nav_account).isVisible ||
app/src/main/java/com/wirelessalien/android/moviedb/activity/BaseActivity.kt:160
getAdultContentParameteraccepts a nullableContext?but immediately force-unwraps it (context!!), which can crash if any caller ever passes null. Either make the parameter non-null everywhere, or (simpler here) handle null safely in this helper.
fun getAdultContentParameter(context: Context?): String {
val preferences = PreferenceManager.getDefaultSharedPreferences(context!!)
val includeAdult = preferences.getBoolean(INCLUDE_ADULT_PREFERENCE, false)
return if (includeAdult) {
"&include_adult=true"
- Files reviewed: 15/15 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
No description provided.