Improve media displayed in widgets [BarcodeWidget]

[grzesiek2010]

Closes #6234

Why is this the best possible solution? Were any other approaches considered?

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?

Do we need any specific form for testing your changes? If so, please attach one.

Does this change require updates to documentation? If so, please file an issue here and include the link below.

Before submitting this PR, please make sure you have:

• added or modified tests for any new or changed behavior
• run ./gradlew connectedAndroidTest (or ./gradlew testLab) and confirmed all checks still pass
• added a comment above any new strings describing it for translators
• added any new strings with date formatting to DateFormatsTest
• verified that any code or assets from external sources are properly credited in comments and/or in the about file.
• verified that any new UI elements use theme colors. UI Components Style guidelines

[grzesiek2010]

@seadowg
please take a quick look to see if the general approach looks good to you.

[seadowg]

I definitely like the general direction of pulling out a view to specifically represent the answer here. I had in mind that we could go a bit further and have a generalized AnswerView with a similar structure that where setAnswer takes an IAnswerData (I think the widget would handle the hidden appearance in that setup). That would get us something that's simple to then use in the hierarchy later.

[grzesiek2010]

@seadowg could you clarify your feedback because I'm not sure if I understand?
Do you want to have one AnswerView for all types of questions? or maybe AnswerView should be an interface and implementations would be created based on data type as we do for the whole widgets in WidgetFactory?

[seadowg]

Do you want to have one AnswerView for all types of questions? or maybe AnswerView should be an interface and implementations would be created based on data type as we do for the whole widgets in WidgetFactory?

I was thinking one AnswerView that can handle any IAnswerData, but I'd imagine that'd be implemented using multiple different child views. My thinking is that we currently make the decision about how to display an answer in two places (in each widget and in the hierarchy list items) and we could move that to one place.

[grzesiek2010]

I was thinking one AnswerView that can handle any IAnswerData, but I'd imagine that'd be implemented using multiple different child views.

That makes sense.

My thinking is that we currently make the decision about how to display an answer in two places (in each widget and in the hierarchy list items) and we could move that to one place.

The view I factored out will be one that can be shared between the question layout and the hierarchy layout.

As I rework other questions, I’ll likely introduce some form of abstraction. Later, when we work on the hierarchy view, we'll need to implement a factory to construct the appropriate view but that can wait for now.

Let's maybe continue with other question types in this pull request to see how it goes, unless you have something against the current changes.

[seadowg]

Let's maybe continue with other question types in this pull request to see how it goes, unless you have something against the current changes.

That sounds good! You're right that it makes more sense to introduce the generalization after 2 or 3 examples rather than trying to plan it ahead of time.

[seadowg]

@grzesiek2010 I've been running into frustrations trying to add an arg to the QuestionWidget constructor, and it led me to thinking a bit more about the eventual structure we want.

Basically I think we need to get to a point where we can modify QuestionWidget (the thing responsible for the shared view logic between every widget) without needing to then modify all the subclasses. I reckon we'll need to deploy some "composition over inheritance" to achieve this. To go with the example we already have here I reckon instead of a BearingWidget we really just want to have a BearingWidgetAnswer and then in WidgetFactory be constructing our bearing question like this (or something similar):

QuestionWidget(context, questionDetails, BearingWidgetAnswer())

That would let us easily modify QuestionWidget's constructor in the future (and remove things like Dagger). I think getting to that sort of place for at least one widget here would be great! Sadly I don't think that kind of restructure is a good idea in one go.

[grzesiek2010]

@seadowg
As discussed earlier, I've reworked another question type (actually four, since there are multiple image questions) to see how it goes. I believe we can have a single interface implemented for all widget answer classes, like BarcodeWidgetAnswer, ImageWidgetAnswer, and so on. However, different implementations require different dependencies. For example, ImageWidgetAnswer needs imageLoader, questionMediaManager, etc. Given that, I think we'll need a factory similar to the one we use for entire widgets.

QuestionWidget(context, questionDetails, BearingWidgetAnswer())

The current implementation adds the answers using XML, but this won’t be possible later in the summary view, where we’ll need to generate answers dynamically. So, it would probably make more sense to add that view programmatically in the widgets as well. Is that what you meant? Any other thoughts?

[seadowg]

The current implementation adds the answers using XML, but this won’t be possible later in the summary view, where we’ll need to generate answers dynamically. So, it would probably make more sense to add that view programmatically in the widgets as well. Is that what you meant? Any other thoughts?

Right it feels like we need to move to the views being added programmatically for this to work. So ideally QuestionWidget has a new constructor arg for the widget answer view (like we've been describing) that is a View and also implements our new interface (WidgetAnswer for example). In Kotlin, we could do this:

class QuestionWidget<A>(private val widgetAnswerView: A) where A : View, A : WidgetAnswer {

I think in Java we just need to use an abstract to combine the types:

public abstract class WidgetAnswerView extends View implements WidgetAnswer {
}

Then we'd be able to add that view into the layout for the QuestionWidget programmatically and interact with it using the interface. Given it's going to be hard to convert everything at once, we probably want to deprecate, but continue to support QuestionWidget#onCreateAnswerView for a while longer.