Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix download dialog selector layout #7516

Merged
merged 3 commits into from
Apr 2, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,188 @@
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.junit.Assert
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<VideoStream, AudioStream>(
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<SubtitlesStream, Stream>(
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<AudioStream, Stream>(
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 {
Assert.assertEquals(
"normal visibility (pos=[$position]) is not correct",
findViewById<View>(R.id.wo_sound_icon).visibility,
normalVisibility,
)
}
spinner.adapter.getDropDownView(position, null, spinner).run {
Assert.assertEquals(
"drop down visibility (pos=[$position]) is not correct",
findViewById<View>(R.id.wo_sound_icon).visibility,
dropDownVisibility
)
}
}

/**
* Helper function that builds a secondary stream list.
*/
private fun <T : Stream> getSecondaryStreamsFromList(streams: List<T?>) =
SparseArray<SecondaryStreamHelper<T>?>(streams.size).apply {
streams.forEachIndexed { index, stream ->
val secondaryStreamHelper: SecondaryStreamHelper<T>? = stream?.let {
SecondaryStreamHelper(
StreamItemAdapter.StreamSizeWrapper(streams, context),
it
)
}
put(index, secondaryStreamHelper)
}
}
}
50 changes: 43 additions & 7 deletions app/src/main/java/org/schabi/newpipe/util/StreamItemAdapter.java
Original file line number Diff line number Diff line change
Expand Up @@ -42,16 +42,21 @@ public class StreamItemAdapter<T extends Stream, U extends Stream> extends BaseA
private final StreamSizeWrapper<T> streamsWrapper;
private final SparseArray<SecondaryStreamHelper<U>> 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 hasAnyVideoOnlyStreamWithNoSecondaryStream;

public StreamItemAdapter(final Context context, final StreamSizeWrapper<T> streamsWrapper,
final SparseArray<SecondaryStreamHelper<U>> secondaryStreams) {
this.context = context;
this.streamsWrapper = streamsWrapper;
this.secondaryStreams = secondaryStreams;
}

public StreamItemAdapter(final Context context, final StreamSizeWrapper<T> streamsWrapper,
final boolean showIconNoAudio) {
this(context, streamsWrapper, showIconNoAudio ? new SparseArray<>() : null);
this.hasAnyVideoOnlyStreamWithNoSecondaryStream =
checkHasAnyVideoOnlyStreamWithNoSecondaryStream();
}

public StreamItemAdapter(final Context context, final StreamSizeWrapper<T> streamsWrapper) {
Expand Down Expand Up @@ -115,10 +120,15 @@ private View getCustomView(final int position, final View view, final ViewGroup
final VideoStream videoStream = ((VideoStream) stream);
qualityString = videoStream.getResolution();

if (secondaryStreams != null) {
if (hasAnyVideoOnlyStreamWithNoSecondaryStream) {
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;
}
Expand Down Expand Up @@ -167,6 +177,32 @@ private View getCustomView(final int position, final View view, final ViewGroup
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 #hasAnyVideoOnlyStreamWithNoSecondaryStream
*/
private boolean checkHasAnyVideoOnlyStreamWithNoSecondaryStream() {
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.
*
Expand Down