-
Notifications
You must be signed in to change notification settings - Fork 314
Move the add button for repeated groups to bottom #2868
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
base: master
Are you sure you want to change the base?
Conversation
13bc15a
to
031d2f5
Compare
Screen_recording.mp4 |
031d2f5
to
57bdd88
Compare
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 - some comments but nothing on the functionalities this PR introduces. for that reason i'm going to approve and please feel free to merge once all comments are addressed.
...re/src/main/java/com/google/android/fhir/datacapture/views/RepeatedGroupAddItemViewHolder.kt
Show resolved
Hide resolved
if (questionnaireItem.isRepeatedGroup) { | ||
add(QuestionnaireAdapterItem.RepeatedGroupAddButton(question.item)) | ||
} |
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.
can you put this inside the buildList
?
also udpate the comment right before the buildList
function.
...src/androidTest/java/com/google/android/fhir/datacapture/test/QuestionnaireUiEspressoTest.kt
Outdated
Show resolved
Hide resolved
...src/androidTest/java/com/google/android/fhir/datacapture/test/QuestionnaireUiEspressoTest.kt
Outdated
Show resolved
Hide resolved
oldItem.item.hasTheSameItem(newItem.item) && | ||
oldItem.item.hasTheSameResponse(newItem.item) && | ||
oldItem.item.hasTheSameValidationResult(newItem.item) |
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.
have you tested having 2 repeated groups?
.inflate(R.layout.pagination_navigation_view, parent, false), | ||
) | ||
QuestionnaireEditAdapter.ViewType.Type.REPEATED_GROUP_HEADER -> TODO() | ||
QuestionnaireEditAdapter.ViewType.Type.REPEATED_GROUP_ADD_BUTTON -> TODO() |
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.
wouldn't this just crash? and is it crashing now for the line above REPEATED_GROUP_HEADER?
unless we filter out these items beforehand?
if we do filter them out correctly beforehand, then here it should throw an error. (illegal state exception).
holder.bind(item.questionnaireNavigationUIState) | ||
} | ||
is QuestionnaireAdapterItem.RepeatedGroupHeader -> TODO() | ||
is QuestionnaireAdapterItem.RepeatedGroupAddButton -> TODO() |
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.
same here
subtype = 0xFFFFFF | ||
} | ||
is QuestionnaireAdapterItem.RepeatedGroupHeader -> TODO() | ||
is QuestionnaireAdapterItem.RepeatedGroupAddButton -> TODO() |
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.
same here
...src/test/java/com/google/android/fhir/datacapture/views/RepeatsGroupAddItemViewHolderTest.kt
Outdated
Show resolved
Hide resolved
ae82ad8
to
e8c4aff
Compare
e8c4aff
to
23ac82c
Compare
IMPORTANT: All PRs must be linked to an issue (except for extremely trivial and straightforward changes).
Fixes #2844
Description
Sets the button to add repeated items at the bottom of the group. Clicking the add button prepends the repeated group item to come before the add button
Alternative(s) considered
Have you considered any alternatives? And if so, why have you chosen the approach in this PR?
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.