Skip to content

Add screen validation for non-array screens#206

Merged
sverhoeven merged 8 commits intomainfrom
165-screen-validation
Oct 11, 2021
Merged

Add screen validation for non-array screens#206
sverhoeven merged 8 commits intomainfrom
165-screen-validation

Conversation

@sverhoeven
Copy link
Copy Markdown
Collaborator

@sverhoeven sverhoeven commented Aug 24, 2021

Pull request details

List of related issues or pull requests

Refs: #165 and #229

Describe the changes made in this pull request

This PR adds validation for each non-array screen aka screens which are not for authors, identifiers or keywords.

If any field on a screen is invalid then the step in the left side stepper that belongs to that screen will get an exclamation triangle.

Instructions to review the pull request

  1. Go to authors screen without filling message and title on start screen
  2. The start step should get an exclamation triangle icon
  3. Fill message and title
  4. Go to authors again
  5. The exclamation triangle icon should have been replaced with the default OK circle.

@sverhoeven sverhoeven changed the title Add screen validation for screens Add screen validation for non-array screens Aug 24, 2021
@sverhoeven sverhoeven self-assigned this Aug 24, 2021
@sverhoeven sverhoeven marked this pull request as ready for review August 24, 2021 14:08
@fdiblen fdiblen mentioned this pull request Aug 24, 2021
@sverhoeven sverhoeven removed their assignment Aug 25, 2021
@fdiblen fdiblen self-requested a review October 6, 2021 12:31
Comment thread src/store/screens.ts Outdated
validator = makeOptionalFieldValidator(subschema)
}
const vr = validator(value.value)
console.log({ subschema, v: value.value, vr })
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
console.log({ subschema, v: value.value, vr })

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

done

Comment thread src/store/screens.ts Outdated
).every(
(v) => v === true
)
console.log(r)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
console.log(r)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

done

Comment thread src/components/Stepper.vue Outdated
Comment thread src/components/Stepper.vue Outdated
title="Related resources"
v-bind:order="4"
v-if="showAdvanced"
v-bind:error="!validScreens['related-resources'].value"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lower camelCase

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

What do you mean?

Copy link
Copy Markdown
Member

@jspaaks jspaaks Oct 11, 2021

Choose a reason for hiding this comment

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

validScreen.relatedResources.value
also in src/store/screens.ts versionSpecific and relatedResources

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

ah, done

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

And version-specific screen in src/components/Stepper.vue may be missing v-bind:error="!validScreens.versionSpecific.value"

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Added it in citation-file-format/cffinit@5a2a911 this morning

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

just noticed that thanks

Copy link
Copy Markdown
Member

@jspaaks jspaaks left a comment

Choose a reason for hiding this comment

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

All good, thanks for the PR

@sverhoeven
Copy link
Copy Markdown
Collaborator Author

Thanks for reviewing

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.

3 participants