Select appearances improvements #3079
Conversation
grzesiek2010
force-pushed
the
selectAppearancesImprovements
branch
2 times, most recently
from
8ee3033
to
210fa04
Compare
| @@ -122,6 +122,12 @@ public static int getNumberOfColumns(FormEntryPrompt formEntryPrompt, Context co | |||
| numColumns = Integer.parseInt(substringFromNumColumns.substring(0, substringFromNumColumns.contains(" ") | |||
| ? substringFromNumColumns.indexOf(' ') | |||
| : substringFromNumColumns.length())); | |||
|
|
|||
| if (numColumns < 1) { | |||
There was a problem hiding this comment.
Although we're only officially supporting up to 10 columns, I don't think this is necessary. I think it would be more confusing than it's worth. I think we should unofficially continue supporting arbitrary values of N.
There was a problem hiding this comment.
Ok I thought it over and I agree it's better to allow using > 10 columns because in some cases it could be useful (if it's a big screen and options are pretty short like A, B, C, etc.) but then I don't understand why you want to document that we support up to 10 columns.
There was a problem hiding this comment.
For spec-level purity. You can see the discussion at XLSForm/xlsform.github.io#125 (comment).
|
Agree this doesn't need QA. Please see what you think of my suggestion and then we can merge. |
grzesiek2010
force-pushed
the
selectAppearancesImprovements
branch
from
210fa04
to
0d1e45f
Compare
What has been done to verify that this works as intended?
I reviewed my changes visually and added automated tests to confirm we can use 1 to 10 columns.
Why is this the best possible solution? Were any other approaches considered?
Changes agreed on the forum: https://forum.opendatakit.org/t/spec-proposal-harmonize-compact-compact-n-horizontal-horizontal-compact-appearances/15565/32?u=grzesiek2010
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?
The pr contains small changes so I don't think testing is required, a code review should 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:
./gradlew checkAlland confirmed all checks still pass OR confirm CircleCI build passes and run./gradlew connectedDebugAndroidTestlocally.