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

fix(O3-2491): Improved Time Rendering Logic Implementation #171

Closed

Conversation

senthil-athiban
Copy link

Requirements

  • This PR has a title that briefly describes the work done including the ticket number. If there is a ticket, make sure your PR title includes a conventional commit label. See existing PR titles for inspiration.
  • My work conforms to the OpenMRS 3.0 Styleguide and design documentation.
  • My work includes tests or is validated by existing tests.

Summary

Currently the form engine supports date and datetime rendering only.

Within the inbuiltControls.ts all the controls are listed and added time seperately

Both date and datetime rendering use OHRIDate (Component Link). So, I decided to add the time logic inside of the component.

Screenshots

https://www.loom.com/share/ac4128e2ea9745798baea042de665866?sid=ba1308d5-2b6f-40d5-bec1-6c3d6866bc6e

Related Issue

https://openmrs.atlassian.net/browse/O3-2491

Other

While Fetching, we have to ensure that time is also separate. questionTypes.

@senthil-athiban
Copy link
Author

Hey @denniskigen, changes are made based on request, time component is rendered on it's own, irrespective of the date component.
Link: https://openmrs.atlassian.net/browse/O3-2491?focusedCommentId=139201

@senthil-athiban senthil-athiban changed the title fix(O3-249): Implmented separate time rendering logic fix(O3-2491): Implmented separate time rendering logic Mar 1, 2024
@samuelmale
Copy link
Member

@senthil-k8s did you see the build failure? Do you still have the bandwidth to push this?

@senthil-athiban
Copy link
Author

I haven't seen this; I'll look into it. Thanks @samuelmale for noticing

@samuelmale
Copy link
Member

@senthil-k8s please note that we need to proactively gear this work up because most of the form-engine tasks under this epic are candidates of the upcoming hackathon and the goal is to have these low hanging fruits out of the way prior to the hackathon. Same case applies other tasks like this. If you think you don't have enough suspense or not sure on how to address the issues, kindly feel free to pickup some other work to create room for the dedicated workforce.

Thanks for your amazing contributions!

@senthil-athiban senthil-athiban changed the title fix(O3-2491): Implmented separate time rendering logic fix(O3-2491): Impemented separate time rendering logic Apr 11, 2024
@senthil-athiban senthil-athiban changed the title fix(O3-2491): Impemented separate time rendering logic fix(O3-2491): Improved Time Rendering Logic Implementation Apr 11, 2024
@senthil-athiban
Copy link
Author

@samuelmale, build error resolved, as well as in other cases. Logic is written in the same date component, which ensures that time and date components rendered separately. As well, the logic works for both rendering the datetime simultaneously. else time needs to be in a separate component?

@samuelmale
Copy link
Member

From the attached GIF, this seems to capture datetime and not strictly time

@senthil-athiban
Copy link
Author

senthil-athiban commented Apr 12, 2024

@samuelmale
Copy link
Member

samuelmale commented Apr 22, 2024

Closing this because of the lack of a real world usecase

@samuelmale samuelmale closed this Apr 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants