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

fix: show spinner till content isnt visible #210

Merged
merged 3 commits into from
Oct 12, 2020
Merged

fix: show spinner till content isnt visible #210

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
64f083c
fix: show spinner till content isnt visible
nightnei Oct 7, 2020
9f240e4
chore: add Object.values and Array.from polyfills
nightnei Oct 12, 2020
9e1cbe1
chore: fix conflicts
nightnei Oct 12, 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
29 changes: 23 additions & 6 deletions ilc/client/handlePageTransaction.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,30 @@ const addContentListener = slotName => {
document.body.setAttribute('name', window.location.hash.slice(1));
}

const status = {
hasAddedNodes: false,
hasTextOrOpticNodes: false,
isAnyChildVisible: false,
};

const observer = new MutationObserver((mutationsList, observer) => {
const hasAddedNodes = !!mutationsList.find(mutation => mutation.addedNodes.length);
if (!hasAddedNodes) return;
if (!status.hasAddedNodes) {
status.hasAddedNodes = !!mutationsList.find(mutation => mutation.addedNodes.length);
}

// if we have rendered MS to DOM but meaningful content isn't rendered, e.g. due to essential data preload
if (!status.hasTextOrOpticNodes) {
const hasText = !!targetNode.innerText.trim().length
const hasOpticNodes = !!targetNode.querySelector(':not(div):not(span)');
status.hasTextOrOpticNodes = hasText || hasOpticNodes;
}

// if we have rendered MS to DOM but temporary hide it for some reason, e.g. to fetch data
if (!status.isAnyChildVisible) {
status.isAnyChildVisible = Array.from(targetNode.children).some(node => node.style.display !== 'none');
Copy link
Contributor

Choose a reason for hiding this comment

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

IE11 does not support Array.from

}

const hasText = !!targetNode.innerText.trim().length
const hasOpticNodes = !!targetNode.querySelector(':not(div):not(span)');
if (!hasText && !hasOpticNodes) return;
if (Object.values(status).some(n => !n)) return;
Copy link
Contributor

Choose a reason for hiding this comment

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

Use if with brackets pls

Copy link
Contributor

Choose a reason for hiding this comment

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

I found myself thinking that we probably do not have any polyfill for Object.values, so this would not work in IE11
So I suggest to test it in IE11 and change this by using supported methods or add polyfill for that if we still support IE11


observer.disconnect();
contentListeners.splice(contentListeners.indexOf(observer), 1);
Expand All @@ -67,7 +84,7 @@ const addContentListener = slotName => {
const targetNode = getSlotElement(slotName);
targetNode.style.display = 'none'; // we will show all new slots, only when all will be settled
hiddenSlots.push(targetNode);
observer.observe(targetNode, { childList: true, subtree: true });
observer.observe(targetNode, { childList: true, subtree: true, attributeFilter: ['style'] });
};

const renderFakeSlot = slotName => {
Expand Down
32 changes: 32 additions & 0 deletions ilc/client/handlePageTransaction.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -326,4 +326,36 @@ describe('handle page transaction', () => {

chai.expect(spinner.getRef()).to.be.null;
});

it('should destroy spinner when all fragments contain visible nodes', async () => {
const newBodyApplication = {
id: 'new-body-application',
class: 'new-body-spa',
};

newBodyApplication.ref = html`
<div id="${newBodyApplication.id}" class="${newBodyApplication.class}" style="display: none;">
Hello! I am hidden MS, so spinner is still visible
</div>
`;

applications.navbar.appendApplication();
applications.body.appendApplication();

handlePageTransaction(slots.body.id, slotWillBe.rerendered);

applications.body.removeApplication();
await clock.runAllAsync();

chai.expect(spinner.getRef()).to.be.not.null;

slots.body.ref.appendChild(newBodyApplication.ref);
await clock.runAllAsync();

chai.expect(spinner.getRef()).to.be.not.null;

document.getElementById(newBodyApplication.id).style.display = '';
await clock.runAllAsync();
chai.expect(spinner.getRef()).to.be.null;
});
});
2 changes: 1 addition & 1 deletion ilc/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
"build": "rimraf public && npm run build:systemjs && npm run build:client && npm run build:polyfills",
"build:client": "webpack --config build/webpack.js --progress --hide-modules",
"build:systemjs": "node ./systemjs/build.js",
"build:polyfills": "mkdir -p ./public && nwget https://polyfill.io/v3/polyfill.min.js?features=URL%2CObject.entries%2CDocumentFragment.prototype.append%2CElement.prototype.append%2CElement.prototype.remove%2CObject.assign%2CElement.prototype.closest%2CCustomEvent -O ./public/polyfill.min.js"
"build:polyfills": "mkdir -p ./public && nwget https://polyfill.io/v3/polyfill.min.js?features=URL%2CObject.entries%2CDocumentFragment.prototype.append%2CElement.prototype.append%2CElement.prototype.remove%2CObject.assign%2CElement.prototype.closest%2CCustomEvent%2CObject.values%2CArray.from -O ./public/polyfill.min.js"
},
"author": "Vladlen Fedosov",
"license": "Apache-2.0",
Expand Down