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.

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

Jump to bottom

Add loadedImageProps and placeholderProps props #46

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

Add loadedImageProps and placeholderProps props #46

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1b72948
Add loadedImageProps prop
Aljullu Mar 16, 2019
9fc73b0
Add placeholderProps prop
Aljullu Aug 7, 2019
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
2 changes: 2 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,9 @@ export default MyImage;
| delayMethod | `String` | `throttle` | Method from lodash to use to delay the scroll/resize events. It can be `throttle` or `debounce`. |
| delayTime | `Number` | 300 | Time in ms sent to the delayMethod. |
| effect | `String` | | Name of the effect to use. Please, read next section with an explanation on how to use them. |
| loadedImageProps | `Object` | | Props passed to the `<img>` element once it's loaded. Useful to pass class names or style attributes that should only be applied once the image is loaded. |
| placeholder | `ReactClass` | `<span>` | React element to use as a placeholder. |
| placeholderProps | `Object` | | Props assigned to the placeholder <span>, useful to overwrite the style or assign other HTML attributes. |
| placeholderSrc | `String` | | Image src to display while the image is not visible or loaded. |
| threshold | `Number` | 100 | Threshold in pixels. So the image starts loading before it appears in the viewport. |
| visibleByDefault | `Boolean` | false | Whether the image must be visible from the beginning. |
Expand Down
41 changes: 29 additions & 12 deletions src/components/LazyLoadImage.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,18 @@ class LazyLoadImage extends React.Component {

getImg() {
const { afterLoad, beforeLoad, delayMethod, delayTime, effect,
placeholder, placeholderSrc, scrollPosition, threshold,
visibleByDefault, wrapperClassName, ...imgProps } = this.props;
loadedImageProps, placeholder, placeholderProps, placeholderSrc,
scrollPosition, threshold, visibleByDefault, wrapperClassName,
...imgProps } = this.props;
const { loaded } = this.state;
const loadedProps = loaded ? loadedImageProps : null;

return <img onLoad={this.onImageLoad()} {...imgProps} />;
return (
<img
onLoad={this.onImageLoad()}
{...imgProps}
{...loadedProps} />
);
}

getLazyLoadImage(image) {
Expand All @@ -58,26 +66,31 @@ class LazyLoadImage extends React.Component {
}

getWrappedLazyLoadImage(lazyLoadImage) {
const { effect, height, placeholderSrc,
const { effect, height, placeholderProps, placeholderSrc,
width, wrapperClassName } = this.props;
const { loaded } = this.state;

const loadedClassName = loaded ?
' lazy-load-image-loaded' :
'';
const props = {
...placeholderProps,
style: {
backgroundImage: loaded ? '' : 'url( ' + placeholderSrc + ')',
Copy link

Choose a reason for hiding this comment

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

Maybe it will be better to put these styles somewhere in the css, so that it will be enough to pass className in the placeholderProps (and merge classes here) to override styles without important hacks

Copy link

Choose a reason for hiding this comment

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

Also it will simplify applying different styles for different resolutions using media queries

Copy link
Owner Author

Choose a reason for hiding this comment

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

Thanks for taking a look @BilyachenkoOY ! The issue is that we need to read placeholderSrc from the props, and that can't be done if we move the styles completely to a CSS file. It's true we could keep backgroundImage in the style while moving everything else to a CSS file, but I think that would be confusing.

In the case you need to make a lot of modifications to the placeholder style, probably it's a better idea passing a node as the placeholder prop instead of adding props to the default placeholder. How does that sound?

Copy link

Choose a reason for hiding this comment

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

I still believe that there should be as few as possible in props and it will be fine if there will be only backgroundImage + height&width since it should be as easy as possible to override styles using wrapperClassName.

As for placeholder , it will be placed inside this <span> and is not suitable for all style manipulations.

It is not a big issue to me to add few important directives to my CSS, so it is up to you

backgroundSize: loaded ? '' : '100% 100%',
color: 'transparent',
display: 'inline-block',
height: height,
width: width,
...placeholderProps.style,
},
};

return (
<span
className={wrapperClassName + ' lazy-load-image-background ' +
effect + loadedClassName}
style={{
backgroundImage: loaded ? '' : 'url( ' + placeholderSrc + ')',
backgroundSize: loaded ? '' : '100% 100%',
color: 'transparent',
display: 'inline-block',
height: height,
width: width,
}}>
{ ...props }>
{lazyLoadImage}
</span>
);
Expand Down Expand Up @@ -105,6 +118,8 @@ LazyLoadImage.propTypes = {
delayMethod: PropTypes.string,
delayTime: PropTypes.number,
effect: PropTypes.string,
loadedImageProps: PropTypes.object,
placeholderProps: PropTypes.object,
placeholderSrc: PropTypes.string,
threshold: PropTypes.number,
visibleByDefault: PropTypes.bool,
Expand All @@ -117,6 +132,8 @@ LazyLoadImage.defaultProps = {
delayMethod: 'throttle',
delayTime: 300,
effect: '',
loadedImageProps: {},
placeholderProps: {},
placeholderSrc: '',
threshold: 100,
visibleByDefault: false,
Expand Down
32 changes: 32 additions & 0 deletions src/components/LazyLoadImage.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,22 @@ describe('LazyLoadImage', function() {
expect(loadedWrapper.style.getPropertyValue('background-size')).toEqual('');
});

it('sets correct props to the loaded image', function() {
const lazyLoadImage = mount(
<LazyLoadImage className="lorem" loadedImageProps={{ className: 'ipsum' }} />
);

let img = findRenderedDOMComponentWithTag(lazyLoadImage.instance(), 'img');

expect(img.className).toEqual('lorem');

Simulate.load(img);

img = findRenderedDOMComponentWithTag(lazyLoadImage.instance(), 'img');

expect(img.className).toEqual('ipsum');
});

it('adds the effect class', function() {
const lazyLoadImage = mount(
<LazyLoadImage effect="blur" />
Expand Down Expand Up @@ -142,4 +158,20 @@ describe('LazyLoadImage', function() {

expect(span.length).toEqual(0);
});

it('renders placeholder with correct style', function() {
const lazyLoadImage = mount(
<LazyLoadImage
placeholderSrc="lorem-ipsum.jpg"
placeholderProps={{
'aria-hidden': 'true',
style: { display: 'block' },
}} />
);

const span = findRenderedDOMComponentWithTag(lazyLoadImage.instance(), 'span');

expect(span.style.getPropertyValue('display')).toEqual('block');
expect(span.getAttribute('aria-hidden')).toEqual('true');
});
});