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

Removed Intro screen #274

Merged
merged 9 commits into from
Dec 19, 2016
Merged

Removed Intro screen #274

merged 9 commits into from
Dec 19, 2016

Conversation

max2me
Copy link
Contributor

@max2me max2me commented Nov 28, 2016

This PR is in reference to resolve #182. It removes an intro screen shown when users begin to fill out new form.

Tested scenarios

A. No intro screen

  1. Start filling out blank form that does not start with a repeat group

Expected result:
Should be taken directly to the first question.

B. Swiping back

  1. Start filling out blank form that does not start with a repeat group
  2. Swipe back

Expected result:
User should stay on the first question, no animations should occur.

C. Repeat group form

  1. Start filling out "Biggest N of Set" form

Expected result:
Dialog should appear offering to add new group. If group is added, then user can fill it out.

If user presses "Do not add" button, then it should skip the group altogether. If user skipped, then swiping back should not do anything. At this point the only way to invoke dialog is to tap button on the toolbar "Go to Prompt" and choose "Go to start" at which point it should work from step 1 above.

@lognaturel
Copy link
Member

I'm starting to see some of the challenges here. Removing the first group in a form starting with a repeat is broken. Add a single group and then long press on the first field to remove the group. This used to bring the user back to the landing screen but now the group is still shown even though it doesn't exist. Add two groups and you'll see the second one can be removed without trouble but the first one not. I suppose deleting the first group should now result in advancing forward rather than back.

There's also a problem with navigation from the first non-group question. Start Biggest N of Set, don't add a group, swipe back, swipe forward and notice you get the first non-group question twice. Also, don't add a group, swipe back, close the app, reopen it and you will be prompted to add a group.

Additionally, it's possible to disable "Go to Prompt" navigation so there does need to be another way to be able to create groups. Ideally, swiping back from the beginning of the form when it starts with a repeat would prompt to create a group.

@max2me
Copy link
Contributor Author

max2me commented Nov 29, 2016

@lognaturel Thank you for sharing these cases. I'll work on them.

  1. Handle removing of the first group in a form starting with a repeat group
  2. Handle double question shown in Biggest N of Set
  3. Provide opportunity to add group when Go To Prompt is disabled

If I recall correctly, when navigating back right now it skips over prompt step so I'll see what can be done there.

I'm starting to think that instead of throwing a modal dialog prompting them to add a group, we can convert it into a non-modal view that has a button "Add another group". That way we have something to show when users swipe back and it will also be less disruptive to the user experience (my bet is users get used to swipes to advance yet with a dialog they have to press a button which might cause a bit of cognitive dissonance).

Just a thought...
If we focus on the goal at hand (streamlining the flow for users) we can get a simpler solution in place. One option would be to leave intro screen in place to support all existing navigation patterns BUT skip it once when users start to fill out the form. This will save them one swipe in a core flow yet will require minimal changes (I think) / will lead to fewer downsides (one of the tricky parts with beginning of the form is how deeply integrated it is with Form Controller).

@lognaturel
Copy link
Member

lognaturel commented Nov 29, 2016

I think given the conversations around this it's time for the screen to go. Changing the modal dialog is going to be a pretty disruptive change.

I didn't think about this deeply and it's not super elegant but it seems adding something like the following at line 1309 could solve the last two problems:

if (event == FormEntryController.EVENT_PROMPT_NEW_REPEAT) {
    createRepeatDialog();
}

@max2me
Copy link
Contributor Author

max2me commented Nov 29, 2016

@lognaturel I could be wrong (will look into this), but I believe when navigating back via stepToPreviousScreenEvent(), EVENT_PROMPT_NEW_REPEAT gets skipped over along with some other conditions. Still need to understand why and if it's something we should change there.

@max2me
Copy link
Contributor Author

max2me commented Nov 29, 2016

@lognaturel You were right about that change, in my mind I mistakenly added it to 2 lines down which wouldnt work. But on line 1309 it definitely works.

@max2me
Copy link
Contributor Author

max2me commented Nov 29, 2016

Okay, I think that should work now. Here is what I tested:

Removing first group when only one added

  1. Start filling out Biggest N of Set
  2. Add a group
  3. Press & Hold to remove it

Expected result:
Dialog being shown if a new group should be added.
If user presses "Do not add", then next question should be shown.
If user presses "Add group", then group should be added.

Remove first group when 2 have been added

  1. Start filling out Biggest N of Set
  2. Add a group, enter "A" as a name
  3. Swipe next
  4. Add a group, enter "B" as a name
  5. Swipe back
  6. While at group A, press and hold on the field, choose Remove group

Expected result:
Should get to group B, when swiping back "Repeat dialog" should be shown, if swiping forward, next question should be shown.

Actual result:
Stays on group A. Filed this as #276 as I can reproduce it on latest master.

Provide an opportunity to add group after initial decline

  1. Start filling out Biggest N of Set
  2. Decline to Add to a group
  3. Swipe back

Expected:
Dialog prompting adding of a new group. If declined, user should go to first non-group question. Swiping back brings dialog again, swiping forward takes user to the next question.

@lognaturel lognaturel mentioned this pull request Dec 1, 2016
Copy link
Member

@lognaturel lognaturel left a comment

Choose a reason for hiding this comment

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

Nice catch on #276, I didn't notice it was an old problem! @yanokwa looks ready to me.

@yanokwa
Copy link
Member

yanokwa commented Dec 2, 2016

@max2me In the "Removing first group when only one added" and "Provide an opportunity to add group after initial decline" scenarios, when the dialog prompts to add a new group, I was expecting to see the blank no question screen that you get when the form starts.

I'll admit it is bit of a nit pick, but I think it could be confusing when you have a group filled in, you delete it, and you still see that group data looking at you. I don't want to let perfect be the enemy of good, but how hard is it to get the behavior I was expecting?

@max2me
Copy link
Contributor Author

max2me commented Dec 6, 2016

@yanokwa Let me look into it. I think showing blank screen behind adding group dialog is fairly straightforward but I'm not sure about removing group scenario (since there is not a dedicated event in form controller for that action, it might be a bit trickier).

createRepeatDialog() is now directly tied to the current event of FormEntryController which simplifies logic a bit
@max2me
Copy link
Contributor Author

max2me commented Dec 6, 2016

@yanokwa @lognaturel We are now showing a blank view whenever presenting "add repeat group" dialog. This change also cleaned up logic a bit.

I've also added showing blank screen when confirming deletion of the group and it subsequently fixed #276 too.

@max2me
Copy link
Contributor Author

max2me commented Dec 6, 2016

Oh, and testing scenarios: (all involve Biggest N of Set form):

  • Scenario 1: Skipping adding a group
    • Start new form
    • Do not add group
    • Swipe back
    • Result: prompt to add new group
  • Scenario 2: Swiping back after adding first group
    • Start new form
    • Add new group
    • Swipe back
    • Result: nothing happening, same group in the view
  • Scenario 3: Removing when multiple groups
    • Start new form
    • Add new group
    • Add new group
    • Swipe back
    • Attempt to remove group
    • If user cancels, nothing happens and group is brought back into view
    • If user confirms, next group is shown
  • Scenario 4: Removing when a single group
    • Start new form
    • Add new group
    • Remove it
    • Result: prompt to add new group is shown

// The more testing scenarios I write, the more succinct they become :)

@lognaturel
Copy link
Member

Love those test scenarios!! This is very nice, I do agree that it's better than it was and cool that it fixes #276. @yanokwa?

@lognaturel
Copy link
Member

@max2me It would be even better if the delete confirmation could be over the group you're about to delete rather than the blank screen. It's easier to confirm deletion when you can see exactly what you're about to delete!

@max2me
Copy link
Contributor Author

max2me commented Dec 11, 2016

@lognaturel @yanokwa Please take a look. I tested removing first, middle and last group when 3 groups have been added and it worked well each time.

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.

Option to remove instructions screen
3 participants