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

fix(background): fortify hasDOMDimensions check for null height #592

Merged
merged 2 commits into from
Apr 1, 2020
Merged

fix(background): fortify hasDOMDimensions check for null height #592

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
8332206
Do no attempt a render when there is no height
stephencookdev Mar 31, 2020
8b10d8d
Add Stephen to contributor list
stephencookdev Mar 31, 2020
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
3 changes: 2 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@
"Max Kolyanov (https://github.com/maxkolyanov)",
"Sherwin Heydarbeygi <[email protected]> (https://github.com/sherwinski)",
"Baldur Helgason <[email protected]> (https://github.com/baldurh)",
"Tanner Stirrat <[email protected]> (https://github.com/tstirrat15)"
"Tanner Stirrat <[email protected]> (https://github.com/tstirrat15)",
"Stephen Cook <[email protected]> (https://github.com/stephencookdev)"
],
"license": "ISC",
"bugs": {
Expand Down
3 changes: 2 additions & 1 deletion src/react-imgix-bg.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ const BackgroundImpl = props => {
className = ""
} = props;
const { w: forcedWidth, h: forcedHeight } = imgixParams;
const hasDOMDimensions = contentRect.bounds.top != null;
const hasDOMDimensions =
contentRect.bounds.width != null && contentRect.bounds.height != null;
const htmlAttributes = props.htmlAttributes || {};
const dpr = toFixed(2, imgixParams.dpr || global.devicePixelRatio || 1);
const ref = htmlAttributes.ref;
Expand Down
47 changes: 47 additions & 0 deletions test/unit/react-imgix.test.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import Imgix, {
__SourceImpl,
__PictureImpl
} from "react-imgix";
import { __BackgroundImpl } from "react-imgix-bg";

function shallow(element, target = __ReactImgixImpl, shallowOptions) {
return shallowUntilTarget(element, target, {
Expand All @@ -23,6 +24,14 @@ function shallow(element, target = __ReactImgixImpl, shallowOptions) {
const shallowSource = element => shallow(element, __SourceImpl);
const shallowPicture = element => shallow(element, __PictureImpl);

const makeBackgroundWithBounds = bounds => props => (
<__BackgroundImpl
measureRef={() => null}
contentRect={{ bounds }}
{...props}
/>
);

const src = "http://domain.imgix.net/image.jpg";
let sut;
let oldConsole, log;
Expand Down Expand Up @@ -504,6 +513,44 @@ describe("When in picture mode", () => {
});
});

describe("When in background mode", () => {
it("should be loading when there is a width but no height", () => {
const Background = makeBackgroundWithBounds({ top: 10, width: 100 });

sut = mount(
<Background
src={`${src}`}
className="bg-img"
contentRect={{ bounds: { top: 10, width: 100 } }}
>
<div>Content</div>
</Background>
);

expect(sut.find("div.bg-img").hasClass("react-imgix-bg-loading")).toBe(
true
);
});

it("should not be loading when a width and height are available", () => {
const Background = makeBackgroundWithBounds({
top: 10,
width: 100,
height: 100
});

sut = mount(
<Background src={`${src}`} className="bg-img">
<div>Content</div>
</Background>
);

expect(sut.find("div.bg-img").hasClass("react-imgix-bg-loading")).toBe(
false
);
});
});

describe("When using the component", () => {
let className = "img--enabled";
beforeEach(() => {
Expand Down