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

add custom validation #707

Merged
merged 2 commits into from
Aug 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,16 @@ Changelog is rather internal in nature. See release notes for the public overvie

## Upcoming version 5.x.x (`develop` branch)

[#707]
- **Description:** Card Validations
- **Products impact:**
- **Addresses:** [#695](https://github.com/learningequality/kolibri-design-system/issues/695)
- **Components:** `KCard`
- **Breaking:** No
- **Impacts a11y:** No
- **Guidance:**
[#707] https://github.com/learningequality/kolibri-design-system/pull/707

[#709]
Copy link
Member

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep! resolving it

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed now!

- **Description:** Update spaces to the latest design`KCard`
- **Products impact:** Card updates
Expand Down
37 changes: 31 additions & 6 deletions lib/KCard/index.vue
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,30 @@

import BaseCard from './BaseCard.vue';

const Layouts = {
HORIZONTAL: 'horizontal',
VERTICAL: 'vertical',
};

const Thumbnail_Displays = {
NONE: 'none',
SMALL: 'small',
LARGE: 'large',
};

function cardValidator(allowedValues, propName) {
Copy link
Member

Choose a reason for hiding this comment

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

Neat :)

return function(value) {
if (!Object.values(allowedValues).includes(value)) {
throw new Error(
`Invalid ${propName} value: '${value}'. Allowed values are: ${Object.values(
allowedValues
).join(', ')}`
);
}
return true;
};
}

export default {
name: 'KCard',
components: { BaseCard },
Expand Down Expand Up @@ -110,15 +134,16 @@
layout: {
type: String,
default: 'horizontal',
validator: cardValidator(Layouts, 'layout'),
},
/**
* Controls how the thumbnail appears in the card.
* Expected Options: 'none' (default), 'small', or 'large'.
* */
thumbnailDisplay: {
type: String,
required: false,
default: 'none',
validator: cardValidator(Thumbnail_Displays, 'thumbnailDisplay'),
},
/**
* Sets the thumbnail path.
Expand Down Expand Up @@ -160,13 +185,13 @@
return this.stylesAndClasses.thumbnailStyles;
},
/*
Returns dynamic classes and few style-like data that CSS was cumbersome/impossible to use for ,or are in need of using theme, grouped by all possible combinations of layouts.
Returns dynamic classes and few style-like data that CSS was cumbersome/impossible to use for ,or are in need of using theme, grouped by all possible combinations of layouts.

New styles and classes are meant to be added to this single-source-of-truth object so
that we can easily locate all styling being applied to a particular layout
New styles and classes are meant to be added to this single-source-of-truth object so
that we can easily locate all styling being applied to a particular layout

Could be further simplified by using solely dynamic styles, but that would have detrimental effects on our auto RTL machinery and we would need to take care manually of more places.
*/
Could be further simplified by using solely dynamic styles, but that would have detrimental effects on our auto RTL machinery and we would need to take care manually of more places.
*/
stylesAndClasses() {
/* In px. Needs to be the same as $spacer variable in styles part */
const SPACER = 24;
Expand Down