Skip to content

PB-4564: Form validation#819

Merged
ltshb merged 8 commits intodevelopfrom
feat-PB-454-form-validation
May 13, 2024
Merged

PB-4564: Form validation#819
ltshb merged 8 commits intodevelopfrom
feat-PB-454-form-validation

Conversation

@ltshb
Copy link
Contributor

@ltshb ltshb commented May 2, 2024

Now forms are only validated after user action. This provide a better user
feedback especially when the form has several fields, the form is only validated
when the user click on the action button.

This has been done for all forms:

  • report a problem
  • import file tool
  • drawing media link
    Test link

@ltshb ltshb changed the title wip PB-4564: Form validation May 2, 2024
@cypress
Copy link

cypress bot commented May 2, 2024

Passing run #2076 ↗︎

0 203 20 0 Flakiness 0

Details:

PB-454: Remove useless css class and html div
Project: web-mapviewer Commit: 624e34acbe
Status: Passed Duration: 05:33 💡
Started: May 13, 2024 4:16 AM Ended: May 13, 2024 4:22 AM

Review all test suite changes for PR #819 ↗︎

@ltshb ltshb force-pushed the feat-PB-454-form-validation branch 5 times, most recently from 7826332 to 49235dc Compare May 6, 2024 09:51
@ltshb ltshb marked this pull request as ready for review May 6, 2024 09:52
@ltshb ltshb force-pushed the feat-PB-454-form-validation branch from 49235dc to 34157c1 Compare May 6, 2024 10:02
@ltshb ltshb requested a review from pakb May 6, 2024 10:02
class="mb-2"
:class="{ 'fw-bolder': required }"
:for="inputEmailId"
:data-cy="`${dataCyPrefix}-label`"
Copy link
Contributor

Choose a reason for hiding this comment

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

It might be better (more comprehensible) to add static/fixed data-cy="..." here, and then add another one whenever the component is used

<EmailInput data-cy="some-identifier" />

It will be applied to the div wrapping the label and input, and we can still access them in Cypress test with cy.get('[data-cy="some-identifier"] [data-cy="..."]')

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, hopefully it was easy with search and replace, will see if the test passes 🤞🏼

Comment on lines +28 to +75

export function useFieldValidation(
props,
model,
emits,
{
customValidate = () => {
return { valid: true, invalidMessage: '' }
},
requiredInvalidMessage = 'field_required',
} = {}
) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add a bit of documentation that describe what inputs are expected and what they do?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@ltshb ltshb force-pushed the feat-PB-454-form-validation branch 2 times, most recently from fc6fb85 to f11fba6 Compare May 7, 2024 12:35
@ltshb ltshb requested a review from pakb May 7, 2024 13:15
@ltshb ltshb force-pushed the feat-PB-454-form-validation branch from 9bb39c1 to 0b06c83 Compare May 7, 2024 13:15
ltshb added 7 commits May 7, 2024 20:08
They border are now identical to the input border.
Now forms are only validated after user action. This provide a better user
feedback especially when the form has several fields, the form is only validated
when the user click on the action button.
@ltshb ltshb force-pushed the feat-PB-454-form-validation branch from 866aeed to 23a66b1 Compare May 7, 2024 18:08
the need-validation class was useless therefore I removed it. Also removed a
useless html div.

Be consistent in the css class of all input fields.
@ltshb ltshb force-pushed the feat-PB-454-form-validation branch from 23a66b1 to 624e34a Compare May 13, 2024 04:13
@ltshb ltshb merged commit 0ae369b into develop May 13, 2024
@ltshb ltshb deleted the feat-PB-454-form-validation branch May 13, 2024 04:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants