Making the "Ready to Send" screen more actionable #5693

grzesiek2010 · 2023-07-25T20:38:53Z

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?

There is nothing important to discuss here. I've created a custom view for the new element to make testing easier and isolated.

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?

It's rather an isolated change so testing the list of forms to make sure the banner is displayed correctly would be enough.

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

No.

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

grzesiek2010 · 2023-07-26T09:17:49Z

strings/src/main/res/values/strings.xml

+    # Forms ready to send banner
+    ##############################################
+    -->
+    <string name="last_form_sent_seconds_ago">Last form sent: %d second(s) ago</string>


I could have used Quantity strings instead of adding (s) but we usually use the latter because it's easier. Supporting so many languages and keeping such strings translated correctly would be difficult.

In this case I think we should go for the quantity string. Many workflows involve looking at this screen a LOT. I think we want it to look as clean as we can. CC @alyblenkin

I agree! I took a screengrab below – it isn't great for readability.

Ok, I've improved that.

seadowg · 2023-08-01T15:53:55Z

@alyblenkin probably good for you to have a look at this before I dive into a code review

alyblenkin · 2023-08-01T16:31:55Z

I think this is looking good!

This is very picky, but is it possible to align the text?

It doesn't read well with the (s), but it looks like that's easier from the comment above and better for translations.

grzesiek2010 · 2023-08-09T13:55:47Z

This is very picky, but is it possible to align the text?

@alyblenkin
I'm not sure if I understand what you want to change there. Could you explain?

alyblenkin · 2023-08-09T14:58:21Z

Maybe this is a better screenshot to explain what I mean

grzesiek2010 · 2023-08-10T08:31:25Z

Ah thanks now I see, ok it's fixed.

alyblenkin

Looks great!

collect_app/src/main/AndroidManifest.xml

collect_app/src/main/java/org/odk/collect/android/readytosend/ReadyToSendBanner.kt

...androidTest/java/org/odk/collect/android/feature/instancemanagement/SendFinalizedFormTest.kt

collect_app/src/main/res/layout/ready_to_send_banner.xml

…gemen'

collect_app/src/main/java/org/odk/collect/android/instancemanagement/send/ReadyToSendBanner.kt

...ct_app/src/main/java/org/odk/collect/android/instancemanagement/send/ReadyToSendViewModel.kt

seadowg

Just a couple of tweaks to tests here so this is ready for QA!

...t_app/src/test/java/org/odk/collect/android/instancemanagement/send/ReadyToSendBannerTest.kt

...ct_app/src/main/java/org/odk/collect/android/instancemanagement/send/ReadyToSendViewModel.kt

seadowg · 2023-09-08T10:10:22Z

...pp/src/test/java/org/odk/collect/android/instancemanagement/send/ReadyToSendViewModelTest.kt

+
+    @Test
+    fun `numberOfSentInstances should represent the real number of instances with STATUS_SUBMITTED in the database`() {
+        scheduler.runBackground()


It surprised me that you didn't also have to call runForeground to have data updated by the foreground part of the task. It looks like FakeScheduler runs the foreground part for you though as it splits "background" and "foreground" on a slightly different level (the way the Scheduler was called really). Do you think we should change it so that runForeground would be required here? I could go either way on it really!

The real implementation works in such a way that the foreground part is triggered automatically once the background one is done so I think in tests it can be like that too unless we need to do something in between. For now, we don't need that so I think we can leave it as-is.

dbemke · 2023-09-12T08:05:35Z

@grzesiek2010 Can you define "more actionable”? When the new banner is shown – what does it mean displayed correctly? Is it connected with auto send options etc.? Does the PR include also the new snackbar "Look like you are offline…” ?

dbemke · 2023-09-12T08:25:25Z

It seems that the banner is refreshed only if the user reopens "Ready to send" ( goes to the main menu and back to "Ready to send").
When a first form was sent in a new project and there is one more form to send the banner is not shown unless "Ready to send" is reopened. The screenshot shows "Ready to send" after sending the first form in a project - no banner.

If the user reopens "Ready to send" and sends some more forms e.g. in the screenshot the last form was just sent the banner doesn't change and there is still information about 1 form to send although it was already sent which might be very confusing for the user.

If don't keep activities setting is on the banner and the time is refreshed after sending without reopening "Ready to send"

alyblenkin · 2023-09-12T18:01:50Z

@grzesiek2010 Can you define "more actionable”? When the new banner is shown – what does it mean displayed correctly? Is it connected with auto send options etc.? Does the PR include also the new snackbar "Look like you are offline…” ?

@dbemke - By actionable we mean easier for the user to understand the status of when their forms were last sent and which ones still need to be actioned, so they can make an informed decision within this page. Right now it's hard for the user to remeber when they last sent their forms without leaving the page and checking their "Sent" forms. We've talked about having a snackbar to indicate when the user is offline however, it's a bit more complicated to implement, so we aren't showing it as part of this issue.

If the user reopens "Ready to send" and sends some more forms e.g. in the screenshot the last form was just sent the banner doesn't change and there is still information about 1 form to send although it was already sent which might be very confusing for the user. If don't keep activities setting is on the banner and the time is refreshed after sending without reopening "Ready to send"

Agreed, this would be a very confusing state. The activity banner should still be at the top to indicate the last sent form. The second line say "0 forms to send" instead of displaying the empty state with the banner.

grzesiek2010 · 2023-09-13T21:29:40Z

It seems that the banner is refreshed only if the user reopens "Ready to send" ( goes to the main menu and back to "Ready to send").

All those problems should be fixed.

dbemke · 2023-09-14T11:18:27Z

After the fix when the user sends forms in "Ready to send" the banner is updated but if a form was sent with a big sized file (sending takes some time) in "Start new form" and then goes to "Ready to send" after the form was sent there is still info about 1 form to send.

Steps to reproduce:

In "Start new form" send a form with a big file.
Go to "Ready to Send" (the form is still there being sent).
Wait until the form was sent and check the banner.

grzesiek2010 · 2023-09-15T09:36:24Z

@dbemke I'm not able to reproduce but I've added one change so please give it a try maybe now it works well.

dbemke · 2023-09-15T13:39:27Z

Just had a quick look at the fix and seems to be working well. We'll finish testing the PR after Central testing is finished.

dbemke · 2023-09-26T08:56:20Z

The banner takes too much space if RTL is applied. There isn't the information how many forms are ready to send.

Steps to reproduce:

In Collect settings auto send is set to off and RTL language.
Finalize some forms.
Go to Ready to send

grzesiek2010 · 2023-09-26T11:15:17Z

The banner takes too much space if RTL is applied. There isn't the information how many forms are ready to send.

Fixed.

srujner · 2023-09-26T12:24:33Z

Tested with Success!

Verified on device with Android 13

Verified cases:

Regression Check on Form lists;
Sending finalized Forms, Editing saved Forms, Viewing sent Forms, Deleting saved Forms;
All issues from comments has been fixed;
Very long list of forms
Sort by name, by date added
Change language and use sorting options
RTL language
Use search bar
Screen rotation
Try to multiclik on form list;
Light and Dark theme.

dbemke · 2023-09-26T12:26:25Z

Tested with Success!

Verified on device with Android 10

grzesiek2010 force-pushed the COLLECT-5605 branch from 8226f0f to e7fdc1d Compare July 25, 2023 20:49

grzesiek2010 marked this pull request as ready for review July 25, 2023 21:15

grzesiek2010 requested a review from seadowg July 25, 2023 21:15

grzesiek2010 commented Jul 26, 2023

View reviewed changes

seadowg removed their request for review August 1, 2023 15:54

grzesiek2010 force-pushed the COLLECT-5605 branch from a00922e to f79165b Compare August 9, 2023 14:18

grzesiek2010 requested a review from alyblenkin August 10, 2023 08:31

alyblenkin approved these changes Aug 10, 2023

View reviewed changes

grzesiek2010 requested a review from seadowg August 10, 2023 17:39

seadowg requested changes Aug 28, 2023

View reviewed changes

grzesiek2010 force-pushed the COLLECT-5605 branch 4 times, most recently from 3a63570 to 2c49cc1 Compare August 29, 2023 12:41

grzesiek2010 requested a review from seadowg August 29, 2023 13:09

grzesiek2010 added 10 commits August 31, 2023 12:42

Added new strings

48be3ab

Added banner

5d3c075

Moved related classes to one package

e4eb74a

Added tests

33d0f65

Improved strings

5858729

Fixed margins

656ba4e

Fixed whenThereAreSentAndReadyToSendForms_displayTheBanner test

b29e21e

Added missing text appearance

fd65ab5

Renamed 'readytosend' package to 'send' and moved it to 'instancemana…

7bf40a6

…gemen'

Reworked ReadyToSendBanner to use viewmodel

d9a2217

grzesiek2010 force-pushed the COLLECT-5605 branch from 2c49cc1 to 0721c66 Compare August 31, 2023 11:22

seadowg requested changes Aug 31, 2023

View reviewed changes

grzesiek2010 added 2 commits September 1, 2023 09:38

Factored out TimeInMs object

603eb0f

Code improvements

639576e

grzesiek2010 requested a review from seadowg September 1, 2023 15:07

seadowg requested changes Sep 8, 2023

View reviewed changes

seadowg added the needs testing label Sep 8, 2023

grzesiek2010 added 4 commits September 13, 2023 21:21

Update the banner after sending forms

c64065c

Improved ReadyToSendViewModelTest

259040b

Imroved ReadyToSendBannerTest

eea486d

Improved ReadyToSendBannerTest

86adad6

grzesiek2010 requested a review from seadowg September 13, 2023 21:33

Improve updating the banner

e52bf13

grzesiek2010 force-pushed the COLLECT-5605 branch from 57076bc to e52bf13 Compare September 15, 2023 09:25

seadowg approved these changes Sep 15, 2023

View reviewed changes

Fixed ready_to_sen_banner layout

67d59bf

dbemke added behavior verified and removed needs testing labels Sep 26, 2023

grzesiek2010 merged commit ab1af33 into getodk:master Sep 26, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Making the "Ready to Send" screen more actionable #5693

Making the "Ready to Send" screen more actionable #5693

grzesiek2010 commented Jul 25, 2023 •

edited

Loading

grzesiek2010 Jul 26, 2023

lognaturel Aug 1, 2023 •

edited

Loading

alyblenkin Aug 1, 2023

grzesiek2010 Aug 9, 2023

seadowg commented Aug 1, 2023

alyblenkin commented Aug 1, 2023

grzesiek2010 commented Aug 9, 2023

alyblenkin commented Aug 9, 2023

grzesiek2010 commented Aug 10, 2023

alyblenkin left a comment

seadowg left a comment

seadowg Sep 8, 2023

grzesiek2010 Sep 13, 2023

dbemke commented Sep 12, 2023

dbemke commented Sep 12, 2023 •

edited

Loading

alyblenkin commented Sep 12, 2023

grzesiek2010 commented Sep 13, 2023

dbemke commented Sep 14, 2023

grzesiek2010 commented Sep 15, 2023

dbemke commented Sep 15, 2023

dbemke commented Sep 26, 2023 •

edited

Loading

grzesiek2010 commented Sep 26, 2023

srujner commented Sep 26, 2023

dbemke commented Sep 26, 2023

Making the "Ready to Send" screen more actionable #5693

Making the "Ready to Send" screen more actionable #5693

Conversation

grzesiek2010 commented Jul 25, 2023 • edited Loading

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

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

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?

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

Does this change require updates to documentation? If so, please file an issue here and include the link below.

Before submitting this PR, please make sure you have:

grzesiek2010 Jul 26, 2023

Choose a reason for hiding this comment

lognaturel Aug 1, 2023 • edited Loading

Choose a reason for hiding this comment

alyblenkin Aug 1, 2023

Choose a reason for hiding this comment

grzesiek2010 Aug 9, 2023

Choose a reason for hiding this comment

seadowg commented Aug 1, 2023

alyblenkin commented Aug 1, 2023

grzesiek2010 commented Aug 9, 2023

alyblenkin commented Aug 9, 2023

grzesiek2010 commented Aug 10, 2023

alyblenkin left a comment

Choose a reason for hiding this comment

seadowg left a comment

Choose a reason for hiding this comment

seadowg Sep 8, 2023

Choose a reason for hiding this comment

grzesiek2010 Sep 13, 2023

Choose a reason for hiding this comment

dbemke commented Sep 12, 2023

dbemke commented Sep 12, 2023 • edited Loading

alyblenkin commented Sep 12, 2023

grzesiek2010 commented Sep 13, 2023

dbemke commented Sep 14, 2023

grzesiek2010 commented Sep 15, 2023

dbemke commented Sep 15, 2023

dbemke commented Sep 26, 2023 • edited Loading

grzesiek2010 commented Sep 26, 2023

srujner commented Sep 26, 2023

dbemke commented Sep 26, 2023

grzesiek2010 commented Jul 25, 2023 •

edited

Loading

lognaturel Aug 1, 2023 •

edited

Loading

dbemke commented Sep 12, 2023 •

edited

Loading

dbemke commented Sep 26, 2023 •

edited

Loading