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

fix: remove loading from ElementLoader check #12

Merged
merged 3 commits into from
Dec 4, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
17 changes: 15 additions & 2 deletions src/loaders/ElementLoader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ egjs-imready
Copyright (c) 2020-present NAVER Corp.
MIT license
*/
import { addAutoSizer } from "../AutoSizer";
import { ImReadyLoaderOptions } from "../types";
import Loader from "./Loader";

Expand All @@ -14,13 +15,25 @@ export class ElementLoader<T extends HTMLElement> extends Loader<T> {
public setHasLoading(hasLoading: boolean) {
this.hasLoading = hasLoading;
}
public checkElement() {
if (!this.hasDataSize) {
public check() {
if (this.isSkip) {
// I'm Ready
this.onAlreadyReady(true);
return false;
}

if (this.hasDataSize) {
addAutoSizer(this.element, this.options.prefix);
this.onAlreadyPreReady();
} else {
// has not data size
this.trigger("requestChildren");
}
return true;
}
public checkElement() {
return true;
}
public destroy() {
this.removeAutoSizer();
this.trigger("requestDestroy");
Expand Down
2 changes: 1 addition & 1 deletion src/loaders/Loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export default abstract class Loader<T extends HTMLElement = any> extends Compon
this.isSkip = hasSkipAttribute(this.element);
}
public check() {
if (this.isSkip || hasSkipAttribute(this.element) || !this.checkElement()) {
if (this.isSkip || !this.checkElement()) {
// I'm Ready
this.onAlreadyReady(true);
return false;
Expand Down
44 changes: 44 additions & 0 deletions test/unit/Image.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,50 @@ describe("Test image", () => {
"preReadyElement", "preReady", "readyElement", "ready",
]);
});
it("should check that call preReady if include the loading img and not loading img.", async () => {
// Given
el.innerHTML = `
<img src="https://naver.github.io/egjs-infinitegrid/assets/image/23.jpg" loading="lazy"/>
<img src="https://naver.github.io/egjs-infinitegrid/assets/image/24.jpg"/>
`;

const imgs = el.querySelectorAll("img");

// inject loading attribute for not supported browser
imgs[0].setAttribute("loading", "lazy");
Object.defineProperty(imgs[0], "loading", {
value: "lazy",
});

const events = checkEventOrders(im);

// When
im.check([el]);

await waitEvent(im, "preReady");
// loading element1 is preReady
const element1Size1 = getSize(imgs[0]);
// no loading element2 is also preReady
const element2Size1 = getSize(imgs[1]);

await waitEvent(im, "ready");
const element1Size2 = getSize(imgs[0]);
const element2Size2 = getSize(imgs[1]);

// Then
expect(element1Size1).to.be.deep.equals([0, 0]);
expect(element2Size1).to.be.not.deep.equals([0, 0]);
expect(element1Size2).to.be.not.deep.equals([0, 0]);
// It is the same because preReady and ready occur at the same time.
expect(element2Size2).to.be.deep.equals(element2Size1);
// preReadyElement
expect(events[0].hasLoading).to.be.equals(true);
// preReady
expect(events[1].hasLoading).to.be.equals(true);
expectOrders(events, [
"preReadyElement", "preReady", "readyElement", "ready",
]);
});
it("should check that call preReady if include the loading attribute.", async () => {
// Given
el.innerHTML = `
Expand Down