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

Show fields when copying from previous log #6659

Closed
wants to merge 9 commits into from

Conversation

Omkar76
Copy link
Contributor

@Omkar76 Omkar76 commented Nov 17, 2023

WHAT

🤖[deprecated] Generated by Copilot at bf5fef5

Refactor daily rounds form component to clone previous log values more easily.

Proposed Changes

  • Fixes Copy values for Log Updates #6602?
  • Made fields visible regardless of whether "copy values from previous log?" is checked or not.
  • Keeping the checkbox unchecked by default as requested in issue.
  • When checkbox is clicked values from previous log get copied to form field. when it's unchecked that resets the form to initial state.

@coronasafe/care-fe-code-reviewers @coronasafe/code-reviewers

Merge Checklist

  • Add specs that demonstrate bug / test a new feature.
  • Update product documentation.
  • Ensure that UI text is kept in I18n files.
  • Prep screenshot or demo video for changelog entry, and attach it to issue.
  • Request for Peer Reviews
  • Completion of QA

HOW

🤖[deprecated] Generated by Copilot at bf5fef5

  • Simplify the initial state of the form and errors by using the initForm and initError constants directly (link)
  • Replace the hasPreviousLog state variable with a previousLog state variable that stores the data of the previous log, if any (link, link)
  • Set the clone_last form field to false by default, instead of using the count of the previous logs as a condition (link)
  • Add a new function clonePreviousLog that copies the values from the previousLog state variable to the form state variable, except for some fields that should not be cloned (link)
  • Modify the rendering logic of the form fields based on the clone_last form field value (link)

@Omkar76 Omkar76 requested a review from a team November 17, 2023 06:07
@Omkar76 Omkar76 requested a review from a team as a code owner November 17, 2023 06:07
Copy link

vercel bot commented Nov 17, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
care-storybook ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 6, 2023 7:10pm

Copy link

netlify bot commented Nov 17, 2023

Deploy Preview for care-egov-staging ready!

Name Link
🔨 Latest commit 3adfbce
🔍 Latest deploy log https://app.netlify.com/sites/care-egov-staging/deploys/6570c6e7c15120000837f4f9
😎 Deploy Preview https://deploy-preview-6659--care-egov-staging.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@nihal467
Copy link
Member

nihal467 commented Nov 21, 2023

@Omkar76

image

  • When you click to copy the previous value, check the rhythm field, the unknown option is not getting auto-filled
  • Once you click to copy the previous value, and the data are reflected, upon editing, first need to save it and click submit, ( including this two time submission was intentional behaviour or a bug ? )

@nihal467 nihal467 added question Further information is requested test failed and removed needs testing labels Nov 21, 2023
@nihal467
Copy link
Member

nihal467 commented Nov 21, 2023

@Omkar76 Revert all change back, only make a the below change :

when a user click on the check box, show a description of the expected behavior

@nihal467 nihal467 added changes required and removed question Further information is requested test failed labels Nov 21, 2023
@github-actions github-actions bot added the merge conflict pull requests with merge conflict label Nov 22, 2023
Copy link

👋 Hi, @Omkar76,
Conflicts have been detected against the base branch. Please rebase your branch against the base branch.


This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there.

@nihal467
Copy link
Member

@Omkar76
image

once i select it, i can unselect the option, by clicking on the button

@Omkar76
Copy link
Contributor Author

Omkar76 commented Dec 12, 2023

@Nihal46

once i select it, i can unselect the option, by clicking on the button

Do you mean the button should toggle the clone_last variable?

In that case I think it would have been better to keep it as checkbox, as done in my previous commit. (ad57c3c)

Button is going to be confusing for users. The button has text "Copy values from previous log". If we use this to toggle, we need to change the button text also. What should the text be when button is used to toggle clone_last to false?

PS: Still thinking checkbox is correct control here. In case we still want to move forward with button - let me know what button text should be in both states. Thanks.

@rithviknishad
Copy link
Member

rithviknishad commented Dec 12, 2023

PS: Still thinking checkbox is correct control here.

I agree that button is confusing if shown this way.

Maybe we can show two buttons instead of 1 "Save" button that does:

  1. Creates a fresh log update
  2. Copies previous values and creates a log update

@github-actions github-actions bot added the merge conflict pull requests with merge conflict label Dec 15, 2023
Copy link

👋 Hi, @Omkar76,
Conflicts have been detected against the base branch. Please rebase your branch against the base branch.


This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there.

Copy link

Hi, This pr has been automatically marked as stale because it has not had any recent activity. It will be automatically closed if no further activity occurs for 7 more days. Thank you for your contributions.

@github-actions github-actions bot added the stale label Dec 23, 2023
Copy link

Hi, @gigincg, @nihal467, @khavinshankar, @mathew-alex, This pr has been automatically closed because it has not had any recent activity. Thank you for your contributions. Feel free to repopen the pr.

@github-actions github-actions bot closed this Dec 30, 2023
@nihal467 nihal467 reopened this Jan 16, 2024
@nihal467
Copy link
Member

nihal467 commented Jan 16, 2024

Inital Screen

  • Move the button adjacent to the save button
  • Rename the button from save to Continue
  • Rename the button " copied from the previous log " to " Copy Values from Previous Log and Save" ( Similar as the above screenshot )
  • Initially, users only will see the above screenshot part of the form, once they land on the log update page.

image

  • Rather than " Field values will be copied from the previous log update " text box, replace it with the below notification:

  • Values Copied from previous log update and Normal Log Update created successfully:- Notification For Normal Log Update

  • Values Copied from previous log update and Critical Log Update created successfully:- Notification For Critical Log Update

  • the notification should be shown in the right side corner by replacing the Consultation update created notification successfully

@nihal467
Copy link
Member

@Omkar76 you can work on the comment

@Omkar76
Copy link
Contributor Author

Omkar76 commented Jan 16, 2024

@nihal467

Values Copied from previous log update and Normal Log Update created successfully:

I don't think a change is required to notification that is shown in top right corner. This message implies that after clicking copy button the log update was created immediately without any further user interaction - without user making any more changes to form values.

This seems to deviate from original purpose of this PR. We wanted users to be able to see what values are being copied from previous log update and make changes if desired.

@nihal467
Copy link
Member

image

@Omkar76 the notification needs to be modified due to the following releases :

  • we are removing the text box, that mentions "All values from the previous log update" when we click the button "Copy value from the previous log", so as a result, we need to mention in the notification that, the values are copied from the previous log
  • The current notification says "Consultation update created" but we need to mention whether it's a normal log update or critical care log update

@github-actions github-actions bot removed the stale label Jan 17, 2024
Copy link

Hi, This pr has been automatically marked as stale because it has not had any recent activity. It will be automatically closed if no further activity occurs for 7 more days. Thank you for your contributions.

@github-actions github-actions bot added the stale label Jan 24, 2024
Copy link

github-actions bot commented Feb 1, 2024

Hi, @gigincg, @nihal467, @khavinshankar, @mathew-alex, This pr has been automatically closed because it has not had any recent activity. Thank you for your contributions. Feel free to repopen the pr.

@github-actions github-actions bot closed this Feb 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge conflict pull requests with merge conflict stale test failed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Copy values for Log Updates
4 participants