Skip to content

Adding "imgClass" prop to the CldImage vue component #148

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Adding "imgClass" prop to the CldImage vue component #148

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d1f1fa2
Adding "imgClass" prop to the CldImage vue component
alxistn Aug 27, 2021
1ac8f72
Fix empty space when imgClass prop is empty
alxistn Aug 30, 2021
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: 9 additions & 1 deletion src/components/CldImage/CldImage.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,14 @@ export default {
type: String,
default: "",
validator: value => !value || !!ACCESSIBILITY_TRANSFORMATIONS[value]
},

/**
* The css class that will be applied to the <img> tag
*/
imgClass: {
type: String,
default: "",
}
},
data() {
Expand All @@ -86,7 +94,7 @@ export default {
this.imageLoaded = true;
},
renderImageOnly(src, hasPlaceholder = false) {
const imgClass = `${IMAGE_CLASSES.DEFAULT} ${!this.imageLoaded ? IMAGE_CLASSES.LOADING : IMAGE_CLASSES.LOADED}`
const imgClass = `${IMAGE_CLASSES.DEFAULT} ${!this.imageLoaded ? IMAGE_CLASSES.LOADING : IMAGE_CLASSES.LOADED} ${this.imgClass}`
Copy link
Contributor

Choose a reason for hiding this comment

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

Please note that this adds an empty space at the end in case this.imgClass is empty, and thus our tests in Travis are failing (You can see the breaking tests on the pull request page).

Maybe we can trim extra spaces at the end to avoid this problem?

Copy link
Author

Choose a reason for hiding this comment

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

I've added a ternary operation that add the class if it is not empty, it seems to works fine.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, one last thing - can you please add a test that confirms that this feature works as expected?
I'd imagine a test that uses this new propr and confirms that the class appears on the right element in the DOM to be enough.

const style = {
...(this.responsive ? RESPONSIVE_CSS[this.responsive] : {}),
...(!this.imageLoaded && hasPlaceholder ? IMAGE_WITH_PLACEHOLDER_CSS[IMAGE_CLASSES.LOADING] : {})
Expand Down