Display images in the summary view#7234
Conversation
grzesiek2010
force-pushed
the
images
branch
3 times, most recently
from
bcd10a8 to
bde4db2
Compare
seadowg
left a comment
seadowg
left a comment
There was a problem hiding this comment.
Some initial thoughts before going deeper.
|
|
||
| override fun clearAnswer() { | ||
| fileAnswerDelegate.deleteFile() | ||
| widgetValueChanged() |
There was a problem hiding this comment.
Could the delegate also take the current widget as a constructor param so it can handle the widgetValueChanged calls as well?
| modifier: Modifier, | ||
| answer: String, | ||
| contentScale: ContentScale, | ||
| mediaWidgetAnswerViewModel: MediaWidgetAnswerViewModel, |
There was a problem hiding this comment.
Passing the ViewModel here seems inconsistent with the other "widget answer" Composables: they mostly take data and listeners (like onClick) and handle wiring up the ViewModel in WidgetAnswer.
| } | ||
| } | ||
| Constants.CONTROL_IMAGE_CHOOSE -> ImageWidgetAnswer( | ||
| if (summaryView) { |
There was a problem hiding this comment.
Could this param be named to "centered" or something more oriented towards the visual difference?
|
|
||
| @Test | ||
| override fun usingReadOnlyOptionShouldMakeAllClickableElementsDisabled() { | ||
| whenever(formEntryPrompt.isReadOnly).thenReturn(true) |
There was a problem hiding this comment.
This could use MockFormEntryPromptBuilder instead
| @Test | ||
| fun `when read-only override option is used should all clickable elements be disabled`() { | ||
| readOnlyOverride = true | ||
| whenever(formEntryPrompt.isReadOnly).thenReturn(false) |
There was a problem hiding this comment.
This could use MockFormEntryPromptBuilder instead
| } | ||
|
|
||
| @Test | ||
| fun `when prompt has current answer shows in image view`() { |
2 participants
Work towards #6084
Closes #7168
Why is this the best possible solution? Were any other approaches considered?
The new
ImageWidgetAnswercomponent continues the approach we already introduced forvideoandfilequestions. Similar to those question types, I started refactoring the widgets so they can share the same answer component with the summary view.To avoid making this PR too large, I started with the basic image question. I decided not only to rework it to use the new
ImageWidgetAnswer, but also to shift frominheritancetocompositionby detaching it from its previous hierarchy. It previously extendedBaseImageWidget, which had become a fairly convoluted structure.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?
Images should be displayed in the summary view regardless of which image question type they come from (
image/draw/signature/annotate). The summary view should be tested with all image question types, but we don’t need to test each image question type in form entry. Only the basic image question has been refactored, so that is the one that requires thorough testing. Only the basic image question currently shares the new image component with the summary view, so only that question type will display images with rounded corners. We plan to apply the same approach to the other image question types in the future.Do we need any specific form for testing your changes? If so, please attach one.
The
All question typesforms should be enough.Does this change require updates to documentation? If so, please file an issue here and include the link below.
No.
Before submitting this PR, please make sure you have:
./gradlew connectedAndroidTest(or./gradlew testLab) and confirmed all checks still passDateFormatsTest