Fix select_one/multiple list height#7311
Conversation
845dd55 to
ee059d3
Compare
991a93c to
644d521
Compare
|
Im not sure if it is intended change but there are 2 scrolls on select forms with images e.g. SelectIcons form from user 1291 in this project https://test.getodk.cloud/projects/425/app-users |
Yes. Before, we only limited the height of the list to 90% of the screen when there were more than 40 items - that's how we determined whether the list takes up too much space and should be limited (in that case, there were two scrollbars in previous versions as well). However, it wasn't perfect, as this example shows: you can have only a few items but with big images, which ends up taking a lot of space as well. |
|
Tested with success Verified on phone with android 10, 16 and tablet with android 16 Verified cases:
|
@grzesiek2010 could we hide one of the scroll bars? Is there actually two scrollable surfaces in play? |
|
Tested with success Verified on phone with android 14 |
Yes, there are two scrollable surfaces. When we limit the height of the |

Closes #7310
Why is this the best possible solution? Were any other approaches considered?
The problem was caused by the fact that we always set the height of the list of choices (if it was a long list), regardless of whether the choices were displayed directly in questions or in a dialog. We don't need to do that for the dialog case.
The smallest solution would be to pass a boolean flag and only set the height when it is
true, keeping the old behavior. However, this is a good opportunity to improve the approach slightly, which is what I did by passing the height directly. This has two advantages:RecyclerView.MAX_ITEMS_WITHOUT_SCREEN_BOUND) that was previously used to determine whether limiting the height was necessary or not: https://github.com/getodk/collect/pull/7311/changes#diff-345fc063e7a817a7c0a68f814d7caf8ddafa8709c69474945fad7ba9f00a33b3L29How 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 reworks the way we determine the height of the list of choices displayed directly in questions or in a dialog when the
minimalappearance is used.In the dialog case, the list always takes the full screen height. However, for lists displayed directly in questions, it uses approximately 90% of the screen height. This is because answers are displayed inside a scroll view that may also contain other questions (
field-list). To make this work correctly, we set a maximum height for the list.Please test for regressions, especially that choices displayed in field-lists with other questions below them are rendered correctly in both very long list and very short list cases.
Do we need any specific form for testing your changes? If so, please attach one.
The form is linked to the issue.
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 connectedAndroidTest(or./gradlew testLab) and confirmed all checks still passDateFormatsTest