Skip to content
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

Clearing the old state of date and date-time ViewHolders #1581

Merged
merged 5 commits into from
Sep 12, 2022

Conversation

aditya-07
Copy link
Collaborator

@aditya-07 aditya-07 commented Sep 5, 2022

IMPORTANT: All PRs must be linked to an issue (except for extremely trivial and straightforward changes).

Fixes #1580

Description

Cause:
The date and date time view holders only set text to the TextInput if it was empty before. This logic is required when answer update causes an update in the adapter and subsequent bindView on the ViewHolder.
But, this causes issue when the ViewHolder is recycled for a new QuestionnaireItem and the ViewHolder retains the old text (date).

Fix:
Cleanup up the the ViewHolder state when bind is called for a different QuestionnaireItem.
UPDATE
As per discussion with @jingtang10 , instead of cleaning state using a hack, isTextUpdateRequired is added to the particular QuestionnaireItemViewHolderDelegates to check if textview has to be update or not based on current text and the QuestionnaireResponse.QuestionnaireResponseItemAnswerComponent .

Alternative(s) considered

  1. RecyclerView.Adapter#onViewRecycled could be used to forward a callback to the individual QuestionnaireItemViewHolderDelegate to do cleanup.
  2. Have a way for QuestionnaireItemViewHolderDelegate to know that the bind is called as an update for the same item user is updating or is a regular bind.

Both of these require refactoring in all the QuestionnaireItemViewHolderDelegate and can be taken as a separate PR if required.

Type
Choose one: Bug fix
Screenshots (if applicable)
Date Picker
Date Time Picker

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).

Copy link
Collaborator

@santosh-pingle santosh-pingle left a comment

Choose a reason for hiding this comment

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

If possible, can you please add one positive and negative test case in both pickers, otherwise it looks ok to me. thanks!

@jingtang10 jingtang10 assigned aditya-07 and unassigned jingtang10 Sep 12, 2022
@aditya-07 aditya-07 assigned jingtang10 and unassigned aditya-07 Sep 12, 2022
@aditya-07 aditya-07 enabled auto-merge (squash) September 12, 2022 10:01
@aditya-07 aditya-07 merged commit eb234be into google:master Sep 12, 2022
@aditya-07 aditya-07 deleted the ak/bugfix-1580-date-picker branch April 19, 2023 08:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Something isn't working
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Input of one datepicker view get updated to another date picker view in layout.
3 participants