Skip to content
This repository has been archived by the owner on May 21, 2018. It is now read-only.

Added loading svg prior to init #71

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 22 additions & 1 deletion PopcornEditor.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,30 @@ function PopcornEditor() {

this.init = function (el, url) {
var editor = (typeof el === 'string') ? document.getElementById(el) : el,
url = url || 'PopcornEditor/editor.html';
url = url || 'PopcornEditor/editor.html',
loadingDiv = null,
loadingSpinner = null;

window.addEventListener('message', onmessage);
this.listen('loaded', function () {
document.querySelector('#loading').remove();
});

loadingDiv = document.createElement('div');
loadingDiv.setAttribute('id', 'loading');
loadingDiv.style.width = '100%';
loadingDiv.style.height = '100%';
loadingDiv.style.backgroundColor = 'white';
loadingDiv.style.display = 'flex';
loadingDiv.style.justifyContent = 'center';
Copy link
Contributor

Choose a reason for hiding this comment

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

If it meshes with the architecture you're thinking here, I'd put this in index.html or some css file, just to be clean.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

index.html is just meant to be a an example though correct?


loadingSpinner = document.createElement('img');
loadingSpinner.setAttribute('class', 'loading-spinner');
loadingSpinner.setAttribute('src', 'PopcornEditor/resources/ring-alt.svg');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sanity check here: is this the proper way we should read resources from the library outside of the iframe?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I'm a bit confused. This is how I see it atm:

  • PopcornEditor loads an iframe
  • In the iframe is the actual editor, which includes PopcornEditor/resources/ring-alt.svg.
  • This code reaches into the src that's meant for the content inside the iframe to get an image.

If the stuff inside src is meant to be its own beast, I'd put this image outside in the context of PopcornEditor.js.

Otherwise, ¯_(ツ)_/¯ sure! This makes sense.

loadingSpinner.style.width = '30%';

loadingDiv.appendChild(loadingSpinner);
editor.appendChild(loadingDiv);
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment about putting this in index.html as above. Could probably go within the html there, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same goes for above. PopcornEditor.js can be included in anyone's web page so I assume that while they would use index.html as a reference, they shouldn't necessarily have to copy over the css and stuff in it.


this.iframe = document.createElement('iframe'),

Expand Down
1 change: 1 addition & 0 deletions PopcornEditor/resources/ring-alt.svg
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.