diff --git a/app/src/androidTest/java/org/schabi/newpipe/util/StreamItemAdapterTest.kt b/app/src/androidTest/java/org/schabi/newpipe/util/StreamItemAdapterTest.kt new file mode 100644 index 000000000..0b01bfaf6 --- /dev/null +++ b/app/src/androidTest/java/org/schabi/newpipe/util/StreamItemAdapterTest.kt @@ -0,0 +1,187 @@ +package org.schabi.newpipe.util + +import android.content.Context +import android.util.SparseArray +import android.view.View +import android.view.View.GONE +import android.view.View.INVISIBLE +import android.view.View.VISIBLE +import android.widget.Spinner +import androidx.test.core.app.ApplicationProvider +import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.test.filters.MediumTest +import androidx.test.internal.runner.junit4.statement.UiThreadStatement +import org.hamcrest.MatcherAssert.assertThat +import org.hamcrest.Matchers.`is` +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.schabi.newpipe.R +import org.schabi.newpipe.extractor.MediaFormat +import org.schabi.newpipe.extractor.stream.AudioStream +import org.schabi.newpipe.extractor.stream.Stream +import org.schabi.newpipe.extractor.stream.SubtitlesStream +import org.schabi.newpipe.extractor.stream.VideoStream + +@MediumTest +@RunWith(AndroidJUnit4::class) +class StreamItemAdapterTest { + private lateinit var context: Context + private lateinit var spinner: Spinner + + @Before + fun setUp() { + context = ApplicationProvider.getApplicationContext() + UiThreadStatement.runOnUiThread { + spinner = Spinner(context) + } + } + + @Test + fun videoStreams_noSecondaryStream() { + val adapter = StreamItemAdapter( + context, + getVideoStreams(true, true, true, true), + null + ) + + spinner.adapter = adapter + assertIconVisibility(spinner, 0, VISIBLE, VISIBLE) + assertIconVisibility(spinner, 1, VISIBLE, VISIBLE) + assertIconVisibility(spinner, 2, VISIBLE, VISIBLE) + assertIconVisibility(spinner, 3, VISIBLE, VISIBLE) + } + + @Test + fun videoStreams_hasSecondaryStream() { + val adapter = StreamItemAdapter( + context, + getVideoStreams(false, true, false, true), + getAudioStreams(false, true, false, true) + ) + + spinner.adapter = adapter + assertIconVisibility(spinner, 0, GONE, GONE) + assertIconVisibility(spinner, 1, GONE, GONE) + assertIconVisibility(spinner, 2, GONE, GONE) + assertIconVisibility(spinner, 3, GONE, GONE) + } + + @Test + fun videoStreams_Mixed() { + val adapter = StreamItemAdapter( + context, + getVideoStreams(true, true, true, true, true, false, true, true), + getAudioStreams(false, true, false, false, false, true, true, true) + ) + + spinner.adapter = adapter + assertIconVisibility(spinner, 0, VISIBLE, VISIBLE) + assertIconVisibility(spinner, 1, GONE, INVISIBLE) + assertIconVisibility(spinner, 2, VISIBLE, VISIBLE) + assertIconVisibility(spinner, 3, VISIBLE, VISIBLE) + assertIconVisibility(spinner, 4, VISIBLE, VISIBLE) + assertIconVisibility(spinner, 5, GONE, INVISIBLE) + assertIconVisibility(spinner, 6, GONE, INVISIBLE) + assertIconVisibility(spinner, 7, GONE, INVISIBLE) + } + + @Test + fun subtitleStreams_noIcon() { + val adapter = StreamItemAdapter( + context, + StreamItemAdapter.StreamSizeWrapper( + (0 until 5).map { + SubtitlesStream(MediaFormat.SRT, "pt-BR", "https://example.com", false) + }, + context + ), + null + ) + spinner.adapter = adapter + for (i in 0 until spinner.count) { + assertIconVisibility(spinner, i, GONE, GONE) + } + } + + @Test + fun audioStreams_noIcon() { + val adapter = StreamItemAdapter( + context, + StreamItemAdapter.StreamSizeWrapper( + (0 until 5).map { AudioStream("https://example.com/$it", MediaFormat.OPUS, 192) }, + context + ), + null + ) + spinner.adapter = adapter + for (i in 0 until spinner.count) { + assertIconVisibility(spinner, i, GONE, GONE) + } + } + + /** + * @return a list of video streams, in which their video only property mirrors the provided + * [videoOnly] vararg. + */ + private fun getVideoStreams(vararg videoOnly: Boolean) = + StreamItemAdapter.StreamSizeWrapper( + videoOnly.map { + VideoStream("https://example.com", MediaFormat.MPEG_4, "720p", it) + }, + context + ) + + /** + * @return a list of audio streams, containing valid and null elements mirroring the provided + * [shouldBeValid] vararg. + */ + private fun getAudioStreams(vararg shouldBeValid: Boolean) = + getSecondaryStreamsFromList( + shouldBeValid.map { + if (it) AudioStream("https://example.com", MediaFormat.OPUS, 192) + else null + } + ) + + /** + * Checks whether the item at [position] in the [spinner] has the correct icon visibility when + * it is shown in normal mode (selected) and in dropdown mode (user is choosing one of a list). + */ + private fun assertIconVisibility( + spinner: Spinner, + position: Int, + normalVisibility: Int, + dropDownVisibility: Int + ) { + spinner.setSelection(position) + spinner.adapter.getView(position, null, spinner).run { + assertThat( + "normal visibility (pos=[$position])", + findViewById(R.id.wo_sound_icon).visibility, `is`(normalVisibility) + ) + } + spinner.adapter.getDropDownView(position, null, spinner).run { + assertThat( + "drop down visibility (pos=[$position])", + findViewById(R.id.wo_sound_icon).visibility, `is`(dropDownVisibility) + ) + } + } + + /** + * Helper function that builds a secondary stream list. + */ + private fun getSecondaryStreamsFromList(streams: List) = + SparseArray?>(streams.size).apply { + streams.forEachIndexed { index, stream -> + val secondaryStreamHelper: SecondaryStreamHelper? = stream?.let { + SecondaryStreamHelper( + StreamItemAdapter.StreamSizeWrapper(streams, context), + it + ) + } + put(index, secondaryStreamHelper) + } + } +} diff --git a/app/src/main/java/org/schabi/newpipe/util/StreamItemAdapter.java b/app/src/main/java/org/schabi/newpipe/util/StreamItemAdapter.java index c89da9d45..b66080d1b 100644 --- a/app/src/main/java/org/schabi/newpipe/util/StreamItemAdapter.java +++ b/app/src/main/java/org/schabi/newpipe/util/StreamItemAdapter.java @@ -42,16 +42,20 @@ public class StreamItemAdapter extends BaseA private final StreamSizeWrapper streamsWrapper; private final SparseArray> secondaryStreams; + /** + * Indicates that at least one of the primary streams is an instance of {@link VideoStream}, + * has no audio ({@link VideoStream#isVideoOnly()} returns true) and has no secondary stream + * associated with it. + */ + private final boolean hasVideoOnlyWithNoSecondaryStream; + public StreamItemAdapter(final Context context, final StreamSizeWrapper streamsWrapper, final SparseArray> secondaryStreams) { this.context = context; this.streamsWrapper = streamsWrapper; this.secondaryStreams = secondaryStreams; - } - public StreamItemAdapter(final Context context, final StreamSizeWrapper streamsWrapper, - final boolean showIconNoAudio) { - this(context, streamsWrapper, showIconNoAudio ? new SparseArray<>() : null); + this.hasVideoOnlyWithNoSecondaryStream = checkHasVideoOnlyWithNoSecondaryStream(); } public StreamItemAdapter(final Context context, final StreamSizeWrapper streamsWrapper) { @@ -115,10 +119,15 @@ public class StreamItemAdapter extends BaseA final VideoStream videoStream = ((VideoStream) stream); qualityString = videoStream.getResolution(); - if (secondaryStreams != null) { + if (hasVideoOnlyWithNoSecondaryStream) { if (videoStream.isVideoOnly()) { - woSoundIconVisibility = secondaryStreams.get(position) == null ? View.VISIBLE - : View.INVISIBLE; + woSoundIconVisibility = hasSecondaryStream(position) + // It has a secondary stream associated with it, so check if it's a + // dropdown view so it doesn't look out of place (missing margin) + // compared to those that don't. + ? (isDropdownItem ? View.INVISIBLE : View.GONE) + // It doesn't have a secondary stream, icon is visible no matter what. + : View.VISIBLE; } else if (isDropdownItem) { woSoundIconVisibility = View.INVISIBLE; } @@ -167,6 +176,32 @@ public class StreamItemAdapter extends BaseA return convertView; } + /** + * @param position which primary stream to check. + * @return whether the primary stream at position has a secondary stream associated with it. + */ + private boolean hasSecondaryStream(final int position) { + return secondaryStreams != null && secondaryStreams.get(position) != null; + } + + /** + * @return if there are any video-only streams with no secondary stream associated with them. + * @see #hasVideoOnlyWithNoSecondaryStream + */ + private boolean checkHasVideoOnlyWithNoSecondaryStream() { + for (int i = 0; i < streamsWrapper.getStreamsList().size(); i++) { + final T stream = streamsWrapper.getStreamsList().get(i); + if (stream instanceof VideoStream) { + final boolean videoOnly = ((VideoStream) stream).isVideoOnly(); + if (videoOnly && !hasSecondaryStream(i)) { + return true; + } + } + } + + return false; + } + /** * A wrapper class that includes a way of storing the stream sizes. *