Dialog valadation required fields

[kuthedk]

Checklist before requesting a review

• I have performed a self-review of my code.
• My code follows the style guidelines enforced by static analysis tools.
• If it is a core feature, I have added thorough tests.
• My changes generate no new warnings.
• New and existing unit tests pass locally with my changes.

Pull Request Details

Wrike Task: Supporting Evidence - Inform what fields are required/not required while attaching evidence

Changes made:

• Input dialog is configurable to set fields as required
• when the input is in focus and not filled out, it is highlighted in red
• it highlights all other unfilled required fields.
• the confirmation button is disabled unless all required fields are filled out
• Added Unit Tests
• Added System Tests

To Review:

• Static Analysis by Reviewer

• Observe changes made

  1. Deploy Rosalution

  # From the root of rosalution
  docker-compose down
  docker system prune -a --volumes
  docker-compose up --build -d

  2. Go to: http://local.rosalution.cgds/rosalution/
  3. Login with a user
  • username: developer
  4. click on a case
  5. navigate to the supporting evidence section and add supporting evidence.
     
  6. verify that it will not let you attempt to save or add the evidence until the required fields are filled out
  7. confirm/save/add the evidence once all required fields are filled out
     
     

• All Github Actions checks have passed.

[SeriousHorncat]

In the description of the ticket; the described behavior is

"When the 'save' button is clicked, the dialog should not allow the dialog to close and required fields should go red. "

This is intended to mean that the dialog needs to update the input's colors to visually communicate that the required empty inputs need to be filled out. That each of the required inputs, turns red which needs the missing required content.

This behavior should not be present until the dialog is clicked to 'submit' or 'save' or 'add', etc.

The screenshot also includes a "*required" red text under each input field that needs adjustment, in order as a written reminder that text needs to be entered.

The default styling for inputting content into required fields should NOT turn red until the user attempts to submit the dialog without the required information. And only the required fields are to turn red, otherwise, they are grey or purple.

Also, for when the input field is in focus (have the cursor actively inside it and typing) it should have a double border. When the field does not have focus, it should have a single border. These are only red when the user attempts to accept a dialog with empty inputs.

Revise this work to include this functionality and it will be to ready review again then.

[SeriousHorncat]

Good job with the update to validate the input fields after the "save" or "add" button There are two things I am observing right now that will need to be fixed:

1. When the form has only one input field for information; and it needs to validate that the information is present; it shows the "*required" text twice on the form. It should only display once.

2. When there are two fields; when validating the inputs; if the field value is valid (like in the picture below), it should not turn red to indicate that it is required. Otherwise, it might accidentally confuse the user to believe that the information in the field is somehow incorrect. In the below case, only the 2nd field should turn red.

[SeriousHorncat]

Observed field for single input.

->

Success.

---

For dual input:
Successful in showing focus with the extra lighter red border.

Filled in one field and tried again, successfully showing the empty field still red.

Both required fields are now entered.

[SeriousHorncat]

Just one change request for now in the meantime. There are several things I want to think about some more when I am back working later today or on Tuesday when we return to work before I give some feedback or move forward to approve.

[SeriousHorncat]

Am leaving a question for ya on the validation. We maintain the state of the dialog inside of 'inputDialog.js'. We can probably avoid the complicated nature of maintaining the watches in this case if the validation is within there.

[SeriousHorncat]

Why did you decide to do the validation within the child components instead of the inputDialog.js where the state of the dialog is managed? When the ‘confirmation(userInput)’ https://github.com/uab-cgds-worthey/rosalution/pull/90/files#diff-f5058686e21862d83e368c13b0f4ccf587074f2fd093876b28427a714b08db17R118 is called, the validation can be done before the ‘close(userInput)’ call, and if it fails, update the state of the dialog to be invalid. The changed state of the 'dialog' in data that has the state as props would cause the re-rerender to update to show the invalid state.

[kuthedk]

I didn't think about that approach when working on this feature. I was more inclined to use the Watches the way I did because of the Slack conversation where we agreed that it was a valid path forward, including the article that was linked in that message as well Vue form input validation using watchers - LogRocket Blog.

As for why in the child and not the parent component, I was taking the approach of handling validation in the child component to provide better encapsulation, each child component is responsible for its own state and behavior, which can lead to a cleaner, more maintainable code base.

When we are validating in each child component, we ensure that each part of our input dialog independently verifies its own "correctness" before notifying the parent component that they are valid. This follows the Single Responsibility Principle, meaning each component or module is responsible for a single part of the functionality, and that responsibility should be entirely encapsulated by the component.

This pattern also scales well if we add more input types in the future, where each new input type would have its own specific set of validation rules.

That being said, if we're not expecting to add additional input types, we could move the validation logic to the parent component as a refactor from the current implementation.

[SeriousHorncat]

I appreciate you trying to stick to using Watchers for this solution. I didn't anticipate it needed to go so far as to needing to defer the callback to be done after the next DOM update cycle. https://github.com/uab-cgds-worthey/rosalution/pull/90/files#diff-b5c0bc889b479c474f59d646d55e2afe0501fcc38086e574bc130e7b62f2176cR76.

After taking time to look at it in more detail; let's consider this exploratory work and document it. This code would be good to be contained in Rosalution's "/docs/prototypes/" directory with its own subproject. I will follow up with a change request to document it.

[SeriousHorncat]

Since using watchers for this solution turned out more complicated than anticipated, I would like to document this work within Rosalutions './docs/prototypes/' as its own subproject we can reference in the future and revisit adding this feature to Rosalution at a later time. Let's avoid copying all of Rosalution into the prototype.

So for this PR, Create a new Vue3JS project that includes the inputDialog.js, and the dialog components that are used to present the inputDialog.js' state along with the corresponding unit and system tests. The code created here can be moved to the prototype and rosalution's dialog can revert to its original state.

[SeriousHorncat]

I appreciate you trying to stick to using Watchers for this solution. I didn't anticipate it needed to go so far as to needing to defer the callback to be done after the next DOM update cycle. https://github.com/uab-cgds-worthey/rosalution/pull/90/files#diff-b5c0bc889b479c474f59d646d55e2afe0501fcc38086e574bc130e7b62f2176cR76.

After taking time to look at it in more detail; let's consider this exploratory work and document it. This code would be good to be contained in Rosalution's "/docs/prototypes/" directory with its own subproject. I will follow up with a change request to document it.

[SeriousHorncat]

The work is close, but three things:

• Reverting the files to match what is in 'main' so it doesn't add extra changes to the commit history
• For the prototype project, documentation on how to run and view the inputDialog and revising the PR for the steps to verify that the inputDialog prototype runs and works so that we can see the code in action. This way, when we return to it, we can work and iterate over the prototype before bringing it into rosalution.
• Migrate the unit and system tests with the prototype. You invested the efforts to verify and test that the code does as intended and these will be useful tools to rely on if we change things up.

[SeriousHorncat]

To reduce the number of changes listed in this commit, let's revert these files to match the ones in the main branch that have no functional changes within them.