-
Notifications
You must be signed in to change notification settings - Fork 295
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
Add the next button navigation view to a default long scroll #2134
Add the next button navigation view to a default long scroll #2134
Conversation
1c3d155
to
ed74d4d
Compare
5d423ec
to
bc9abd3
Compare
bc9abd3
to
0ccb597
Compare
…ttom-to-enable-next
…ttom-to-enable-next
datacapture/src/main/java/com/google/android/fhir/datacapture/views/NavigationViewHolder.kt
Outdated
Show resolved
Hide resolved
datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireViewModel.kt
Outdated
Show resolved
Hide resolved
datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireViewModel.kt
Outdated
Show resolved
Hide resolved
5ed31e7
to
3e31e93
Compare
…ttom-to-enable-next
3e31e93
to
f695551
Compare
f695551
to
fbb6ba3
Compare
@santosh-pingle Since you are looking into this PR, its ready for review. |
@LZRS thanks for this PR - it's a very well written PR! I want to highlight though in @shelaghm's comment here #1764 (comment) she said Your PR changes this right? is there an option to keep the current behaviour? if so can you add a video? |
Yeah, the PR changes that. Within the PR, I tried to set it as configurable in the I will attach video for both cases |
…ttom-to-enable-next
looks good - can you please add tests @LZRS ? |
Cool, I'll add the tests and ping once done |
datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireAdapterItem.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @LZRS again for this. I've updated this PR and done a couple of improvements:
- removed the unnecessary navigation recycler view. in the case where the navigation controls are floating at the bottom of the screen, they can just be a simple reused layout instead of using a recycler view which is costly.
- removed the disabled state for pagination controls which I don't think is ever used actually.
I also want to simplify the QuestionnaireNavigationViewUIState class altogether as I find it to be overly complicated.
I'll try to merge this PR asap.
Thanks
Okay, thanks @jingtang10 |
questionnaireEditRecyclerView.visibility = View.VISIBLE | ||
reviewModeEditButton.visibility = View.GONE | ||
|
||
// Set bottom navigation | ||
bottomNavContainerFrame.visibility = View.VISIBLE | ||
NavigationViewHolder(bottomNavContainerFrame) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
state.bottomNavItems.single()
crashes when QuestionnaireFragment.Builder.setShowNavigationInDefaultLongScroll
is set to true since it's empty
…2134) * Configurable bottom navigation to be part of page scroll * Shared layout for bottom navigation items, for scroll and sticky * Use separate recyclerview for bottom navigation items * Fix build errors * Remove unnecessary disabled state * Delete the unnecessary navigation recycler view * Simplify unnecessary .apply --------- Co-authored-by: Francis Odhiambo <[email protected]> Co-authored-by: Jing Tang <[email protected]>
IMPORTANT: All PRs must be linked to an issue (except for extremely trivial and straightforward changes).
Fixes #1764
Description
Adds support for having the bottom navigation buttons as part of the Questionnaire recyclerview, that would ensure the user has viewed all the field items before clicking next
Alternative(s) considered
Have you considered any alternatives? And if so, why have you chosen the approach in this PR?
Detecting scroll to the bottom of the Recyclerview to toggle visibility of the bottom buttons - was not smooth and the bottom buttons were not showing in a predictable manner
Type
Choose one: (Bug fix | Feature | Documentation | Testing | Code health | Builds | Releases | Other)
Screenshots (if applicable)
Checklist
./gradlew spotlessApply
and./gradlew spotlessCheck
to check my code follows the style guide of this project../gradlew check
and./gradlew connectedCheck
to test my changes locally.