Remove AudioHelper and ScreenContext
#6609
Conversation
… constructor args
|
|
||
| public void setAudioHelper(AudioHelper audioHelper) { | ||
| this.audioHelper = audioHelper; | ||
| public void setAudioPlayer(AudioPlayer audioPlayer) { |
There was a problem hiding this comment.
I'd like to clean up this need to have a way of dynamically setting an AudioPlayer on the select adapter, but it would mean restructuring the select minimal dialog implementation which would be yet another rabbit hole to go down. That'll be saved for #6279.
| public QuestionWidget(Context context, QuestionDetails questionDetails) { | ||
| protected AudioPlayer audioPlayer; | ||
|
|
||
| public QuestionWidget(Context context, Dependencies dependencies, QuestionDetails questionDetails) { |
There was a problem hiding this comment.
I think we'll hopefully move towards a world where changing this constructor isn't so awkward (#6534 is already pushing in the direction). For now, I felt like we should introduce an easier way to do it that doesn't rely on Dagger (which only works for application level dependencies) rather just adding AudioPlayer to every widget's contructor.
seadowg
changed the title
AudioHelper and ScreenContext
collect_app/src/main/java/org/odk/collect/android/formentry/media/PromptAutoplayer.java
Outdated
Show resolved
Hide resolved
|
|
||
| private val currentlyPlaying = MutableLiveData<CurrentlyPlaying?>(null) | ||
| private val error = MutableLiveData<Exception?>() | ||
| private val error = MutableLiveData<Consumable<Exception>?>() |
There was a problem hiding this comment.
It doesn't need to be nullable now as it uses Consumable right? We used null to make the errors one-time occurrences.
There was a problem hiding this comment.
Ah yeah I guess we never set the error back to null now. It feels like the error should still be null once there's a successful load clip load though. I'll tweak that.
|
Tested with Success! Tested on device with Android: 10,12 Verified cases:
|
|
Tested with Success! Tested on device with Android 15 |
|
Tested with Success! Tested on device with Android: 10 |
5 participants
Work towards #5420
ScreenContextwas making it hard to convert form entry to aFragmentas using it makes assumptions about how the views (widgets) sit within an Activity. To get rid of it, I also needed to remove the deprecatedAudioHelper.Why is this the best possible solution? Were any other approaches considered?
Comments inline.
How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?
The main area affected here is forms that use audio. By this I mean in question labels, selects or the audio widget itself (although recording hasn't been changed at all).
Before submitting this PR, please make sure you have:
./gradlew connectedAndroidTest(or./gradlew testLab) and confirmed all checks still passDateFormatsTest