Skip to content

Commit

Permalink
fix: Show "discard edits" not "discard observation" when cancelling e…
Browse files Browse the repository at this point in the history
…dit (#472)

* chore: use react-native touchables in tests (RNGH = flaky tests)

It seems like in e2e tests the detox tap() is too fast for RNGH to
register, causing button taps to fail:
software-mansion/react-native-reanimated#596

* fix: Show "discard edits" not "discard observation" when cancelling edit

fixes #381

* add tests

* fix crash in test
  • Loading branch information
gmaclennan committed Sep 29, 2020
1 parent 946d33b commit 211d855
Show file tree
Hide file tree
Showing 10 changed files with 97 additions and 38 deletions.
50 changes: 40 additions & 10 deletions e2e/firstTest.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,6 @@ describe("Mapeo", () => {
});

describe("Create observation", () => {
test("Tapping add from map screen shows 'choose what is happening' screen", async () => {
await byId("addButtonMap").tap();
await expect(byText("Choose what is happening")).toBeVisible();
});

// expo-camera does not work in an emulator see
// https://github.com/expo/expo/issues/5529
test.skip("Tapping add from camera screen shows 'choose what is happening' screen", async () => {
Expand All @@ -42,6 +37,42 @@ describe("Mapeo", () => {
// await waitForVisible(byText("Choose what is happening"), 10000);
await expect(byText("Choose what is happening")).toBeVisible();
});

test("Clicking back button after add shows confirmation alert and discards observation", async () => {
await byId("addButtonMap").tap();
await device.pressBack();
await expect(byText("Discard observation?")).toBeVisible();
await byText("DISCARD WITHOUT SAVING").tap();
await byId("observationListButton").tap();
await expect(byId("observationsEmptyView")).toExist();
});

test("Can create observation", async () => {
await byId("addButtonMap").tap();
await byId("animalCategoryButton").tap();
await byId("saveButton").tap();
await byText("SAVE").tap();
await byId("observationListButton").tap();
await expect(byId("observationListItem:0")).toExist();
});

// NOTE: This test relies on the observation created by the previous test
test("Clicking back button after edit shows confirmation alert and discards edits", async () => {
await byId("observationListButton").tap();
await byId("observationListItem:0").tap();
await byId("editButton").tap();
await byId("observationDescriptionField").typeText("Test description");
// Closes keyboard
await device.pressBack();
// Cancels edit
await device.pressBack();
// Checks this says "Discard changes" not "Discard observation", which
// happens when cancelling a new observation
await expect(byText("Discard changes?")).toBeVisible();
await expect(byText("DISCARD CHANGES")).toBeVisible();
// Delete and re-install to remove added observation
await device.launchApp({ delete: true });
});
});

describe("GPS", () => {
Expand All @@ -52,7 +83,6 @@ describe("Mapeo", () => {
});
});

// NOTE: This should be last, because of the restart after language change
describe("Settings", () => {
test("Clicking observation list then settings icon shows settings screen", async () => {
await navigateToSettings();
Expand All @@ -65,7 +95,7 @@ describe("Mapeo", () => {
await expect(byText("Mapeo version")).toBeVisible();
});

// NOTE: This should be the last test within "Settings"
// NOTE: This test needs to be last
test("Changing language in settings changes app language", async () => {
await navigateToSettings();
await byId("settingsLanguageButton").tap();
Expand All @@ -80,9 +110,9 @@ describe("Mapeo", () => {
// language stays on the same screen
await byId("observationListButton").tap();
await expect(byText("Observaciones")).toBeVisible();
// Delete and re-install the app after changing language, so that it
// resets to the default language
await device.launchApp({ delete: true });
// This was failing in Github actions for some reason, so this test needs
// to be last (since everything will be in Spanish from now on)
// await device.launchApp({ delete: true });
});
});
});
12 changes: 10 additions & 2 deletions messages/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,20 @@
"description": "Button on dialog to keep editing (cancelling close action)",
"message": "Continue editing"
},
"AppContainer.EditHeader.discardChangesContent": {
"description": "Button on dialog to cancel observation edits",
"message": "Discard changes"
},
"AppContainer.EditHeader.discardChangesTitle": {
"description": "Title of dialog that shows when cancelling observation edits",
"message": "Discard changes?"
},
"AppContainer.EditHeader.discardContent": {
"description": "Button on dialog to close without saving",
"description": "Button on dialog to cancel a new observation",
"message": "Discard without saving"
},
"AppContainer.EditHeader.discardTitle": {
"description": "Title of dialog that shows when closing an observation without saving",
"description": "Title of dialog that shows when cancelling a new observation",
"message": "Discard observation?"
},
"screens.AboutMapeo.androidBuild": {
Expand Down
1 change: 1 addition & 0 deletions src/frontend/screens/CategoryChooser.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ const Item = React.memo(
onPress={() => onSelect(item)}
activeOpacity={1}
underlayColor="#000033"
testID={`${item.id}CategoryButton`}
>
<View style={styles.cellContainer}>
<CategoryCircleIcon iconId={item.icon} size="medium" />
Expand Down
2 changes: 1 addition & 1 deletion src/frontend/screens/Observation/ObservationHeaderRight.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ const ObservationHeaderRight = ({ navigation }: Props) => {
if (!observation) return null;
const isMine = observation.value.deviceId === deviceId;
return isMine ? (
<IconButton onPress={handlePress}>
<IconButton onPress={handlePress} testID="editButton">
<EditIcon />
</IconButton>
) : (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ const DescriptionField = () => {
autoFocus={false}
scrollEnabled={false}
textContentType="none"
testID="observationDescriptionField"
/>
)}
</Field>
Expand Down
2 changes: 1 addition & 1 deletion src/frontend/screens/ObservationEdit/SaveButton.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ const SaveButton = ({ navigation }: Props) => {
}, [savingStatus, navigation]);

return (
<IconButton onPress={handleSavePress}>
<IconButton onPress={handleSavePress} testID="saveButton">
<SaveIcon inprogress={savingStatus === "loading"} />
</IconButton>
);
Expand Down
4 changes: 3 additions & 1 deletion src/frontend/screens/ObservationsList/ObservationListItem.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ type Props = {
onPress: string => any,
style?: ViewStyleProp,
observationId: string,
testID: string,
};

const photoOverlap = 10;
Expand Down Expand Up @@ -49,6 +50,7 @@ const ObservationListItem = ({
onPress = () => {},
style,
observationId,
testID,
}: Props) => {
const [{ observation, preset }] = useObservation(observationId);
const deviceId = useDeviceId();
Expand All @@ -62,7 +64,7 @@ const ObservationListItem = ({
return (
<TouchableHighlight
onPress={() => onPress(observationId)}
testID={"ObservationListItem:" + observationId}
testID={testID}
style={{ flex: 1, height: 80 }}
>
<View
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ type Props = {
const ObservationEmptyView = ({ onPressBack }: Props) => {
const { formatMessage: t } = useIntl();
return (
<View style={styles.root} testID="observationEmptyView">
<View style={styles.root} testID="observationsEmptyView">
<View style={styles.iconContainer}>
<View style={styles.iconCircle}>
<ObservationListIcon size={150} />
Expand Down
5 changes: 3 additions & 2 deletions src/frontend/screens/ObservationsList/ObservationsListView.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ const ObservationsListView = ({
}

return (
<View style={styles.container}>
<View style={styles.container} testID="observationsListView">
<FlatList
initialNumToRender={rowsPerWindow}
getItemLayout={getItemLayout}
Expand All @@ -94,10 +94,11 @@ const ObservationsListView = ({
scrollViewContent={styles.scrollViewContent}
windowSize={3}
removeClippedSubviews
renderItem={({ item }) => {
renderItem={({ item, index }) => {
return (
<ObservationListItem
key={item.id}
testID={`observationListItem:${index}`}
observationId={item.id}
style={styles.listItem}
onPress={onPressObservation}
Expand Down
56 changes: 36 additions & 20 deletions src/frontend/sharedComponents/CustomHeaderLeft.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,22 @@ const m = defineMessages({
discardTitle: {
id: "AppContainer.EditHeader.discardTitle",
defaultMessage: "Discard observation?",
description:
"Title of dialog that shows when closing an observation without saving",
description: "Title of dialog that shows when cancelling a new observation",
},
discardConfirm: {
id: "AppContainer.EditHeader.discardContent",
defaultMessage: "Discard without saving",
description: "Button on dialog to close without saving",
description: "Button on dialog to cancel a new observation",
},
discardChangesTitle: {
id: "AppContainer.EditHeader.discardChangesTitle",
defaultMessage: "Discard changes?",
description: "Title of dialog that shows when cancelling observation edits",
},
discardChangesConfirm: {
id: "AppContainer.EditHeader.discardChangesContent",
defaultMessage: "Discard changes",
description: "Button on dialog to cancel observation edits",
},
discardCancel: {
id: "AppContainer.EditHeader.discardCancel",
Expand Down Expand Up @@ -100,8 +109,6 @@ const CustomHeaderLeft = ({ onPress: originalOnPress, ...props }: any) => {
routeName === "CategoryChooser" &&
prevRouteNameInStack === "Home");

const shouldCloseToHome = isNew && shouldConfirm;

const handleCloseRequest = React.useCallback(() => {
const isUntouched =
existingObservation &&
Expand All @@ -116,27 +123,36 @@ const CustomHeaderLeft = ({ onPress: originalOnPress, ...props }: any) => {
return;
}

Alert.alert(t(m.discardTitle), undefined, [
{
text: t(m.discardConfirm),
onPress: () => {
clearDraft();
if (shouldCloseToHome) navigation.navigate("Home");
else navigation.goBack();
if (isNew) {
Alert.alert(t(m.discardTitle), undefined, [
{
text: t(m.discardConfirm),
onPress: () => {
clearDraft();
navigation.navigate("Home");
},
},
},
{
text: t(m.discardCancel),
onPress: () => {},
},
]);
{ text: t(m.discardCancel), onPress: () => {} },
]);
} else {
Alert.alert(t(m.discardChangesTitle), undefined, [
{
text: t(m.discardChangesConfirm),
onPress: () => {
clearDraft();
navigation.goBack();
},
},
{ text: t(m.discardCancel), onPress: () => {} },
]);
}
}, [
clearDraft,
draftObservation.photos,
draftObservation.value,
existingObservation,
isNew,
navigation,
shouldCloseToHome,
shouldConfirm,
t,
]);
Expand All @@ -155,7 +171,7 @@ const CustomHeaderLeft = ({ onPress: originalOnPress, ...props }: any) => {
<HeaderBackButton
{...props}
onPress={handleCloseRequest}
backImage={shouldCloseToHome ? HeaderCloseIcon : HeaderBackIcon}
backImage={isNew && shouldConfirm ? HeaderCloseIcon : HeaderBackIcon}
/>
);
};
Expand Down

0 comments on commit 211d855

Please sign in to comment.