Skip to content

Conversation

LZRS
Copy link
Collaborator

@LZRS LZRS commented Aug 25, 2025

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

  • I have read and acknowledged the Code of conduct.
  • I have read the Contributing page.
  • I have signed the Google Individual CLA, or I am covered by my company's Corporate CLA.
  • I have discussed my proposed solution with code owners in the linked issue(s) and we have agreed upon the general approach.
  • I have run ./gradlew spotlessApply and ./gradlew spotlessCheck to check my code follows the style guide of this project.
  • I have run ./gradlew check and ./gradlew connectedCheck to test my changes locally.
  • I have built and run the demo app(s) to verify my change fixes the issue and/or does not break the demo app(s).

@LZRS LZRS force-pushed the repeat_group_bottom_add_button branch 3 times, most recently from 13bc15a to 031d2f5 Compare August 26, 2025 13:50
@LZRS
Copy link
Collaborator Author

LZRS commented Aug 26, 2025

Screen_recording.mp4

@LZRS LZRS marked this pull request as ready for review August 26, 2025 13:55
@LZRS LZRS requested a review from a team as a code owner August 26, 2025 13:55
@LZRS LZRS requested a review from MJ1998 August 26, 2025 13:55
@LZRS LZRS force-pushed the repeat_group_bottom_add_button branch from 031d2f5 to 57bdd88 Compare August 26, 2025 17:25
Copy link
Collaborator

@jingtang10 jingtang10 left a 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.

Comment on lines +1028 to +1030
if (questionnaireItem.isRepeatedGroup) {
add(QuestionnaireAdapterItem.RepeatedGroupAddButton(question.item))
}
Copy link
Collaborator

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.

Comment on lines +390 to +392
oldItem.item.hasTheSameItem(newItem.item) &&
oldItem.item.hasTheSameResponse(newItem.item) &&
oldItem.item.hasTheSameValidationResult(newItem.item)
Copy link
Collaborator

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()
Copy link
Collaborator

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()
Copy link
Collaborator

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()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here

@github-project-automation github-project-automation bot moved this from New to PR under Review in Android FHIR SDK Sep 25, 2025
@LZRS LZRS force-pushed the repeat_group_bottom_add_button branch from ae82ad8 to e8c4aff Compare September 29, 2025 09:43
@LZRS LZRS force-pushed the repeat_group_bottom_add_button branch from e8c4aff to 23ac82c Compare September 30, 2025 08:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: PR under Review
Development

Successfully merging this pull request may close these issues.

Move the add button for repeated groups from top to bottom
2 participants