Create Sandcastle Standalone Mode #7250

OmarShehata · 2018-11-08T17:51:25Z

This is attempt 2 to create a standalone Sandcastle model. This is the previous attempt #7097.

This is a refactor in that it works exactly the same way from the user's prespective. Any Sandcastle URL like:

https://cesiumjs.org/Cesium/Build/Apps/Sandcastle/#c=ZYw7C8Iw...

Can be run in "standalone" mode by adding standalone.html:

https://cesiumjs.org/Cesium/Build/Apps/Sandcastle/standalone.html#c=ZYw7C8Iw...

This will work regardless if the demo is loaded from the gallery, from a custom base64 string, or from a GitHub gist (and whatever else Sandcastle does that I am not aware of).

It also opens this mode instead of the blob URL when clicking "Open in new window" button. This fixes that button (which currently will not load ion imagery) as well as creating a way to view Sandcastle examples on mobile.

And best of all, it's a completely client side solution!

The only catch is it's kind of a hack. The standalone mode is actually just the same Sandcastle page with everything hidden and a few HTML edits. Ideally we'd have a responsive view but we don't, and this is the next best thing.

What is nice about this is that there's nothing to maintain. It's the exact same behavior/code.

Can standalone.html just be index.html with a query parameter?

Yeah but then it won't hide elements until JavaScript runs which will cause a very visible jitter and look ugly.

cesium-concierge · 2018-11-08T17:51:28Z

Thanks for the pull request @OmarShehata!

✔️ Signed CLA found.
❔ CHANGES.md was not updated.
- If this change updates the public API in any way, please add a bullet point to CHANGES.md.

Reviewers, don't forget to make sure that:

Cesium Viewer works.
Works in 2D/CV.
Works (or fails gracefully) in IE11.

I am a bot who helps you make Cesium awesome! Contributions to my configuration are welcome.

🌍 🌎 🌏

emackey · 2018-11-08T18:22:49Z

Interesting approach here! I think it's an improvement over the existing system.

One downside is that I used to always use "open in new window" to get the Sandcastle code into the top-level frame, for easier debugging. Yeah I know that the user can pick the "bucket.html" frame from a drop-down in the console, but, sometimes putting your own code in the top frame just makes life easier for debugging.

With that in mind, does standalone.html need so much structure? Could it be essentially an empty shell, like bucket.html, and inject the demo into itself, rather than injecting it into a child iframe?

The ideal situation I'm trying to replicate is what happens when you right-click a gallery item thumbnail and say "open in new tab", and it loads the gallery item directly into the top frame. That method doesn't work for hashes, of course, but it seems like it should be able to do something similar, loading the bucket at the top and injecting the hash's code.

OmarShehata · 2018-11-08T19:00:32Z

With that in mind, does standalone.html need so much structure? Could it be essentially an empty shell, like bucket.html, and inject the demo into itself, rather than injecting it into a child iframe?

That's exactly the approach I started with, and I agree that would be much nicer. I had to start stripping down CesumSandcastle.js into SandcastleStandalone.js since it relies a lot on the existence of these divs. It started to feel like I was rebuilding Sandcastle and I was running into issues with it not initiating the demo because it was waiting on some div. So I went back and tried this even simpler approach.

Part of it is my unfamiliarity with how Sandcastle dynamically loads and runs demos. If all we need to run the demo is to put the bucket in a div on the page, what's the iframe currently there for?

emackey · 2018-11-08T20:24:37Z

The iframe is just there to keep the inner workings of the demo fully separate from the inner workings of the Sandcastle editing environment. Your demo can damage the DOM or the global state inside the iframe and then reload from scratch without losing your work-in-progress in Sandcastle. The demo's global CSS styles are likewise firewalled away from Sandcastle's editor and toolbar. No need for any of these protections when opening a separate window.

So I think standalone.html could actually itself look quite similar to bucket-requirejs.html's bare contents, with an extra bit of JS to decode the query parameter and dump it into the page. You might be able to move the activateBucketScripts function to a helper file that is included by both Sandcastle and standalone, and call that function on the top-level document in the standalone version. You might have to pass in htmlEditor.getValue() as a parameter instead of allowing the function to ask for it directly. This function will take a bunch of "inert" script tags (ones that appeared as a result of setting head.innerHTML or such, which does not execute code), and remove and re-add them to the DOM programmatically, such that live JS code referenced by the script tags is loaded and executed.

So, my thinking is:

Something quite similar to bucket-requirejs.html loads first (page is blank).
The query parameter is decoded and dumped into the current document (both the HTML and JS parts). The page gains all of the DOM elements and script tags for the demo, but because the web works the way it does, none of the new script tags do anything yet.
activateBucketScripts runs, causing the script tags to come alive in the correct sequence.

OmarShehata · 2018-11-14T14:52:02Z

@emackey I was halfway through implementing this new architecture when I realized I was getting an error when adding the script tag before the HTML (because it was executing immediately). Which meant I didn't really need to do the activateBucketScripts at all as long as I wait for the added HTML to load (like in my Sandcastle CSS PR).

This works in Chrome, Edge, Firefox and IE11. This means we can just:

Something quite similar to bucket-requirejs.html loads first (page is blank).
Get the HTML from the query parameter and add it.
Once HTML is loaded, add the script tag.

And that's it. Am I missing anything here?

I'll clean this up and push it in the next few hours.

emackey · 2018-11-14T15:08:17Z

Sounds great!

OmarShehata · 2018-11-14T18:53:47Z

Ok, I've finished refactoring this. It's a lot simpler now, and definitely feels a lot faster I think without all the baggage of dojo/dijit in the new window!

I abstracted away a couple functions in Sandcastle-helpers.js. One thing I had trouble with was getting it to use the right version in the unbuilt/built versions. It does it now by using require for the unbuilt and just a normal script tag for the built, because doing:

<script type="text/javascript" src="../../Source/Cesium.js"></script>

Did not work (because that file needs require).

I had a bit of a revelation while working on this: I was getting the old Sandcastle CSS issue #5265 very consistently when adding the HTML first and then the JavaScript. Using the MutationObserver didn't actually work because waiting for the DOM element to be added wasn't enough, it seemed like it still had to load. I realized that just listening for:

window.addEventListener("load", function(event) {

Works perfectly. Which makes me wonder if that would just be a better way to do it in CesiumSandcastle.js as opposed to using the MutationObserver.

But for now, this should be clean and working. Let me know how this looks @emackey .

emackey

This is a great approach. Found one minor path issue, but I have a bigger problem: On my machine, the load event has already fired before the code sets up the callback. So I end up stuck on the "Loading..." screen indefinitely, and the script never loads. Win 10, Chrome, both NodeJS and IIS servers. No idea why.

Apps/Sandcastle/standalone.html

OmarShehata · 2018-11-17T06:02:42Z

Ok, so I was only able to recreate this on Firefox. Turns out the load event just gets fired too early (before the page really finishes loading). The problem isn't really that the HTML element doesn't fully load before the script runs (before the widgets are created by the script anyway), it's that the CSS imported in that HTML doesn't finish getting applied before the script runs.

There's no robust way to check for if the CSS has finished loading. Best I could find was this trick of declaring some CSS, and checking for when it has been applied in a loop. This now works on all the browsers I could find. Can you give it another try @emackey ?

emackey

This is a really awesome improvement. There's one weird edge case with paths in the "development" subfolder. Not sure what the best solution is, but I've never liked having those demos be a level down from others. Maybe they could move up to a "devGallery" or such alongside the main gallery, at the same path depth. Not sure, just an idea. Would like to see this get merged soon.

Apps/Sandcastle/CesiumSandcastle.js

Apps/Sandcastle/standalone.html

OmarShehata · 2018-11-19T19:16:49Z

Ok, @emackey , it's not the ideal solution (which I think would be just flattening everything in one folder and using the gallery-index.js to label the development ones, but that seems like too big of a refactor at the moment), but I got it to work by just adding a query parameter that alters the paths to have it to work.

I'm not very familiar with running the development examples, but it should be working now for development and normal examples.

Adjust relative paths

emackey

OK, I think this is ready. @mramato or @hpinkos did you want to take a final glance over this before it gets merged?

mramato · 2018-11-19T20:59:28Z

Did we try IE11? (I'm on Linux right now so I can't test)

mramato · 2018-11-19T21:00:15Z

Apps/Sandcastle/standalone.html

+        var width = getComputedStyle(loaderElement).getPropertyValue('width');
+        var done = false;
+        if (width === '12px') {
+            frameDelay --;


mramato · 2018-11-19T21:08:05Z

Why is gallery a query parameter instead of part of the hash? You normally don't use query parameters for things that don't involve the server.

mramato · 2018-11-19T21:13:57Z

Apps/Sandcastle/CesiumSandcastle.js

@@ -110,6 +111,8 @@ require({
    if (!defined(window.Cesium)) {
        window.Cesium = Cesium;
    }
+    // Used by Sandcastle-helpers.js


Why doesn't sandcastle-helpers use AMD syntax and include pako itself?

mramato · 2018-11-19T21:24:17Z

This does not appear to work at all with the built version of Sandcastle.

mramato · 2018-11-19T21:25:38Z

Did we try IE11? (I'm on Linux right now so I can't test)

Just tried IE11, it appears broken there as well (first it identified the click as a pop-up, but then I allowed popups and it still failed (or seemed to hang?)

mramato · 2018-11-19T21:27:33Z

Apps/Sandcastle/CesiumSandcastle.js

-        window.open(htmlBlobURL, '_blank');
+        var data = makeCompressedBase64String(htmlEditor.getValue(), jsEditor.getValue());
+        var url = getBaseUrl();
+        url = url.replace('index.html','') + 'standalone.html' + '?gallery=' + baseHref + '#c=' + data;


baseHref needs to be url encoded (that might be why it's broken on IE). query parameters need to be encoded with sandcastle-helpers (but as with my other comment, this shouldn't be a query parameter then it doesn't matter)

Not sure why we're sending an absolute url either, couldn't this just be relative to the current Sandcastle root?

OmarShehata · 2018-12-14T20:35:40Z

Ok, good news. I added a "compile bucket.css" step (f90f1c7) in the build process, which correctly fixes the CSS race conditions both in standlone and in the regular Sandcastle. It also greatly simplifies standalone.html because we don't have to do add a dummy HTML element and poll for the CSS to be added.

I tested all the combinations of built, unbuilt, for regular and development examples, for Chrome, Edge, IE11 and Firefox. The only issue with IE11 is when the URL is too long.

I spent too long trying to get Sandcastle-helpers.js to use AMD require syntax. The problem I ran into was that when built, requirejs isn't included, so I would need to separately build and include Sandcastle-helpers. However this is not done for CesiumSandcastle.js. Apparently the built version of Sandcastle does include require.js, and I wasn't sure what the best approach here was.

@emackey do you mind testing it one last time?

OmarShehata · 2018-12-26T18:08:51Z

@hpinkos @mramato do you think we can get this reviewed/merged in time for the next release?

mramato · 2019-01-03T14:48:48Z

Update filesToClean in gulpfile.js to include your new file.

mramato · 2019-01-03T14:54:50Z

This all sounds good to me. Not sure about bucketRaw.css as the name, why not keep it bucket.css and have the new one be called bucket.min.css or similar?

@emackey can you do the final test/merge here? I don't have access to IE11 at the moment and you know this code a lot better than me. Thanks.

(And thanks again @OmarShehata sorry for the delay)

OmarShehata · 2019-01-03T15:10:30Z

@mramato thanks for the review! I'll address your suggestions. For the naming, I remember doing this because I thought it otherwise wouldn't be backwards compatible (since all the Sandcastles in the wild will try to import bucket.css) right?

mramato · 2019-01-03T15:33:53Z

That sounds like a good reason, but @emackey may know better.

OmarShehata · 2019-01-03T17:56:29Z

I made sure the generated bucket css gets removed on npm run clean and I made a final test that it still works after rebuilding, and on IE11. @emackey let me know if you notice anything else in your tests.

emackey · 2019-01-03T18:57:01Z

What happened with Travis CI?

all the Sandcastles in the wild will try to import bucket.css right?

Correct.

Overall this looks good. It no longer works in my IIS subfolder, but seems to work fine when used with server.js as intended. Let's merge it once Travis is happy.

mramato · 2019-01-03T20:15:54Z

Overall this looks good. It no longer works in my IIS subfolder, but seems to work fine when used with server.js as intended. Let's merge it once Travis is happy.

This doesn't sounds right. Why doesn't it work under IIS? This shouldn't have a server component at all since it needs to work out of S3 as well.

OmarShehata · 2019-01-03T20:20:10Z

Travis is probably failing due to #7249. I restarted the build.

It does work for me when served from a simple static server (with python -m http.server).

emackey · 2019-01-03T21:56:26Z

This doesn't sounds right. Why doesn't it work under IIS?

There are absolute paths involved, or relative paths that go "too high". I serve Cesium out of a /Cesium subfolder. Using the python or node servers will serve it at the root level.

I spent a while this morning trying to narrow down exactly where the problem is. It affects the "open in new window" feature of both built and non-built Sandcastle, with different errors for each.

mramato · 2019-01-03T21:57:58Z

That definitely sounds like something we should fix before merging this, because it means our deployed branches won't work either.

emackey · 2019-01-03T21:58:01Z

@OmarShehata Try launching your python server in the parent folder, above the Cesium source tree.

emackey · 2019-01-03T21:59:09Z

gulpfile.js

+    var standaloneStream = gulp.src([
+        'Apps/Sandcastle/standalone.html'
+        ])
+        .pipe(gulpReplace('../../ThirdParty/requirejs-2.1.20/require.js', '/Build/CesiumUnminified/Cesium.js'))


Here's one suspect path. It looks server-root-relative.

Okay yeah, I can recreate it locally now.

OmarShehata · 2019-01-04T18:38:04Z

Ok so this was the issue:

Does it matter that in the built version I do /Build/CesiumUnminified/Cesium.js instead of ../../CesiumUnminified/Cesium.js ? I think the issue there is that the document head is added after that loads, so while ../../CesiumUnminified/Cesium.js is the correct URL, subsequent requests are incorrect. I'm not sure if there's an easy way around that.

from #7250 (comment)

This wasn't an issue for regular Sandcastle because it was still using requirejs in the built version (it's including some modules straight out of Source/). I thought The Right Way was to avoid require in the built, but re-doing it this way, and adding the base href early on, solves this issue and makes everything a little simpler. It's now a little closer to the way CesiumSandcastle.js works.

I've tested it on every combination of browser (IE11, Firefox and Chrome), built/unbuilt, hosted out of npm run start and in a subdirectory with python -m http.server. @emackey I am confident this should work (except for too long urls on IE11 issue). Please let me know if you notice anything else.

mramato · 2019-01-04T19:02:26Z

Just to confirm. With your changes, is the built version of Sandcastle using the built version of Cesium? Or is "Open in new Window" always using the built version? What is the exact behavior here.

The most important thing is that "Open in New Window" or Standalone mode uses the built version (either in all cases if it has to be that way, or at least when using the built Sandcastle)

OmarShehata · 2019-01-04T19:58:25Z

Sandcastle itself hasn't changed. It uses Source/Cesium in unbuilt and CesiumUnminified in built. Standalone mode now works this way as well. I confirmed this looking at the network tab as well.

What I meant by:

This wasn't an issue for regular Sandcastle because it was still using requirejs in the built version (it's including some modules straight out of Source/).

Was that even though built Sandcastle uses the built version of Cesium, it still includes some modules from Source/ (like defined). This is the current behavior in master/in production.

emackey · 2019-01-04T20:08:28Z

This works for me in IIS 👍

mramato · 2019-01-04T20:15:42Z

One last thing, update CHANGES

OmarShehata · 2019-01-04T20:19:57Z

@mramato I thought we don't put Sandcastle changes in CHANGES (#7152 (comment)). There's no change to the CesiumJS API here.

mramato · 2019-01-04T20:24:09Z

@mramato I thought we don't put Sandcastle changes in CHANGES (#7152 (comment)). There's no change to the CesiumJS API here.

That comment was regarding breaking changes, which for the most part don't apply to Sandcastle, we do mention new examples or functionality in Sandcastle when it happens. In this case it also fixes a bug because Open in New Window used to not work in many cases and also couldn't allow you to share the url, right?

OmarShehata · 2019-01-04T20:54:12Z

Makes sense. Should be ready now!

Omar Shehata added 2 commits November 8, 2018 12:11

Create static standalone sandcastle mode

887da68

Use standalone mode for open in new window

cdd3350

Simplify and refactor standalone sandcastle

73534fe

Eslint fix

7db0785

emackey suggested changes Nov 16, 2018

View reviewed changes

Apps/Sandcastle/standalone.html Outdated Show resolved Hide resolved

Fix path

3e710ed

Wait for CSS to load

a678381

emackey reviewed Nov 19, 2018

View reviewed changes

Apps/Sandcastle/CesiumSandcastle.js Show resolved Hide resolved

Apps/Sandcastle/standalone.html Outdated Show resolved Hide resolved

Make sure open in new window works for development examples

0914c4f

emackey and others added 2 commits November 19, 2018 14:57

Adjust relative paths

1871b93

Merge pull request #2 from emackey/standalone-patch-1

880ad06

Adjust relative paths

emackey approved these changes Nov 19, 2018

View reviewed changes

mramato reviewed Nov 19, 2018

View reviewed changes

Omar Shehata added 2 commits November 20, 2018 14:42

Fix standalone with built version

ca39c8d

Make sure standalone works in built/ie11

9711de3

Omar Shehata added 2 commits December 14, 2018 15:29

Make it work for built Sandcastle

bbc7df6

lint

d89ffbe

Clean generated bucket css

7f5398b

emackey reviewed Jan 3, 2019

View reviewed changes

Make sure standalone mode works when served from subdirectory

388d463

Omar Shehata added 2 commits January 4, 2019 15:34

Merge branch 'master' into standlone-sandcastle

e7685da

Update changes

3a4a4a9

Merge branch 'master' into standlone-sandcastle

ec62839

emackey merged commit 84ca98d into CesiumGS:master Jan 4, 2019

Create Sandcastle Standalone Mode #7250

Create Sandcastle Standalone Mode #7250

Conversation

OmarShehata commented Nov 8, 2018

Can standalone.html just be index.html with a query parameter?

cesium-concierge commented Nov 8, 2018

emackey commented Nov 8, 2018

OmarShehata commented Nov 8, 2018

emackey commented Nov 8, 2018

OmarShehata commented Nov 14, 2018

emackey commented Nov 14, 2018

OmarShehata commented Nov 14, 2018

emackey left a comment

Choose a reason for hiding this comment

OmarShehata commented Nov 17, 2018

emackey left a comment

Choose a reason for hiding this comment

OmarShehata commented Nov 19, 2018

emackey left a comment

Choose a reason for hiding this comment

mramato commented Nov 19, 2018

mramato Nov 19, 2018

Choose a reason for hiding this comment

mramato commented Nov 19, 2018

mramato Nov 19, 2018

Choose a reason for hiding this comment

mramato commented Nov 19, 2018

mramato commented Nov 19, 2018

mramato Nov 19, 2018

Choose a reason for hiding this comment

mramato Nov 19, 2018

Choose a reason for hiding this comment

OmarShehata commented Dec 14, 2018

OmarShehata commented Dec 26, 2018

mramato commented Jan 3, 2019

mramato commented Jan 3, 2019

OmarShehata commented Jan 3, 2019

mramato commented Jan 3, 2019

OmarShehata commented Jan 3, 2019

emackey commented Jan 3, 2019

mramato commented Jan 3, 2019

OmarShehata commented Jan 3, 2019

emackey commented Jan 3, 2019

mramato commented Jan 3, 2019

emackey commented Jan 3, 2019

emackey Jan 3, 2019

Choose a reason for hiding this comment

OmarShehata Jan 3, 2019 • edited Loading

Choose a reason for hiding this comment

OmarShehata commented Jan 4, 2019

mramato commented Jan 4, 2019

OmarShehata commented Jan 4, 2019

emackey commented Jan 4, 2019

mramato commented Jan 4, 2019

OmarShehata commented Jan 4, 2019

mramato commented Jan 4, 2019

OmarShehata commented Jan 4, 2019

OmarShehata Jan 3, 2019 •

edited

Loading