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 Image for srcset and lazy loading #2103

Closed
wants to merge 69 commits into from
Closed
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
69 commits
Select commit Hold shift + click to select a range
7d6d66a
feat: add ImageFromUrl for srcset and lazy loading
nzambello Jan 5, 2021
46f7527
feat: ImageFromUrl accepts other props for img
nzambello Jan 5, 2021
4f295fb
docs: updated changelog
nzambello Jan 5, 2021
fa9f0bc
refactor: Image component for responsive images
nzambello Jan 15, 2021
7c36913
Merge branch 'master' into add_srcset_imgs
nzambello Jan 15, 2021
2c651dc
test: updated cypress image block test
nzambello Jan 15, 2021
eda9c57
Merge branch 'master' into add_srcset_imgs
nzambello Jan 15, 2021
fa0df06
docs: Update CHANGELOG.md
nzambello Jan 19, 2021
a3a8df9
fix: hero block image styles
nzambello Jan 20, 2021
1cfa340
fix: hero image mobile
nzambello Jan 21, 2021
37b1c15
Merge remote-tracking branch 'origin/master' into add_srcset_imgs
tiberiuichim Jan 22, 2021
5a183c6
Merge branch 'master' into add_srcset_imgs
nzambello Mar 22, 2021
397bee8
Merge branch 'master' into add_srcset_imgs
nzambello Apr 22, 2021
86b7ade
fix: Image noscript styles
nzambello Apr 22, 2021
542b86f
feat: add maxSize and useOriginal props to Image
nzambello Apr 27, 2021
4da4fb0
docs: add Image ref+
nzambello Apr 27, 2021
2bf73d9
test: update Image tests
nzambello Apr 27, 2021
931fd27
Merge branch 'master' into add_srcset_imgs
nzambello May 11, 2021
d3fe9bb
Merge branch 'master' into add_srcset_imgs
giuliaghisini May 27, 2021
c168e48
fix: image noscript className
nzambello Jul 20, 2021
7540b9f
Merge branch 'master' into add_srcset_imgs
nzambello Aug 29, 2021
e9a030e
test: update snapshot, fix cypress objectBrowser img, fix eslint check
nzambello Aug 30, 2021
a786b98
test: fix cypress test for new image component
nzambello Aug 30, 2021
fbf2c93
Merge branch 'master' into add_srcset_imgs
nzambello Sep 2, 2021
d88e7fd
Merge branch 'master' into add_srcset_imgs
nzambello Sep 19, 2021
e458667
Merge branch 'master' into add_srcset_imgs
nzambello Oct 23, 2021
5bd771d
chore: update image scales from plone.volto sizes
nzambello Oct 27, 2021
a748813
feat: add srcset to image only when visible and placeholder loaded to…
nzambello Oct 27, 2021
1cb2b46
test: update scales in cy tests
nzambello Oct 27, 2021
5f6beb9
Merge branch 'master' into add_srcset_imgs
nzambello Oct 27, 2021
e0ee9e5
test: update scales in cy tests
nzambello Oct 27, 2021
5cceee9
Merge branch 'master' into add_srcset_imgs
nzambello Oct 28, 2021
f4bb5d4
Merge branch 'master' into add_srcset_imgs
nzambello Nov 2, 2021
5ff9660
Merge branch 'master' into add_srcset_imgs
nzambello Nov 8, 2021
b828e35
Merge branch 'master' into add_srcset_imgs
nzambello Nov 14, 2021
103b8f7
Merge branch 'master' into add_srcset_imgs
nzambello Nov 19, 2021
1fc051b
fix: handle svg images whith stranger widht height determined by plone
giuliaghisini Nov 19, 2021
8f89c41
feat: add image layout options, fix loading async
nzambello Nov 19, 2021
071fd69
refactor: use volto Image component in views
nzambello Nov 19, 2021
a210d23
Merge branch 'add_srcset_imgs' of github.com:plone/volto into add_src…
nzambello Nov 19, 2021
609ecdb
docs: update changelog
nzambello Nov 19, 2021
ceda054
test: update tests and snapshots
nzambello Nov 19, 2021
86df9d1
test: update cy test
nzambello Nov 23, 2021
4b56ef2
Merge branch 'master' into add_srcset_imgs
nzambello Nov 24, 2021
1c2b895
Merge branch 'master' into add_srcset_imgs
nzambello Nov 25, 2021
1ac4e2b
refactor: default view classes
nzambello Nov 25, 2021
4fc573a
fix: image srcset render
nzambello Nov 25, 2021
1576e3d
docs: update Image docs
nzambello Nov 26, 2021
53925eb
Merge branch 'master' into add_srcset_imgs
nzambello Nov 30, 2021
98e0f46
Merge branch 'master' into add_srcset_imgs
giuliaghisini Dec 20, 2021
cf533c1
chore: add the next item grather then imageRef width, to avoid less q…
giuliaghisini Feb 2, 2022
47df2bd
Merge branch 'master' into add_srcset_imgs
giuliaghisini Feb 14, 2022
1932873
fix: handle also image height
giuliaghisini Mar 29, 2022
4637a37
chore: handle devicePixelRatio
giuliaghisini Mar 29, 2022
7b6af2e
fix object images
giuliaghisini Mar 29, 2022
3660875
Merge branch 'master' into add_srcset_imgs
giuliaghisini Apr 4, 2022
d22b2a3
chore: added sizes to srcset and fixed lazy loading effect
giuliaghisini May 3, 2022
a5af866
re-added timout because need for initial visible images
giuliaghisini May 3, 2022
b3c879d
do not lazy critical images
giuliaghisini May 4, 2022
fba8125
removed condition on image complete to load images on top of the page
giuliaghisini May 4, 2022
af854dc
removed loading lazy from placeholder
giuliaghisini May 4, 2022
ed1e9e5
fixed noscript img
giuliaghisini May 4, 2022
5418d2b
fixes
giuliaghisini May 4, 2022
c74c3b3
added 'responsive prop to Image and width and heght inline if maxWidt…
giuliaghisini May 4, 2022
5a101df
added style aspect-ratio to help browser determining image area if no…
giuliaghisini May 6, 2022
6f4ff20
added style aspect-ratio to help browser determining image area if no…
giuliaghisini May 6, 2022
1fb6a0a
fixed order of srcset in internalUrl case image
giuliaghisini May 6, 2022
4d764ca
handle minSize in getImageAttributes
giuliaghisini May 9, 2022
a07bc4c
fix: fixed src of imageObject
giuliaghisini Jun 22, 2022
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
### Feature

- Lazy load image in blocks Image and HeroImage @mamico
- Add ImageFromUrl with srcset and lazy loading using Plone miniatures @nzambello
nzambello marked this conversation as resolved.
Show resolved Hide resolved

### Bugfix

Expand Down
2 changes: 1 addition & 1 deletion cypress/integration/blocks-image.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ if (Cypress.env('API') !== 'guillotina') {
// then image src must be equal to image name
cy.get('.block img')
.should('have.attr', 'src')
.and('eq', '/my-page/image.png/@@images/image');
.and('eq', '/my-page/image.png/@@images/image/listing');
});

it('Create a image block document in edit mode', () => {
Expand Down
11 changes: 2 additions & 9 deletions src/components/manage/Blocks/HeroImageLeft/View.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import React from 'react';
import PropTypes from 'prop-types';
import { flattenToAppURL } from '@plone/volto/helpers';
import Image from '@plone/volto/components/theme/Image/Image';
Copy link
Member

Choose a reason for hiding this comment

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

Not from components?

import { Image } from '@plone/volto/components


/**
* View image block class.
Expand All @@ -15,14 +15,7 @@ import { flattenToAppURL } from '@plone/volto/helpers';
const View = ({ data }) => (
<div className="block hero">
<div className="block-inner-wrapper">
{data.url && (
<img
src={`${flattenToAppURL(data.url)}/@@images/image`}
alt=""
className="hero-image"
loading="lazy"
/>
)}
{data.url && <Image image={data.url} className="hero-image" />}
<div className="hero-body">
{data.title && <h1>{data.title}</h1>}
{data.description && <p>{data.description}</p>}
Expand Down
4 changes: 3 additions & 1 deletion src/components/manage/Blocks/HeroImageLeft/View.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ import renderer from 'react-test-renderer';
import View from './View';

test('renders a view hero component', () => {
const component = renderer.create(<View data={{ url: 'heroimage.jpg' }} />);
const component = renderer.create(
<View data={{ url: 'http://localhost:8080/Plone/heroimage.jpg' }} />,
);
const json = component.toJSON();
expect(json).toMatchSnapshot();
});
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,36 @@ exports[`renders a view hero component 1`] = `
<div
className="block-inner-wrapper"
>
<img
alt=""
className="hero-image"
loading="lazy"
src="heroimage.jpg/@@images/image"
<picture>
<img
alt=""
className="hero-image"
loading="lazy"
role="img"
src="/heroimage.jpg/@@images/image/listing"
style={
Object {
"objectFit": "cover",
"width": "100%",
}
}
/>
</picture>
<noscript
dangerouslySetInnerHTML={
Object {
"__html": "
<img
src=\\"/heroimage.jpg/@@images/image/listing\\"
srcset=\\"/heroimage.jpg/@@images/image/large 768w, /heroimage.jpg/@@images/image/preview 400w, /heroimage.jpg/@@images/image/mini 200w, /heroimage.jpg/@@images/image/thumb 128w, /heroimage.jpg/@@images/image/tile 64w, /heroimage.jpg/@@images/image/icon 32w, /heroimage.jpg/@@images/image/listing 16w, /heroimage.jpg/@@images/image 1200w\\"
alt=\\"\\"
className=\\"hero-image\\"
role=\\"img\\"
loading=\\"lazy\\"
style={{ width: '100%', objectFit: 'cover' }}
",
}
}
/>
<div
className="hero-body"
Expand Down
25 changes: 4 additions & 21 deletions src/components/manage/Blocks/Image/View.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import React from 'react';
import PropTypes from 'prop-types';
import { Link } from 'react-router-dom';
import cx from 'classnames';
import Image from '@plone/volto/components/theme/Image/Image';

import { flattenToAppURL, isInternalURL } from '@plone/volto/helpers';

Expand All @@ -30,33 +31,15 @@ const View = ({ data, detached }) => (
<>
{(() => {
const image = (
<img
<Image
image={data.url}
alt={data.alt}
className={cx({
'full-width': data.align === 'full',
large: data.size === 'l',
medium: data.size === 'm',
small: data.size === 's',
})}
src={
isInternalURL(data.url)
? // Backwards compat in the case that the block is storing the full server URL
(() => {
if (data.size === 'l')
return `${flattenToAppURL(data.url)}/@@images/image`;
if (data.size === 'm')
return `${flattenToAppURL(
data.url,
)}/@@images/image/preview`;
if (data.size === 's')
return `${flattenToAppURL(
data.url,
)}/@@images/image/mini`;
return `${flattenToAppURL(data.url)}/@@images/image`;
})()
: data.url
}
alt={data.alt || ''}
loading="lazy"
/>
);
if (data.href) {
Expand Down
4 changes: 2 additions & 2 deletions src/components/manage/Blocks/Image/View.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ describe('Image View Component', () => {
test('renders a view image component with a local image', () => {
const { getByRole } = render(<View data={{ url: '/image.jpg' }} />);
const img = getByRole('img');
expect(img).toHaveAttribute('src', '/image.jpg/@@images/image');
expect(img).toHaveAttribute('src', '/image.jpg/@@images/image/listing');
Copy link
Member

Choose a reason for hiding this comment

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

listing scale by default? why is that? that scale is not in the plone.volto ones.

expect(img).toHaveAttribute('loading', 'lazy');
});
test('renders a view image component with a local image with a link', () => {
Expand All @@ -19,7 +19,7 @@ describe('Image View Component', () => {
);
const img = getByRole('img');
const a = container.querySelector('a');
expect(img).toHaveAttribute('src', '/image.jpg/@@images/image');
expect(img).toHaveAttribute('src', '/image.jpg/@@images/image/listing');
expect(a).toHaveAttribute('href', '/front-page');
});
test('renders a view image component with an external image', () => {
Expand Down
8 changes: 4 additions & 4 deletions src/components/manage/Blocks/LeadImage/View.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { Link } from 'react-router-dom';
import cx from 'classnames';
import { settings } from '~/config';

import { flattenToAppURL } from '@plone/volto/helpers';
import Image from '@plone/volto/components/theme/Image/Image';

/**
* View image block class.
Expand All @@ -30,10 +30,10 @@ const View = ({ data, properties }) => (
<>
{(() => {
const image = (
<img
<Image
image={properties.image}
alt={properties.image_caption}
className={cx({ 'full-width': data.align === 'full' })}
src={flattenToAppURL(properties.image.download)}
alt={properties.image_caption || ''}
/>
);
if (data.href) {
Expand Down
40 changes: 39 additions & 1 deletion src/components/manage/Blocks/LeadImage/View.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,45 @@ test('renders a view image component', () => {
data={{}}
properties={{
image: {
download: 'image.png',
download: 'http://localhost:8080/Plone/test-images/@@images/image',
width: 1920,
scales: {
listing: {
download:
'http://localhost:8080/Plone/test-images/@@images/image/listing',
width: 16,
},
icon: {
download:
'http://localhost:8080/Plone/test-images/@@images/image/icon',
width: 32,
},
tile: {
download:
'http://localhost:8080/Plone/test-images/@@images/image/tile',
width: 64,
},
thumb: {
download:
'http://localhost:8080/Plone/test-images/@@images/image/thumb',
width: 128,
},
mini: {
download:
'http://localhost:8080/Plone/test-images/@@images/image/mini',
width: 200,
},
preview: {
download:
'http://localhost:8080/Plone/test-images/@@images/image/preview',
width: 400,
},
large: {
download:
'http://localhost:8080/Plone/test-images/@@images/image/large',
width: 768,
},
},
},
}}
/>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,36 @@ exports[`renders a view image component 1`] = `
<p
className="block image align center"
>
<img
alt=""
className=""
src="image.png"
<picture>
<img
alt=""
className=""
loading="lazy"
role="img"
src="http://localhost:8080/Plone/test-images/@@images/image/listing"
style={
Object {
"objectFit": "cover",
"width": "100%",
}
}
/>
</picture>
<noscript
dangerouslySetInnerHTML={
Object {
"__html": "
<img
src=\\"http://localhost:8080/Plone/test-images/@@images/image/listing\\"
srcset=\\"/test-images/@@images/image/listing 16w, /test-images/@@images/image/icon 32w, /test-images/@@images/image/tile 64w, /test-images/@@images/image/thumb 128w, /test-images/@@images/image/mini 200w, /test-images/@@images/image/preview 400w, /test-images/@@images/image/large 768w, /test-images/@@images/image 1920w\\"
alt=\\"\\"
className=\\"\\"
role=\\"img\\"
loading=\\"lazy\\"
style={{ width: '100%', objectFit: 'cover' }}
",
}
}
/>
</p>
`;
8 changes: 3 additions & 5 deletions src/components/manage/Blocks/Listing/DefaultTemplate.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { ConditionalLink } from '@plone/volto/components';
import { flattenToAppURL } from '@plone/volto/helpers';
import { settings } from '~/config';

import Image from '@plone/volto/components/theme/Image/Image';
import DefaultImageSVG from '@plone/volto/components/manage/Blocks/Listing/default-image.svg';
import { isInternalURL } from '@plone/volto/helpers/Url/Url';

Expand Down Expand Up @@ -34,11 +35,8 @@ const DefaultTemplate = ({ items, linkMore, isEditMode }) => {
<img src={DefaultImageSVG} alt="" />
)}
{item[settings.listingPreviewImageField] && (
<img
src={flattenToAppURL(
item[settings.listingPreviewImageField].scales.preview
.download,
)}
<Image
image={item[settings.listingPreviewImageField]}
alt={item.title}
/>
)}
Expand Down
73 changes: 73 additions & 0 deletions src/components/theme/Image/Image.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
import React, { useState, useEffect } from 'react';
import PropTypes from 'prop-types';
import { getImageAttributes } from '@plone/volto/helpers';

/**
* Image component
* @param {object | string} image - Plone image as object or url
* @param {string} alt - Alternative text for image
* @param {string} className - CSS class attribute
* @param {string} role - img role attribute
* @param {boolean} critical - whether to load the image
*/
const Image = ({
image,
alt = '',
className,
role = 'img',
critical = false,
...imageProps
}) => {
const { src, srcSet } = getImageAttributes(image);
const [isClient, setIsClient] = useState(false);
const [srcset, setSrcset] = useState(
critical && srcSet ? srcSet.join(', ') : null,
);

useEffect(() => {
setIsClient(true);
if (!srcset && srcSet?.length > 0) setSrcset(srcSet.join(', '));
}, [srcSet, srcSet?.length, srcset]);

return (
<>
<picture>
{srcset && (isClient || critical) && <source srcSet={srcset} />}
<img
src={src}
alt={alt}
className={className}
role={role}
loading="lazy"
style={{ width: '100%', objectFit: 'cover' }}
pnicolli marked this conversation as resolved.
Show resolved Hide resolved
{...imageProps}
/>
</picture>
{!critical && (
<noscript
dangerouslySetInnerHTML={{
__html: `
<img
src="${src}"
${srcSet?.length && `srcset="${srcSet.join(', ')}"`}
alt="${alt}"
className="${className}"
nzambello marked this conversation as resolved.
Show resolved Hide resolved
role="${role}"
Copy link
Member

Choose a reason for hiding this comment

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

In this HTML string I think that attribute values strings should be escaped based on the rules in this SO answer.

loading="lazy"
style={{ width: '100%', objectFit: 'cover' }}
`,
}}
/>
)}
</>
);
};

Image.propTypes = {
image: PropTypes.oneOfType([PropTypes.object, PropTypes.string]),
alt: PropTypes.string,
className: PropTypes.string,
role: PropTypes.string,
};

export default Image;
Loading