-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Repeat titles #1648
Repeat titles #1648
Conversation
Since we can build an appropriate form using xls (here is an example: |
👍👍👍 Looks great to me. I agree that looking at complex nested group/repeat structures is a good idea. @hooverlunch collect-debug-v1.11.0-34-g64bcd9156.apk.zip is a debug build for you to confirm that this is what you had in mind. |
@opendatakit-bot unlabel "needs testing" |
I played around with this and a form with nested repeats: One unfortunate thing to note is that to get the repeat position you now have to use The breadcrumb ends up looking something like @hooverlunch last chance for comments? |
Looking very nice! So exciting! A couple questions... Arrow ThingysI thought we were going to get rid of these and have tapping a repeat group name take you to a subscreen? Maybe I forgot. But they are super non-material-like and weird. Breadcrumb BugCurrently on the data entry screen I get: whilst on the jump screen I get Note the extra (0). I'm assuming that can be easily fixed? Breadcrumb StyleI think I can live with It seems to me that Especially if you look at two of them side by side:
vs.
It seems more natural for the Friends breadcrumb to stay unchanged since there is only one Friends group. Separately, it also it would be nice if the number were optional. Having the label available removes the need for it in some cases. But these are both things that could go into a separate issue if they're not easy to do right now, or if y'all disagree with me! Thank you all so much for your work on this! |
Thanks, @hooverlunch! The visual upgrade of this view is a separate initiative. @shobhitagarwal1612 still has a PR open at #1271. That one is more risky and has been on the back burner. Will try to block off some time for it soon but I'd like to get more Android-savvy eyes on it too. We can also file issues for some of the other things you have brought up. For this, I noticed one other issue. Let's say a form developer has included groups within repeats to give their form some structure as in nested-repeats-bad.xml.txt. Now on the jump screen instead of
I'll get
Form developers are most likely to include groups like this when the form is long so I think the number of the repeat is important to include. That would give us
or with the previous form:
Does that work for you @grzesiek2010 and @hooverlunch? |
I've filed #1670 since it's an issue even without this change. I'll file the breadcrumb style issues after this is merged since they depend on this change. |
Yes I think it's fine with me @lognaturel. We can also see how users react. I don't think any of these things constitute brokenness (except for the (0) thing), just style. |
@lognaturel do you want those numbers in brackets like in your example? I'm asking because in v1.11.1 we display those numbers without brackets so should we always use brackets? |
@lognaturel @hooverlunch |
I'd tend to do the parentheses so it's consistent with the breadcrumb for now but I don't feel strongly about it. @hooverlunch? |
I agree, parens at end seems more consistent. |
@lognaturel @hooverlunch ok thanks, I'll implement it. |
64bcd91
to
e49f0fd
Compare
@lognaturel it's ready. |
Closes #183
What has been done to verify that this works as intended?
I've tested attached forms.
Navigation between questions.
Why is this the best possible solution? Were any other approaches considered?
The solution has been proposed here getodk/xforms-spec#141
Are there any risks to merging this code? If so, what are they?
Yes. I think we should test also some strange scenarios like 3 or 4 nested groups etc.
Do we need any specific form for testing your changes? If so, please attach one.
repeatGroupNew.xml.txt
repeatGroupNewAOOP.xml.txt
repeatGroupOriginal.xml.txt
repeatGroupOriginalAOOP.xml.txt