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.

Sign up for GitHub

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

Reworked the way we display images in ImageWidgets #5433

Merged
merged 5 commits into from
Mar 9, 2023
Merged

Reworked the way we display images in ImageWidgets #5433

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d803e64
Reworked the way we display images in ImageWidgets
grzesiek2010 Feb 3, 2023
4c3aba3
Updated image widget tests
grzesiek2010 Feb 3, 2023
12c186d
Added tests to verify that the image view and the eroro view are not …
grzesiek2010 Feb 3, 2023
4d985d0
Added tests to verify that the error message is displayed when the an…
grzesiek2010 Feb 3, 2023
7e06f6d
Fixed IntentGroupTest
grzesiek2010 Feb 3, 2023
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
7 changes: 5 additions & 2 deletions ...t_app/src/androidTest/java/org/odk/collect/android/feature/formentry/IntentGroupTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,11 @@
import static androidx.test.espresso.matcher.ViewMatchers.isDisplayed;
import static androidx.test.espresso.matcher.ViewMatchers.isEnabled;
import static androidx.test.espresso.matcher.ViewMatchers.withClassName;
import static androidx.test.espresso.matcher.ViewMatchers.withEffectiveVisibility;
import static androidx.test.espresso.matcher.ViewMatchers.withId;
import static androidx.test.espresso.matcher.ViewMatchers.withTagValue;
import static androidx.test.espresso.matcher.ViewMatchers.withText;
import static org.hamcrest.CoreMatchers.allOf;
import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.core.StringEndsWith.endsWith;
Expand All @@ -47,6 +49,7 @@

import androidx.core.content.FileProvider;
import androidx.test.core.app.ApplicationProvider;
import androidx.test.espresso.matcher.ViewMatchers;

import org.junit.Rule;
import org.junit.Test;
Expand Down Expand Up @@ -204,7 +207,7 @@ public void collect_shouldNotCrashWhenAnyErrorIsThrownWhileReceivingAnswer() {
}

private void assertImageWidgetWithoutAnswer() {
onView(withTagValue(is("ImageView"))).check(doesNotExist());
onView(allOf(withTagValue(is("ImageView")), withEffectiveVisibility(ViewMatchers.Visibility.VISIBLE))).check(doesNotExist());
onView(withId(R.id.capture_image)).check(doesNotExist());
onView(withId(R.id.choose_image)).check(doesNotExist());
}
Expand All @@ -223,7 +226,7 @@ private void assertFileWidgetWithoutAnswer() {
}

private void assertImageWidgetWithAnswer() {
onView(withTagValue(is("ImageView"))).perform(scrollTo()).check(matches(isDisplayed()));
onView(allOf(withTagValue(is("ImageView")), withEffectiveVisibility(ViewMatchers.Visibility.VISIBLE))).check(matches(not(doesNotExist())));
onView(withId(R.id.capture_image)).check(doesNotExist());
onView(withId(R.id.choose_image)).check(doesNotExist());
}
Expand Down
9 changes: 4 additions & 5 deletions collect_app/src/main/java/org/odk/collect/android/widgets/AnnotateWidget.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ public AnnotateWidget(Context context, QuestionDetails prompt, QuestionMediaMana
imageClickHandler = new DrawImageClickHandler(DrawActivity.OPTION_ANNOTATE, RequestCodes.ANNOTATE_IMAGE, R.string.annotate_image);
imageCaptureHandler = new ImageCaptureHandler();
setUpLayout();
addCurrentImageToLayout();
updateAnswer();
adjustAnnotateButtonAvailability();
addAnswerView(answerLayout, WidgetViewUtils.getStandardMargin(context));
}
Expand All @@ -87,9 +87,9 @@ protected void setUpLayout() {
answerLayout.addView(chooseButton);
answerLayout.addView(annotateButton);
answerLayout.addView(errorTextView);
answerLayout.addView(imageView);

hideButtonsIfNeeded();
errorTextView.setVisibility(View.GONE);
}

@Override
Expand Down Expand Up @@ -141,7 +141,7 @@ public void onButtonClick(int buttonId) {
}

private void adjustAnnotateButtonAvailability() {
if (binaryName == null || imageView == null || imageView.getVisibility() == GONE) {
if (binaryName == null || imageView.getVisibility() == GONE) {
annotateButton.setEnabled(false);
}
}
Expand All @@ -155,7 +155,7 @@ && getFormEntryPrompt().getAppearanceHint().toLowerCase(Locale.ENGLISH).contains

private int calculateScreenOrientation() {
Bitmap bmp = null;
if (imageView != null) {
if (imageView.getDrawable() != null) {
bmp = ((BitmapDrawable) imageView.getDrawable()).getBitmap();
}

Expand All @@ -164,7 +164,6 @@ private int calculateScreenOrientation() {
}

private void captureImage() {
errorTextView.setVisibility(View.GONE);
Intent intent = new Intent(android.provider.MediaStore.ACTION_IMAGE_CAPTURE);
// We give the camera an absolute filename/path where to put the
// picture because of bug:
Expand Down
39 changes: 19 additions & 20 deletions collect_app/src/main/java/org/odk/collect/android/widgets/BaseImageWidget.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@

public abstract class BaseImageWidget extends QuestionWidget implements FileWidget, WidgetDataReceiver {

@Nullable
protected ImageView imageView;
protected String binaryName;
protected TextView errorTextView;
Expand Down Expand Up @@ -81,10 +80,8 @@ public IAnswerData getAnswer() {
@Override
public void clearAnswer() {
deleteFile();
if (imageView != null) {
imageView.setImageDrawable(null);
}

imageView.setImageDrawable(null);
imageView.setVisibility(View.GONE);
errorTextView.setVisibility(View.GONE);
widgetValueChanged();
}
Expand All @@ -106,7 +103,7 @@ public void setData(Object object) {
if (newImage.exists()) {
questionMediaManager.replaceAnswerFile(getFormEntryPrompt().getIndex().toString(), newImage.getAbsolutePath());
binaryName = newImage.getName();
addCurrentImageToLayout();
updateAnswer();
widgetValueChanged();
} else {
Timber.e(new Error("NO IMAGE EXISTS at: " + newImage.getAbsolutePath()));
Expand All @@ -131,29 +128,23 @@ public void cancelLongPress() {
}
}

protected void addCurrentImageToLayout() {
answerLayout.removeView(imageView);
protected void updateAnswer() {
imageView.setVisibility(View.GONE);
errorTextView.setVisibility(View.GONE);

if (binaryName != null) {
File f = getFile();
if (f != null && f.exists()) {
imageView = createAnswerImageView(getContext());
answerLayout.addView(imageView);
imageView.setVisibility(View.VISIBLE);
imageLoader.loadImage(imageView, f, ImageView.ScaleType.FIT_CENTER, new GlideImageLoader.ImageLoaderCallback() {
@Override
public void onLoadFailed() {
answerLayout.removeView(imageView);
imageView = null;
imageView.setVisibility(View.GONE);
errorTextView.setVisibility(View.VISIBLE);
}

@Override
public void onLoadSucceeded() {
imageView.setOnClickListener(v -> {
if (imageClickHandler != null) {
imageClickHandler.clickImage("viewImage");
}
});
}
});
}
Expand All @@ -169,6 +160,13 @@ protected void setUpLayout() {
answerLayout.setOrientation(LinearLayout.VERTICAL);

binaryName = getFormEntryPrompt().getAnswerText();

imageView = createAnswerImageView(getContext());
imageView.setOnClickListener(v -> {
if (imageClickHandler != null) {
imageClickHandler.clickImage("viewImage");
}
});
}

/**
Expand Down Expand Up @@ -221,7 +219,6 @@ public void clickImage(String context) {
}

private void launchDrawActivity() {
errorTextView.setVisibility(View.GONE);
Intent i = new Intent(getContext(), DrawActivity.class);
i.putExtra(DrawActivity.OPTION, drawOption);
if (binaryName != null) {
Expand Down Expand Up @@ -255,7 +252,6 @@ public void captureImage(Intent intent, final int requestCode, int stringResourc

@Override
public void chooseImage(@IdRes final int stringResource) {
errorTextView.setVisibility(View.GONE);
Intent i = new Intent(Intent.ACTION_GET_CONTENT);
i.setType("image/*");
launchActivityForResult(i, ApplicationConstants.RequestCodes.IMAGE_CHOOSER, stringResource);
Expand Down Expand Up @@ -303,8 +299,11 @@ private String getDefaultFilePath() {

protected abstract boolean doesSupportDefaultValues();

@Nullable
public ImageView getImageView() {
return imageView;
}

public TextView getErrorTextView() {
return errorTextView;
}
}
6 changes: 2 additions & 4 deletions collect_app/src/main/java/org/odk/collect/android/widgets/DrawWidget.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
import android.annotation.SuppressLint;
import android.content.Context;
import android.content.Intent;
import android.view.View;
import android.widget.Button;

import org.odk.collect.android.R;
Expand Down Expand Up @@ -47,7 +46,7 @@ public DrawWidget(Context context, QuestionDetails prompt, QuestionMediaManager

imageClickHandler = new DrawImageClickHandler(DrawActivity.OPTION_DRAW, RequestCodes.DRAW_IMAGE, R.string.draw_image);
setUpLayout();
addCurrentImageToLayout();
updateAnswer();
addAnswerView(answerLayout, WidgetViewUtils.getStandardMargin(context));
}

Expand All @@ -58,8 +57,7 @@ protected void setUpLayout() {

answerLayout.addView(drawButton);
answerLayout.addView(errorTextView);

errorTextView.setVisibility(View.GONE);
answerLayout.addView(imageView);
}

@Override
Expand Down
5 changes: 2 additions & 3 deletions collect_app/src/main/java/org/odk/collect/android/widgets/ImageWidget.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ public ImageWidget(Context context, final QuestionDetails prompt, QuestionMediaM
imageClickHandler = new ViewImageClickHandler();
imageCaptureHandler = new ImageCaptureHandler();
setUpLayout();
addCurrentImageToLayout();
updateAnswer();
addAnswerView(answerLayout, WidgetViewUtils.getStandardMargin(context));
}

Expand All @@ -85,9 +85,9 @@ protected void setUpLayout() {
answerLayout.addView(captureButton);
answerLayout.addView(chooseButton);
answerLayout.addView(errorTextView);
answerLayout.addView(imageView);

hideButtonsIfNeeded(appearance);
errorTextView.setVisibility(View.GONE);
}

@Override
Expand Down Expand Up @@ -141,7 +141,6 @@ private void hideButtonsIfNeeded(String appearance) {
}

private void captureImage() {
errorTextView.setVisibility(View.GONE);
if (selfie && new CameraUtils().isFrontCameraAvailable(getContext())) {
Intent intent = new Intent(getContext(), CaptureSelfieActivity.class);
intent.putExtra(CaptureSelfieActivity.EXTRA_TMP_PATH, new StoragePathProvider().getOdkDirPath(StorageSubdirectory.CACHE));
Expand Down
6 changes: 2 additions & 4 deletions collect_app/src/main/java/org/odk/collect/android/widgets/SignatureWidget.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
import android.annotation.SuppressLint;
import android.content.Context;
import android.content.Intent;
import android.view.View;
import android.widget.Button;

import org.odk.collect.android.R;
Expand Down Expand Up @@ -47,7 +46,7 @@ public SignatureWidget(Context context, QuestionDetails prompt, QuestionMediaMan

imageClickHandler = new DrawImageClickHandler(DrawActivity.OPTION_SIGNATURE, RequestCodes.SIGNATURE_CAPTURE, R.string.signature_capture);
setUpLayout();
addCurrentImageToLayout();
updateAnswer();
addAnswerView(answerLayout, WidgetViewUtils.getStandardMargin(context));
}

Expand All @@ -58,8 +57,7 @@ protected void setUpLayout() {

answerLayout.addView(signButton);
answerLayout.addView(errorTextView);

errorTextView.setVisibility(View.GONE);
answerLayout.addView(imageView);
}

@Override
Expand Down
40 changes: 38 additions & 2 deletions collect_app/src/test/java/org/odk/collect/android/widgets/AnnotateWidgetTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,14 @@
import org.odk.collect.shared.TempFiles;

import java.io.File;
import java.io.IOException;

import static java.util.Collections.singletonList;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.nullValue;
import static org.junit.Assert.assertNull;
import static org.mockito.Mockito.when;
import static org.odk.collect.android.support.CollectHelpers.overrideReferenceManager;
Expand Down Expand Up @@ -129,6 +131,40 @@ public void whenReadOnlyOverrideOptionIsUsed_shouldAllClickableElementsBeDisable
assertThat(getSpyWidget().annotateButton.getVisibility(), is(View.GONE));
}

@Test
public void whenThereIsNoAnswer_hideImageViewAndErrorMessage() {
AnnotateWidget widget = createWidget();

assertThat(widget.getImageView().getVisibility(), is(View.GONE));
assertThat(widget.getImageView().getDrawable(), nullValue());

assertThat(widget.getErrorTextView().getVisibility(), is(View.GONE));
}

@Test
public void whenTheAnswerImageCanNotBeLoaded_hideImageViewAndShowErrorMessage() throws IOException {
CollectHelpers.overrideAppDependencyModule(new AppDependencyModule() {
@Override
public ImageLoader providesImageLoader() {
return new SynchronousImageLoader(true);
}
});

String imagePath = File.createTempFile("current", ".bmp").getAbsolutePath();
currentFile = new File(imagePath);

formEntryPrompt = new MockFormEntryPromptBuilder()
.withAnswerDisplayText(DrawWidgetTest.USER_SPECIFIED_IMAGE_ANSWER)
.build();

AnnotateWidget widget = createWidget();

assertThat(widget.getImageView().getVisibility(), is(View.GONE));
assertThat(widget.getImageView().getDrawable(), nullValue());

assertThat(widget.getErrorTextView().getVisibility(), is(View.VISIBLE));
}

@Test
public void whenPromptHasDefaultAnswer_showsInImageView() throws Exception {
String imagePath = File.createTempFile("default", ".bmp").getAbsolutePath();
Expand All @@ -154,7 +190,7 @@ public ImageLoader providesImageLoader() {

AnnotateWidget widget = createWidget();
ImageView imageView = widget.getImageView();
assertThat(imageView, notNullValue());
assertThat(imageView.getVisibility(), is(View.VISIBLE));
Drawable drawable = imageView.getDrawable();
assertThat(drawable, notNullValue());

Expand All @@ -180,7 +216,7 @@ public ImageLoader providesImageLoader() {

AnnotateWidget widget = createWidget();
ImageView imageView = widget.getImageView();
assertThat(imageView, notNullValue());
assertThat(imageView.getVisibility(), is(View.VISIBLE));
Drawable drawable = imageView.getDrawable();
assertThat(drawable, notNullValue());

Expand Down
Loading