Teaser Block Image Overide #6147
Conversation
✅ Deploy Preview for plone-components canceled.
|
| )} | ||
| { | ||
| // eslint-disable-next-line | ||
| url != undefined && !isInternalURL(url) ? ( |
There was a problem hiding this comment.
@Tishasoumya-02 why not simply url && !isInternalURL? Then you don't need to disable the eslint warning.
There was a problem hiding this comment.
Yes, I tried doing this, but blocks-teaser.js cypress tests keeps failing , not sure why this behavior. I also tried using isUndefined() provided by lodash but same results
There was a problem hiding this comment.
Yes, I tried doing this, but
blocks-teaser.jscypress tests keeps failing , not sure why this behavior. I also tried usingisUndefined()provided by lodash but same results
Works for me locally
We also need as part of this fix a cypress test within blocks-teaser that makes use of this preview_image with an external image, maybe it could be an image from demo.plone.org or plone.org @sneridagh what do you suggest for testing this feature of the external image to link to for the teaser block?
An alternative would be an unittest which allows to enter whatever values in the url without actually visiting anything.
There was a problem hiding this comment.
Okay, not sure why it was not passing on my machine, but made the changes and disabled eslint warning
There was a problem hiding this comment.
@ichim-david @Tishasoumya-02 We are already using images from the Github CDN in tests, we can use them too:
Tishasoumya-02
force-pushed
the
teaser-image-overide
branch
from
083cbfc to
605e6ba
Compare
|
@Tishasoumya-02 I confirm it works when adding an external image. See this video for what I mean teaser-image-source-2.mp4 |
| @@ -50,16 +51,22 @@ const TeaserDefaultTemplate = (props) => { | |||
| } | |||
| > | |||
| <div className="teaser-item default"> | |||
| {(href.hasPreviewImage || href.image_field || image) && ( | |||
| {url && !isInternalURL(url) ? ( | |||
There was a problem hiding this comment.
| {url && !isInternalURL(url) ? ( | |
| {url && !image?.image_field ? ( |
@Tishasoumya-02 try it like this, it will solve the situation where you link directly to a scale from within the site because you simply copied the link from an internal image.
Here is a screenshot showing this in action on my devel
We also have no test for this interaction where you add as an image override the link to an image scale.
I would be in favor of adding in that test the link to internal images at a minimum and external image if possible but we still didn't get any feedback from @sneridagh regarding my comment here #6147 (comment)
There was a problem hiding this comment.
@sneridagh these changes fixes the display of external images and internal images from scales. Regarding the test writing it's your call.
There was a problem hiding this comment.
@Tishasoumya-02 could you write a quick test as @ichim-david suggested using an external image (see comment above, can be a GH CDN raw image). Then it will be complete.
|
@ichim-david I have added a test for external image |
|
@Tishasoumya-02 I know it's not your fault but the pull request isn't green try to rename package/volto/news/.gitkeep to .keep as they recognize that as something to ignore. |
|
All tests are passing now :) |
- Correcting the orchid typo from all tests as well
|
@Tishasoumya-02 @ichim-david the |
|
@wesleybl I've merged the pull request. take into consideration that the issue with .gitkeep will affect the other packages or apps as well, if you will edit any of them you will need to make the same change I've notified @Tishasoumya-02 to do in packages/volto. |
|
@Tishasoumya-02 thank you for your work, it took us some time to get here but open source work isn't easy and it takes a lot of work both for the creator and the reviewer to get something to the finish line. |
|
@Tishasoumya-02 @wesleybl @ichim-david new issue created for the towncrier issue at #6225. |
Thank You ! |
#6137