Skip to content

Commit 99d6238

Browse files
mauriciocollilitetex
mauriciocolli
authored and
litetex
committed
Fix download dialog selector layout and add some tests
1 parent 860d28e commit 99d6238

2 files changed

Lines changed: 229 additions & 7 deletions

File tree

‎app/src/androidTest/java/org/schabi/newpipe/util/StreamItemAdapterTest.kt‎

Lines changed: 187 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,187 @@
1+
package org.schabi.newpipe.util
2+
3+
import android.content.Context
4+
import android.util.SparseArray
5+
import android.view.View
6+
import android.view.View.GONE
7+
import android.view.View.INVISIBLE
8+
import android.view.View.VISIBLE
9+
import android.widget.Spinner
10+
import androidx.test.core.app.ApplicationProvider
11+
import androidx.test.ext.junit.runners.AndroidJUnit4
12+
import androidx.test.filters.MediumTest
13+
import androidx.test.internal.runner.junit4.statement.UiThreadStatement
14+
import org.hamcrest.MatcherAssert.assertThat
15+
import org.hamcrest.Matchers.`is`
16+
import org.junit.Before
17+
import org.junit.Test
18+
import org.junit.runner.RunWith
19+
import org.schabi.newpipe.R
20+
import org.schabi.newpipe.extractor.MediaFormat
21+
import org.schabi.newpipe.extractor.stream.AudioStream
22+
import org.schabi.newpipe.extractor.stream.Stream
23+
import org.schabi.newpipe.extractor.stream.SubtitlesStream
24+
import org.schabi.newpipe.extractor.stream.VideoStream
25+
26+
@MediumTest
27+
@RunWith(AndroidJUnit4::class)
28+
class StreamItemAdapterTest {
29+
private lateinit var context: Context
30+
private lateinit var spinner: Spinner
31+
32+
@Before
33+
fun setUp() {
34+
context = ApplicationProvider.getApplicationContext()
35+
UiThreadStatement.runOnUiThread {
36+
spinner = Spinner(context)
37+
}
38+
}
39+
40+
@Test
41+
fun videoStreams_noSecondaryStream() {
42+
val adapter = StreamItemAdapter<VideoStream, AudioStream>(
43+
context,
44+
getVideoStreams(true, true, true, true),
45+
null
46+
)
47+
48+
spinner.adapter = adapter
49+
assertIconVisibility(spinner, 0, VISIBLE, VISIBLE)
50+
assertIconVisibility(spinner, 1, VISIBLE, VISIBLE)
51+
assertIconVisibility(spinner, 2, VISIBLE, VISIBLE)
52+
assertIconVisibility(spinner, 3, VISIBLE, VISIBLE)
53+
}
54+
55+
@Test
56+
fun videoStreams_hasSecondaryStream() {
57+
val adapter = StreamItemAdapter(
58+
context,
59+
getVideoStreams(false, true, false, true),
60+
getAudioStreams(false, true, false, true)
61+
)
62+
63+
spinner.adapter = adapter
64+
assertIconVisibility(spinner, 0, GONE, GONE)
65+
assertIconVisibility(spinner, 1, GONE, GONE)
66+
assertIconVisibility(spinner, 2, GONE, GONE)
67+
assertIconVisibility(spinner, 3, GONE, GONE)
68+
}
69+
70+
@Test
71+
fun videoStreams_Mixed() {
72+
val adapter = StreamItemAdapter(
73+
context,
74+
getVideoStreams(true, true, true, true, true, false, true, true),
75+
getAudioStreams(false, true, false, false, false, true, true, true)
76+
)
77+
78+
spinner.adapter = adapter
79+
assertIconVisibility(spinner, 0, VISIBLE, VISIBLE)
80+
assertIconVisibility(spinner, 1, GONE, INVISIBLE)
81+
assertIconVisibility(spinner, 2, VISIBLE, VISIBLE)
82+
assertIconVisibility(spinner, 3, VISIBLE, VISIBLE)
83+
assertIconVisibility(spinner, 4, VISIBLE, VISIBLE)
84+
assertIconVisibility(spinner, 5, GONE, INVISIBLE)
85+
assertIconVisibility(spinner, 6, GONE, INVISIBLE)
86+
assertIconVisibility(spinner, 7, GONE, INVISIBLE)
87+
}
88+
89+
@Test
90+
fun subtitleStreams_noIcon() {
91+
val adapter = StreamItemAdapter<SubtitlesStream, Stream>(
92+
context,
93+
StreamItemAdapter.StreamSizeWrapper(
94+
(0 until 5).map {
95+
SubtitlesStream(MediaFormat.SRT, "pt-BR", "https://example.com", false)
96+
},
97+
context
98+
),
99+
null
100+
)
101+
spinner.adapter = adapter
102+
for (i in 0 until spinner.count) {
103+
assertIconVisibility(spinner, i, GONE, GONE)
104+
}
105+
}
106+
107+
@Test
108+
fun audioStreams_noIcon() {
109+
val adapter = StreamItemAdapter<AudioStream, Stream>(
110+
context,
111+
StreamItemAdapter.StreamSizeWrapper(
112+
(0 until 5).map { AudioStream("https://example.com/$it", MediaFormat.OPUS, 192) },
113+
context
114+
),
115+
null
116+
)
117+
spinner.adapter = adapter
118+
for (i in 0 until spinner.count) {
119+
assertIconVisibility(spinner, i, GONE, GONE)
120+
}
121+
}
122+
123+
/**
124+
* @return a list of video streams, in which their video only property mirrors the provided
125+
* [videoOnly] vararg.
126+
*/
127+
private fun getVideoStreams(vararg videoOnly: Boolean) =
128+
StreamItemAdapter.StreamSizeWrapper(
129+
videoOnly.map {
130+
VideoStream("https://example.com", MediaFormat.MPEG_4, "720p", it)
131+
},
132+
context
133+
)
134+
135+
/**
136+
* @return a list of audio streams, containing valid and null elements mirroring the provided
137+
* [shouldBeValid] vararg.
138+
*/
139+
private fun getAudioStreams(vararg shouldBeValid: Boolean) =
140+
getSecondaryStreamsFromList(
141+
shouldBeValid.map {
142+
if (it) AudioStream("https://example.com", MediaFormat.OPUS, 192)
143+
else null
144+
}
145+
)
146+
147+
/**
148+
* Checks whether the item at [position] in the [spinner] has the correct icon visibility when
149+
* it is shown in normal mode (selected) and in dropdown mode (user is choosing one of a list).
150+
*/
151+
private fun assertIconVisibility(
152+
spinner: Spinner,
153+
position: Int,
154+
normalVisibility: Int,
155+
dropDownVisibility: Int
156+
) {
157+
spinner.setSelection(position)
158+
spinner.adapter.getView(position, null, spinner).run {
159+
assertThat(
160+
"normal visibility (pos=[$position])",
161+
findViewById<View>(R.id.wo_sound_icon).visibility, `is`(normalVisibility)
162+
)
163+
}
164+
spinner.adapter.getDropDownView(position, null, spinner).run {
165+
assertThat(
166+
"drop down visibility (pos=[$position])",
167+
findViewById<View>(R.id.wo_sound_icon).visibility, `is`(dropDownVisibility)
168+
)
169+
}
170+
}
171+
172+
/**
173+
* Helper function that builds a secondary stream list.
174+
*/
175+
private fun <T : Stream> getSecondaryStreamsFromList(streams: List<T?>) =
176+
SparseArray<SecondaryStreamHelper<T>?>(streams.size).apply {
177+
streams.forEachIndexed { index, stream ->
178+
val secondaryStreamHelper: SecondaryStreamHelper<T>? = stream?.let {
179+
SecondaryStreamHelper(
180+
StreamItemAdapter.StreamSizeWrapper(streams, context),
181+
it
182+
)
183+
}
184+
put(index, secondaryStreamHelper)
185+
}
186+
}
187+
}

‎app/src/main/java/org/schabi/newpipe/util/StreamItemAdapter.java‎

Lines changed: 42 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -42,16 +42,20 @@ public class StreamItemAdapter<T extends Stream, U extends Stream> extends BaseA
4242
private final StreamSizeWrapper<T> streamsWrapper;
4343
private final SparseArray<SecondaryStreamHelper<U>> secondaryStreams;
4444

45+
/**
46+
* Indicates that at least one of the primary streams is an instance of {@link VideoStream},
47+
* has no audio ({@link VideoStream#isVideoOnly()} returns true) and has no secondary stream
48+
* associated with it.
49+
*/
50+
private final boolean hasVideoOnlyWithNoSecondaryStream;
51+
4552
public StreamItemAdapter(final Context context, final StreamSizeWrapper<T> streamsWrapper,
4653
final SparseArray<SecondaryStreamHelper<U>> secondaryStreams) {
4754
this.context = context;
4855
this.streamsWrapper = streamsWrapper;
4956
this.secondaryStreams = secondaryStreams;
50-
}
5157

52-
public StreamItemAdapter(final Context context, final StreamSizeWrapper<T> streamsWrapper,
53-
final boolean showIconNoAudio) {
54-
this(context, streamsWrapper, showIconNoAudio ? new SparseArray<>() : null);
58+
this.hasVideoOnlyWithNoSecondaryStream = checkHasVideoOnlyWithNoSecondaryStream();
5559
}
5660

5761
public StreamItemAdapter(final Context context, final StreamSizeWrapper<T> streamsWrapper) {
@@ -115,10 +119,15 @@ private View getCustomView(final int position, final View view, final ViewGroup
115119
final VideoStream videoStream = ((VideoStream) stream);
116120
qualityString = videoStream.getResolution();
117121

118-
if (secondaryStreams != null) {
122+
if (hasVideoOnlyWithNoSecondaryStream) {
119123
if (videoStream.isVideoOnly()) {
120-
woSoundIconVisibility = secondaryStreams.get(position) == null ? View.VISIBLE
121-
: View.INVISIBLE;
124+
woSoundIconVisibility = hasSecondaryStream(position)
125+
// It has a secondary stream associated with it, so check if it's a
126+
// dropdown view so it doesn't look out of place (missing margin)
127+
// compared to those that don't.
128+
? (isDropdownItem ? View.INVISIBLE : View.GONE)
129+
// It doesn't have a secondary stream, icon is visible no matter what.
130+
: View.VISIBLE;
122131
} else if (isDropdownItem) {
123132
woSoundIconVisibility = View.INVISIBLE;
124133
}
@@ -167,6 +176,32 @@ private View getCustomView(final int position, final View view, final ViewGroup
167176
return convertView;
168177
}
169178

179+
/**
180+
* @param position which primary stream to check.
181+
* @return whether the primary stream at position has a secondary stream associated with it.
182+
*/
183+
private boolean hasSecondaryStream(final int position) {
184+
return secondaryStreams != null && secondaryStreams.get(position) != null;
185+
}
186+
187+
/**
188+
* @return if there are any video-only streams with no secondary stream associated with them.
189+
* @see #hasVideoOnlyWithNoSecondaryStream
190+
*/
191+
private boolean checkHasVideoOnlyWithNoSecondaryStream() {
192+
for (int i = 0; i < streamsWrapper.getStreamsList().size(); i++) {
193+
final T stream = streamsWrapper.getStreamsList().get(i);
194+
if (stream instanceof VideoStream) {
195+
final boolean videoOnly = ((VideoStream) stream).isVideoOnly();
196+
if (videoOnly && !hasSecondaryStream(i)) {
197+
return true;
198+
}
199+
}
200+
}
201+
202+
return false;
203+
}
204+
170205
/**
171206
* A wrapper class that includes a way of storing the stream sizes.
172207
*

0 commit comments

Comments
 (0)