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

Cleanup and moved Date and Time pickers to material #1312

Merged
merged 10 commits into from
Apr 29, 2022

Conversation

aditya-07
Copy link
Collaborator

@aditya-07 aditya-07 commented Apr 19, 2022

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

Fixes #[issue number]

Description

  1. Refactored code to remove dead code from QuestionnaireItemDatePickerViewHolderFactory.
  2. Moved Date and Time Pickers to Material design in QuestionnaireItemDateTimePickerViewHolderFactory and cleaned up code as well.

Alternative(s) considered
Have you considered any alternatives? And if so, why have you chosen the approach in this PR?

Type
Choose one: Code health

Screenshots (if applicable)

Date-Time-Picker-Material.mov

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

@aditya-07 aditya-07 added the type:bug Something isn't working label Apr 19, 2022
@aditya-07 aditya-07 added type:code health Data capture and removed type:bug Something isn't working labels Apr 19, 2022
@codecov
Copy link

codecov bot commented Apr 19, 2022

Codecov Report

Merging #1312 (78c9b3c) into master (4a61af5) will increase coverage by 0.24%.
The diff coverage is 19.29%.

@@             Coverage Diff              @@
##             master    #1312      +/-   ##
============================================
+ Coverage     83.72%   83.96%   +0.24%     
+ Complexity      690      687       -3     
============================================
  Files           148      148              
  Lines         10708    10694      -14     
  Branches        836      846      +10     
============================================
+ Hits           8965     8979      +14     
+ Misses         1340     1310      -30     
- Partials        403      405       +2     
Impacted Files Coverage Δ
...ws/QuestionnaireItemDatePickerViewHolderFactory.kt 32.35% <3.44%> (+1.68%) ⬆️
...uestionnaireItemDateTimePickerViewHolderFactory.kt 22.53% <14.66%> (+4.35%) ⬆️
...d/fhir/datacapture/utilities/MoreLocalDateTimes.kt 100.00% <100.00%> (ø)
...droid/fhir/datacapture/utilities/MoreLocalDates.kt 85.71% <100.00%> (ø)
...droid/fhir/datacapture/utilities/MoreLocalTimes.kt 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4a61af5...78c9b3c. Read the comment docs.

@jingtang10
Copy link
Collaborator

@santosh-pingle can you please leave a review

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.

LGTM
Code owner please review it, thanks!

@santosh-pingle santosh-pingle self-requested a review April 22, 2022 06:09
Comment on lines +29 to +30
val date = Date.from(atZone(ZoneId.systemDefault()).toInstant())
return DateFormat.getTimeFormat(context).format(date)
Copy link
Collaborator

Choose a reason for hiding this comment

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

this should just call the function in MoreLocalTimes.kt?

@aditya-07 aditya-07 enabled auto-merge (squash) April 29, 2022 11:27
@aditya-07 aditya-07 merged commit 6d6a10b into master Apr 29, 2022
@aditya-07 aditya-07 deleted the ak/datetime-picker-to-material branch April 29, 2022 13:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

3 participants