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.

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

Feat observation fields #352

Merged
merged 6 commits into from
May 15, 2024
Merged

Feat observation fields #352

merged 6 commits into from
May 15, 2024

Conversation

ErikSin
Copy link
Contributor

@ErikSin ErikSin commented May 14, 2024

Add observation fields to observation edit screen and displays observations fields when viewing an observation

image image image

@ErikSin ErikSin requested a review from achou11 May 14, 2024 23:46
@ErikSin
Copy link
Contributor Author

ErikSin commented May 14, 2024

@achou11 alot of this is ported over from mapeo-mobile

Copy link
Member

@achou11 achou11 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple of non-blocking nits but overall didn't find any notable issues when testing


const defaultAcc: Field[] = [];
const fields = !fieldsQuery.data
? undefined
const fields = !data
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: invert the ternary to avoid the negation

@@ -61,7 +59,7 @@ export const ObservationScreen: NativeNavigationComponent<'Observation'> = ({
const {lat, lon, createdBy} = observation;
const isMine = deviceId === createdBy;
// Currently only show photo attachments
const photos = [];
// const photos = [];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: maybe just remove this + comment above?

// onPress: handleDetailsPress,
// });
// }
if (preset?.fieldIds && preset.fieldIds.length) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: think this can be shortened?

Suggested change
if (preset?.fieldIds && preset.fieldIds.length) {
if (preset?.fieldIds?.length) {

{...m.title}
values={{
current: questionNumber,
total: !preset ? 0 : preset.fieldIds.length,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: invert ternary

@ErikSin ErikSin merged commit b330791 into main May 15, 2024
3 checks passed
@ErikSin ErikSin deleted the feat-observation-fields branch May 15, 2024 20:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants