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

Form error improvements #5560

Merged
merged 6 commits into from
Jul 12, 2023
Merged

Form error improvements #5560

merged 6 commits into from
Jul 12, 2023

Conversation

grzesiek2010
Copy link
Member

@grzesiek2010 grzesiek2010 commented Apr 25, 2023

Closes #5507
Closes #5361

What has been done to verify that this works as intended?

I've tested the changes manually and added automated tests.

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

As discussed in the issue we have two types of errors (non-fatal and fatal). If a form contains a non-fatal error there is no need to remember it and restore it when the activity is recreated. If it's a fatal error of course the form view should be closed immediately after clicking the OK button.

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?

Please test both types of form design errors. I don't see anything that should be reviewed more carefully.

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

The forms that are attached to the issues represent non-fatal errors. If you have a form with a fatal error that closes the form view immediately after clicking the OK button then please use it if not you can use the one we have in this test https://github.com/getodk/collect/pull/5560/files#diff-ecc5077e0136dfcd8dedd8caa30a17ecf92399b13200c97e9661a44a7fcd0bd4R24

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:

  • run ./gradlew checkAll and confirmed all checks still pass OR confirm CircleCI build passes and run ./gradlew connectedDebugAndroidTest locally.
  • 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

@dbemke
Copy link

dbemke commented Jul 7, 2023

Following the steps in #5507 allows to save a draft or send the form.
Now while editing the saved form the error dialog appears and after clicking "ok" in the dialog, the form closes- not sure if it's expected?.
If the new form was sent and the user clicks "View" in the snackbar there is a loading state of the form with the error dialog (tapping "ok" closes the view)
viewFormWith Error

@grzesiek2010
Copy link
Member Author

To me it seems right. It works in the same way like before right so reopening such a form is not possible? If so it's fine. I didn't change that behavior. What I've fixed regarding that issue is recreating that error dialog what I've described in #5507 (comment)


We have a mechanism for restoring the error dialog. That means if a user rotates a screen or minimizes the app the dialog is restored. This makes a lot of sense in case of fatal errors because we want to block users from filling such forms and those forms should be immediately closed after closing the error dialog.
However, it doesn't make sense in the case of non-fatal errors. Here the error is remembered and even when we close the error dialog and want to continue filling a form like described in the issue the error dialog will be recreated when for example:

- a user rotates the device
- a user goes to the hierarchy view and back to the form
- a user minimizes the app and reopens it

In all those cases the error dialog will be recreated and it might be displayed in a completely different place in a form (not in the place when it really took place). This is very confusing. Additionally, such a recreated non-fatal error becomes a fatal one and when a user clicks the ok button to close it the whole form is closed like it was a fatal error. This is exactly what is happening in this issue.

@srujner
Copy link

srujner commented Jul 12, 2023

Tested with Success!

Verified on device with Android 12 and 13

Verified cases:

  • Forms with Non Fatal error;
  • Forms with Fatal error;
  • Minimizing the app and Rotating the device when error appears;
  • Going to Hierarchy view and back to the form;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants