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

Show dialog when the user sees the drafts screen for the first time #5785

Merged
merged 7 commits into from
Oct 20, 2023

Conversation

seadowg
Copy link
Member

@seadowg seadowg commented Oct 18, 2023

The dialog currently only shows in non debug builds to avoid needing to build test infrastructure to deal with it.

@seadowg seadowg changed the base branch from master to v2023.3.x October 18, 2023 16:49
@seadowg seadowg marked this pull request as ready for review October 18, 2023 16:51
@dbemke
Copy link

dbemke commented Oct 19, 2023

The dialog is shown in Sent.
SentDraftDialog

Steps to reproduce:

  1. Install the app.
  2. Try demo project.
  3. Tap "Sent" in the main menu

@dbemke
Copy link

dbemke commented Oct 19, 2023

What is the expected result when there are 2 projects in Collect? The dialog is shown in Drafts only in the first project which is opened (when in the second project Drafts is opened for the first time- no dialog)?

@lognaturel
Copy link
Member

The dialog is shown in Sent.

Arg, good catch.

What is the expected result when there are 2 projects in Collect?

We need to be mindful of that in criteria. I think once overall is fine.

@grzesiek2010
Copy link
Member

The dialog is shown in Sent.

This should be fixed.

@lognaturel
Copy link
Member

@seadowg and I briefly discussed this and were hoping to defer the test infrastructure for this until we had more of these education dialogs! But I think your interim approach of dismissing the dialog as needed is ok and we can come back to it. I guess having the dialog show in debug builds and running tests would have revealed the Sent issue!

@dbemke
Copy link

dbemke commented Oct 20, 2023

The dialog disappears (in Drafts) after the screen rotation. I think it would be useful to keep the dialog after the rotation cause users may accidentally change the position of the phone without having a chance to read the dialog.
It's also possible to tap beyond the dialog and the dialog disappears so users won't have a chance to read the dialog in this case, too.

@grzesiek2010
Copy link
Member

The dialog disappears (in Drafts) after the screen rotation. I think it would be useful to keep the dialog after the rotation cause users may accidentally change the position of the phone without having a chance to read the dialog.
It's also possible to tap beyond the dialog and the dialog disappears so users won't have a chance to read the dialog in this case, too.

I think @seadowg used that typo of dialog deliberately maybe even after discussing it with @lognaturel so let's wait for what @lognaturel thinks. To me, it's not that important.

@srujner
Copy link

srujner commented Oct 20, 2023

@lognaturel We have already tested everything and there are no more bugs. If you think that the things in Dominika's comment are ok, you can close and merge this PR.

@grzesiek2010 grzesiek2010 modified the milestones: v2023.2.x, v2023.3 Oct 20, 2023
@lognaturel
Copy link
Member

Great, it seems ok to me, thanks.

@grzesiek2010 grzesiek2010 merged commit 17579c7 into getodk:v2023.3.x Oct 20, 2023
6 checks passed
@seadowg seadowg deleted the education-dialog branch October 24, 2023 08:49
seadowg pushed a commit to seadowg/collect that referenced this pull request Oct 25, 2023
Show dialog when the user sees the drafts screen for the first time
seadowg pushed a commit to seadowg/collect that referenced this pull request Oct 30, 2023
Show dialog when the user sees the drafts screen for the first time
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants