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

Handle form validation for open api form #11963

Merged
merged 7 commits into from
Jul 13, 2021
Merged

Conversation

arnav28
Copy link
Contributor

@arnav28 arnav28 commented Jun 29, 2021

  • Added required validator for all the default fields

- Added required validator for all the default fields
@arnav28 arnav28 added the ui label Jun 29, 2021
? set(this.validationMessages, name, '')
: set(this.validationMessages, name, this.model.validations.attrs[name].message);

// Set form button state
Copy link
Contributor

Choose a reason for hiding this comment

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

I like this!

@@ -106,6 +106,7 @@

.message-inline {
display: flex;
align-items: center;
Copy link
Contributor

Choose a reason for hiding this comment

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

is this going to be true for all message-inline? worth checking that this doesn't have consequences elsewhere.

Copy link
Contributor

@Monkeychip Monkeychip left a comment

Choose a reason for hiding this comment

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

This looks really good. A couple of comments, and the following requests:

  • Can you add a gif to the PR that shows the validations and how they work?
  • Can you add test coverage for this particular validation? You can look at my earlier PR that does this. See file here.
  • We are now required to add a changelog for every pr; it can just be something that expands off the previous validations changelogs, specifying the actual validation target here.

@Monkeychip Monkeychip added this to the 1.8 milestone Jul 1, 2021
@arnav28
Copy link
Contributor Author

arnav28 commented Jul 6, 2021

Screen.Recording.2021-07-06.at.11.28.45.AM.mov

@vercel vercel bot temporarily deployed to Preview – vault-storybook July 6, 2021 18:36 Inactive
@vercel vercel bot temporarily deployed to Preview – vault July 6, 2021 18:36 Inactive
- Handle read only inputs during edit mode
@vercel vercel bot temporarily deployed to Preview – vault July 13, 2021 21:30 Inactive
@arnav28 arnav28 merged commit 55eb96b into main Jul 13, 2021
@arnav28 arnav28 deleted the ui/api-forms-validation branch June 15, 2022 18:48
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.

3 participants