Merge pull request #4377 from Bnyro/master

fix: regressions introduced by new chapters layout

app/src/main/java/com/github/libretube/ui/adapters/ChaptersAdapter.kt

@@ -37,27 +37,18 @@ class ChaptersAdapter(
             chapterTitle.text = chapter.title
             timeStamp.text = DateUtils.formatElapsedTime(chapter.start)
 
-            val chapterEnd = chapters.getOrNull(position + 1)?.start ?: (exoPlayer.duration / 1000)
+            val chapterEnd = chapters.getOrNull(position + 1)?.start
+                ?: (exoPlayer.duration / 1000)
             val durationSpan = chapterEnd - chapter.start
             duration.text = root.context.getString(
                 R.string.duration_span,
                 DateUtils.formatElapsedTime(durationSpan)
             )
 
-            val color = when {
+            val color = if (selectedPosition == position) {
-                selectedPosition == position -> {
+                ThemeHelper.getThemeColor(root.context, android.R.attr.colorControlHighlight)
-                    ThemeHelper.getThemeColor(root.context, android.R.attr.colorControlHighlight)
+            } else {
-                }
+                Color.TRANSPARENT
-
-                chapter.drawable != null -> ColorUtils.setAlphaComponent(
-                    ThemeHelper.getThemeColor(
-                        root.context,
-                        android.R.attr.colorPrimary
-                    ),
-                    50
-                )
-
-                else -> Color.TRANSPARENT
             }
             root.setBackgroundColor(color)
 
@@ -70,6 +61,8 @@ class ChaptersAdapter(
     }
 
     fun updateSelectedPosition(newPosition: Int) {
+        if (selectedPosition == newPosition) return
+
         val oldPosition = selectedPosition
         selectedPosition = newPosition
         notifyItemChanged(oldPosition)

app/src/main/java/com/github/libretube/ui/fragments/AudioPlayerFragment.kt

@@ -180,7 +180,7 @@ class AudioPlayerFragment : Fragment(), AudioPlayerOptions {
             val player = playerService.player ?: return@setOnClickListener
 
             ChaptersBottomSheet(streams.chapters, player)
-                .show(requireActivity().supportFragmentManager)
+                .show(childFragmentManager)
         }
 
         binding.miniPlayerClose.setOnClickListener {

app/src/main/java/com/github/libretube/ui/fragments/PlayerFragment.kt

@@ -777,7 +777,7 @@ class PlayerFragment : Fragment(), OnlinePlayerOptions {
                 // enable the chapters dialog in the player
                 playerBinding.chapterLL.setOnClickListener {
                     ChaptersBottomSheet(chapters, exoPlayer)
-                        .show(requireActivity().supportFragmentManager)
+                        .show(childFragmentManager)
                 }
 
                 setCurrentChapterName()
@@ -1181,10 +1181,14 @@ class PlayerFragment : Fragment(), OnlinePlayerOptions {
 
     // set the name of the video chapter in the exoPlayerView
     private fun setCurrentChapterName(forceUpdate: Boolean = false, enqueueNew: Boolean = true) {
-        playerBinding.chapterLL.isVisible = chapters.isNotEmpty()
+        // return if fragment view got killed already to avoid crashes
+        if (_binding == null) return
 
-        // return if chapters are empty to avoid crashes
+        // only show the chapters layout if there are some chapters available
-        if (chapters.isEmpty() || _binding == null) return
+        playerBinding.chapterLL.isInvisible = chapters.isEmpty()
+
+        // the following logic to set the chapter title can be skipped if no chapters are available
+        if (chapters.isEmpty()) return
 
         // call the function again in 100ms
         if (enqueueNew) binding.player.postDelayed(this::setCurrentChapterName, 100)
@@ -1192,8 +1196,9 @@ class PlayerFragment : Fragment(), OnlinePlayerOptions {
         // if the user is scrubbing the time bar, don't update
         if (scrubbingTimeBar && !forceUpdate) return
 
-        val chapterIndex = PlayerHelper.getCurrentChapterIndex(exoPlayer, chapters) ?: return
+        val chapterName = PlayerHelper.getCurrentChapterIndex(exoPlayer, chapters)?.let {
-        val chapterName = chapters[chapterIndex].title.trim()
+            chapters[it].title.trim()
+        } ?: getString(R.string.no_chapter)
 
         // change the chapter name textView text to the chapterName
         if (chapterName != playerBinding.chapterName.text) {
@@ -1453,7 +1458,7 @@ class PlayerFragment : Fragment(), OnlinePlayerOptions {
 
         if (audioLanguagesAndRoleFlags.isEmpty()) {
             baseBottomSheet.setSimpleItems(
-                listOf(context.getString(R.string.unknown_or_no_audio)),
+                listOf(getString(R.string.unknown_or_no_audio)),
                 null
             )
         } else if (audioLanguagesAndRoleFlags.size == 1 &&
@@ -1467,7 +1472,7 @@ class PlayerFragment : Fragment(), OnlinePlayerOptions {
             // track which has no language or track type set in the video played
             // Consider it as the default audio track (or unknown)
             baseBottomSheet.setSimpleItems(
-                listOf(context.getString(R.string.default_or_unknown_audio_track)),
+                listOf(getString(R.string.default_or_unknown_audio_track)),
                 null
             )
         } else {

app/src/main/java/com/github/libretube/ui/sheets/ChaptersBottomSheet.kt

@@ -1,13 +1,11 @@
 package com.github.libretube.ui.sheets
 
-import android.graphics.Color
 import android.os.Bundle
 import android.os.Handler
 import android.os.Looper
 import android.view.LayoutInflater
 import android.view.View
 import android.view.ViewGroup
-import androidx.core.view.children
 import androidx.core.view.isVisible
 import androidx.media3.exoplayer.ExoPlayer
 import androidx.recyclerview.widget.LinearLayoutManager
@@ -15,49 +13,47 @@ import com.github.libretube.R
 import com.github.libretube.api.obj.ChapterSegment
 import com.github.libretube.databinding.BottomSheetBinding
 import com.github.libretube.helpers.PlayerHelper
-import com.github.libretube.helpers.ThemeHelper
 import com.github.libretube.ui.adapters.ChaptersAdapter
 
 class ChaptersBottomSheet(
     private val chapters: List<ChapterSegment>,
     private val exoPlayer: ExoPlayer
 ): ExpandedBottomSheet() {
-    private lateinit var binding: BottomSheetBinding
+    private var _binding: BottomSheetBinding? = null
+    private val binding get() = _binding!!
 
     override fun onCreateView(
         inflater: LayoutInflater,
         container: ViewGroup?,
         savedInstanceState: Bundle?
     ): View {
-        binding = BottomSheetBinding.inflate(layoutInflater)
+        _binding = BottomSheetBinding.inflate(layoutInflater)
         return binding.root
     }
 
     override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
         binding.optionsRecycler.layoutManager = LinearLayoutManager(context)
-        binding.optionsRecycler.adapter = ChaptersAdapter(chapters, exoPlayer)
+        val adapter = ChaptersAdapter(chapters, exoPlayer)
+        binding.optionsRecycler.adapter = adapter
 
         binding.bottomSheetTitle.text = context?.getString(R.string.chapters)
         binding.bottomSheetTitleLayout.isVisible = true
 
         val handler = Handler(Looper.getMainLooper())
-        val highlightColor =
-            ThemeHelper.getThemeColor(requireContext(), android.R.attr.colorControlHighlight)
 
-        val updatePosition = Runnable {
+        val updatePosition = object: Runnable {
-            // scroll to the current playing index in the chapter
+            override fun run() {
-            val currentPosition =
+                if (_binding == null) return
-                PlayerHelper.getCurrentChapterIndex(exoPlayer, chapters) ?: return@Runnable
+                handler.postDelayed(this, 200)
-            binding.optionsRecycler.smoothScrollToPosition(currentPosition)
+                val currentIndex = PlayerHelper.getCurrentChapterIndex(exoPlayer, chapters) ?: return
-
+                adapter.updateSelectedPosition(currentIndex)
-            val children = binding.optionsRecycler.children.toList()
+            }
-            // reset the background colors of all chapters
-            children.forEach { it.setBackgroundColor(Color.TRANSPARENT) }
-            // highlight the current chapter
-            children.getOrNull(currentPosition)?.setBackgroundColor(highlightColor)
         }
-
         updatePosition.run()
-        handler.postDelayed(updatePosition, 200)
+    }
+
+    override fun onDestroyView() {
+        super.onDestroyView()
+        _binding = null
     }
 }

app/src/main/res/values/strings.xml

@@ -460,6 +460,7 @@
     <string name="default_or_unknown_audio_track">default or unknown</string>
     <string name="unknown_or_no_audio">unknown or no audio</string>
     <string name="continue_watching">Continue watching</string>
+    <string name="no_chapter">No chapter</string>
 
     <!-- Notification channel strings -->
     <string name="download_channel_name">Download Service</string>