-
Notifications
You must be signed in to change notification settings - Fork 81
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
add custom validation #707
add custom validation #707
Conversation
LARGE: 'large' | ||
}; | ||
|
||
function cardValidator(allowedValues, propName) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Neat :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @AllanOXDi. I left one note that would be nice to have before merge, and also lint and changelog checks will need to pass. After that, feel free to merge.
lib/KCard/index.vue
Outdated
}, | ||
thumbnailDisplay: { | ||
type: String, | ||
default: Thumbnail_Displays.NONE, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It'd be ideal if we could use constants as default
values, but unfortunately there's limitation regarding our documentation generation that can't pick it up:
Would you please revert to default: 'none'
on this line? Same applies to default: Layouts .HORIZONTAL,
This will ensure that consumers will understand what values, previously mentioned in the prop description, are used as defaults
@@ -4,15 +4,17 @@ Changelog is rather internal in nature. See release notes for the public overvie | |||
|
|||
## Upcoming version 5.x.x (`develop` branch) | |||
|
|||
[#709] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like a merge conflict? I think this change should stay here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep! resolving it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed now!
f4f45ab
to
3f3f9d6
Compare
Description
Issue addressed
Addresses #PR# HERE
Before/after screenshots
Changelog
[add custom validation #707]
KCard
componentKCard
[add custom validation #707]: add custom validation #707
Steps to test
(optional) Implementation notes
At a high level, how did you implement this?
Does this introduce any tech-debt items?
Testing checklist
Reviewer guidance
After review
CHANGELOG.md
Comments