Fix JavaScript example roulette by wilzbach · Pull Request #1757 · dlang/dlang.org

wilzbach · 2017-06-21T15:33:12Z

As discussed here, to avoid page reflows, we should pick an example ASAP and render its HTML.

Unfortunately this required quite a bit of work as the original code heavily used jQuery for which we don't want to wait until it's loaded. I am not really happy with it, but I guess that's the necessary evil if we want to avoid the ugly page redraws (and don't have a server to do the selection).

I think it would be nice to have a drop down, like on the golang page. I remember back some time ago when I wanted to show one particular example to a friend that I had to reload the page a couple of times to get to it.

That was fairly trivial to add and requiring a title for an example and reduces the manual checking from @CyberShadow ;-)

The example selector still needs styling, hence I am marking this as "Needs work" for now.

CyberShadow · 2017-06-21T16:11:51Z

index.dd

+// Picking the frontpage example after the page has loaded leads to reflows
+// and rerendering. Hence, we pick the example as soon as possible
+/**
+Bootstrap a parent node containing a <pre> element into an editor instance.


What does "bootstrap" mean here?

CyberShadow · 2017-06-21T16:12:26Z

index.dd

+        return btn;
+    }
+    // helper method to construct a textarea in vanilla JavaScript
+    function createTextArea(className, value, text) {


This helper function is used only once, inline?

CyberShadow · 2017-06-21T16:12:49Z

index.dd

+        return el;
+    }
+    // helper method to construct an editor overlay in vanilla JavaScript
+    // typically overlays are output,args,stdin and the editor instance


What does "overlay" mean here?

Everything that "lays over" the code box, e.g.

As mentioned, there weren't any comments, so I have chosen my own terminology. Better ideas?

CyberShadow · 2017-06-21T16:13:40Z

index.dd

+        el.appendChild(inner);
+        return el;
+    }
+    // JavaScript filter for humans


Pretty sure this function is not used for filtering humans. :P

CyberShadow · 2017-06-21T16:37:11Z

Good stuff. I hadn't thought of actually putting the code inline, but this approach does have merits. I forgot we're already loading all our JavaScript at the end of the HTML, so there will be no libraries to use at that point. I wonder though if it's possible to reduce code duplication (if there is any) and the number of things that are in index.dd, e.g. by putting the JavaScript in a separate file and including it as a DDoc macro (e.g. js/index-examples.js.ddoc).

It's not immediately obvious from the code (and I haven't looked at it in depth) whether this is what it already does, but doing just the bare minimum to show the example would be the goal for the inline JavaScript code. Showing the buttons and getting syntax highlighting for the editor working can be a task for the rest of the JavaScript code that's set up. The inline code's goal is to just do the bare minimum for the layout (ideally even avoid a re-layout later) and hand things over to the main code once it's loaded.

wilzbach · 2017-06-21T22:10:30Z

I wonder though if it's possible to reduce code duplication (if there is any)

The first commit is a more or less literal translation of the code in js/run.js.
Sadly the only comment in there is from me (when I added an improvement to it).
Hence, some of the comments I added for better understanding might have a confusing terminology.

and the number of things that are in index.dd, e.g. by putting the JavaScript in a separate file and including it as a DDoc macro (e.g. js/index-examples.js.ddoc).

I thought about this as well, but I wasn't sure whether it's worth the extra step.
OTOH this would allow us to minify/uglify the inline JavaScript.

It's not immediately obvious from the code (and I haven't looked at it in depth) whether this is what it already does, but doing just the bare minimum to show the example would be the goal for the inline JavaScript code.

Yes, that was my goal.

Showing the buttons and getting syntax highlighting for the editor working can be a task for the rest of the JavaScript code that's set up.

You need the buttons - they will trigger a reflow as well ;-)
The code just creates an empty textarea into which the existing setupTextarea injects the CodeMirror editor.

The inline code's goal is to just do the bare minimum for the layout (ideally even avoid a re-layout later) and hand things over to the main code once it's loaded.

Yes it does exactly this - I basically moved the entire layout setup function inline.

wilzbach · 2017-06-21T23:58:28Z

The example selector still needs styling, hence I am marking this as "Needs work" for now.

Done. I think this is styling should get us started:

e.g. by putting the JavaScript in a separate file and including it as a DDoc macro (e.g. js/index-examples.js.ddoc).

Done. I also tried to clean the script a bit further. For example, only the pre elementNode (i.e. the one with the code) is passed in and touched now.

CyberShadow · 2017-06-22T03:07:12Z

Looks nice :) @andralex What do you think?

CyberShadow · 2017-06-22T03:45:56Z

You need the buttons - they will trigger a reflow as well ;-)

Why? Just reserve space for them.

I don't understand why the code needs to do anything beyond displaying one of the divs that are hidden by default.

PetarKirov · 2017-06-24T09:17:10Z

index.dd

    D=<span class="d_inlinecode">$0</span>
    EXAMPLE=$(TAG2 a, id="a$1-control" class="example-control", )$(TAG2 div, id="a$1" class="example-box", $(RUNNABLE_EXAMPLE $2))
-    EXTRA_EXAMPLE=<div class="your-code-here-extra" style="display:none">$(RUNNABLE_EXAMPLE $0)</div>
+    EXTRA_EXAMPLE=<div class="your-code-here-extra" style="display:none">


EXAMPLE_ROULETTE_ENTRY sounds better to me.

PetarKirov · 2017-06-24T09:25:40Z

index-examples.js.ddoc

+Be careful, Ddoc forbids certain patterns, e.g.:
+- using < or >
+- direct assignments (foo = bar) are interpreted as new Ddoc variables
+)


What prevents adding this as a plain js file under e.g. js/example_roulette.js? ~~Can ddoc string-import-mixin other files?~~
Is loading an external file via <script src="${DLANG_SITE}/js/example_roulette.js" type="text/javascript"></script> not an option? Or alternatively, mix the js script in, after ddoc processing, as part of the build.

PetarKirov · 2017-06-24T09:51:45Z

Overall I really like the improvements in this PR (as an end-user), especially the added <select> box!
As an improvement, I would like to propose creating a homepage-examples folder so we can add plain *.d files inside it. This should make it easier for people not well versed with ddoc to contribute and it should be easier to maintain. golang also does it like that: https://golang.org/doc/play/, however the list of examples is hard-coded and they have the first example in-line (I guess in order to improve the page load time / prevent reflows), see https://github.com/golang/go/blob/master/doc/root.html#L35.
What's left is to decide whether the examples should be gathered as part of the build process or should fetch them dynamically with JS (like golang does for the rest of the examples).

dlang-bot · 2017-06-24T09:51:52Z

Thanks for your pull request, @wilzbach!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

PetarKirov · 2017-06-24T09:52:23Z

Sorry, for the close - wrong button :(

wilzbach · 2017-06-24T21:20:29Z

Is loading an external file via <script src="${DLANG_SITE}/js/example_roulette.js" type="text/javascript"></script> not an option?

I thought about this for a while and while this approach has the disadvantage of potentially being a bit slower, it allows to use a proper JavaScript file (syntax-highlighting) and avoids the constant fights against Ddoc when writing inline JS.
Moreover, browsers do peek through the entire DOM and look for source tags while downloading the found ones in parallel.

The simulated page load waterfall for "Fast 3G" shows this nicely:

Of course the simulation, isn't perfect, but it does show that a lot of other stuff needs to be download before the page can be initially drown. DOMContentLoaded (the blue line) is at 4.13s

Or alternatively, mix the js script in, after ddoc processing, as part of the build.

This would add another step to the build process and currently @CyberShadow is opposed to this (see e.g. #1688 (comment)).

wilzbach · 2017-06-24T21:21:57Z

As an improvement, I would like to propose creating a homepage-examples folder so we can add plain *.d files inside it. This should make it easier for people not well versed with ddoc to contribute and it should be easier to maintain.

Excellent idea! But let's do this in another PR. This one is already big enough.

PetarKirov · 2017-06-25T07:40:59Z

js/example_roulette.js

+        el.appendChild(titleEl);
+        el.appendChild(document.createElement("br"));
+        var inner = document.createElement(type);
+        inner.className = className;


So, the returned element and the inner "type" element have same class. Is that on purpose and why?

Yes, because I tried to make a 1:1 copy of the jQuery version, see e.g. https://github.com/dlang/dlang.org/blob/master/js/run.js#L138

stdin = '<div class="d_code_stdin"><span class="d_code_title">Standard input</span><br>' + '<textarea class="d_code_stdin">'+stdin+'</textarea></div>';

Imho "translation" and refactoring shouldn't be mixed (even though the temptation is very strong).

CyberShadow · 2017-06-25T08:30:22Z

OK, so, as I understand, this PR does three things right now:

Fix the JS roulette
Select an example during page load instead of after, to avoid a flash of the default noscript example
Add a dropdown to select an example

IMO these should be separate PRs, or at least commits. Also I still have a strong suspicion that the second one could be much simpler as mentioned above.

PetarKirov · 2017-06-25T09:30:48Z

@wilzbach something in this PR somehow broke the layout, see:

Before:

After:

(That's with Chrome 58 on Windows 10.)

Last time I tried the DAutoTest preview (I think it was yesterday) I didn't notice any layout problems.

wilzbach · 2017-06-25T09:36:44Z

Fix the JS roulette

-> #1767

Select an example during page load instead of after, to avoid a flash of the default noscript example
Add a dropdown to select an example

Hehe. I initially had separate commits, but then squashed because it easier due to the several steps (inline Ddoc -> Ddoc as separate file -> separate Js).

I for now simply dropped the commit.
I will do this as follow-up

@wilzbach something in this PR somehow broke the layout, see:

Yeah, I realized this myself. Already fixed. Sorry.

wilzbach · 2017-06-25T10:26:29Z

js/example_roulette.js

+// step 2: select the first <pre> element (of the .runnable-example <div>)
+var el = findFirst(rouletteChildNode.children, function(e) { return e.nodeName == "PRE";});
+// step 3: Construct runnable example layout within the <pre> element
+buildRunnableExampleLayout(el);


@CyberShadow

Also I still have a strong suspicion that the second one could be much simpler as mentioned above.

Yes, these lines without buildRunnableExampleLayout would - in theory - be enough. However, it still "jumps" due to the buttons added and just

adding the buttons isn't a good solution imho, because (1) it creates code entanglement (at another random, JS place the script needs to be aware of this) and (2) it creates code redundancy and makes it a lot harder if someone wants to change the overall layout

adding sth. el.style.marginBottom = "20px" is a similar hack (and still flashes/"jumps")

wilzbach · 2017-06-27T04:59:28Z

Rebased.

@CyberShadow let's come to a conclusion here: are you okay with this now or do you want me to come up with an ugly hack that "fakes" the bottoms with sth. like marginBottom? As explained I would prefer to have one source of truth for the layout generation.

CyberShadow · 2017-06-27T07:19:26Z

Less code is better. There is way too much code with a bus factor of 1 on dlang.org already. As far as I can see, index.dd just needs like 5 lines of JavaScript, at most. If you don't feel like doing that, I can put it on my list.

wilzbach · 2017-06-27T07:38:32Z

Less code is better. There is way too much code with a bus factor of 1 on dlang.org already. As far as I can see, index.dd just needs like 5 lines of JavaScript, at most. If you don't feel like doing that, I can put it on my list.

Opened another PR with this approach

#1775

As mentioned the redraw flashes are still partially visible ...

wilzbach · 2017-06-30T21:54:24Z

Closing this with regrets as it wasn't trivial to refactor the jQuery code, but in the end the smaller diffs won.

wilzbach added the Needs Work label Jun 21, 2017

CyberShadow reviewed Jun 21, 2017

View reviewed changes

wilzbach force-pushed the fix-js-roulette branch 2 times, most recently from c78067a to c966202 Compare June 21, 2017 23:50

wilzbach removed the Needs Work label Jun 21, 2017

wilzbach force-pushed the fix-js-roulette branch from c966202 to a89a91b Compare June 21, 2017 23:55

wilzbach added the Frontpage example label Jun 22, 2017

PetarKirov reviewed Jun 24, 2017

View reviewed changes

PetarKirov closed this Jun 24, 2017

PetarKirov reopened this Jun 24, 2017

wilzbach force-pushed the fix-js-roulette branch from a89a91b to 1c800be Compare June 24, 2017 21:07

PetarKirov reviewed Jun 25, 2017

View reviewed changes

wilzbach mentioned this pull request Jun 25, 2017

Temporarily fix the example roulette #1767

Merged

wilzbach force-pushed the fix-js-roulette branch from 1c800be to 7c6ca2e Compare June 25, 2017 09:32

wilzbach commented Jun 25, 2017

View reviewed changes

wilzbach force-pushed the fix-js-roulette branch from 7c6ca2e to f478127 Compare June 27, 2017 04:55

Fix JavaScript example roulette

f478127

wilzbach mentioned this pull request Jun 27, 2017

Fix redraw flashes at the JavaScript example roulette #1775

Merged

wilzbach closed this Jun 30, 2017

Uh oh!

Conversation

wilzbach commented Jun 21, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CyberShadow commented Jun 21, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wilzbach commented Jun 21, 2017

Uh oh!

wilzbach commented Jun 21, 2017

Uh oh!

CyberShadow commented Jun 22, 2017

Uh oh!

CyberShadow commented Jun 22, 2017

Uh oh!

PetarKirov Jun 24, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PetarKirov Jun 24, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PetarKirov commented Jun 24, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dlang-bot commented Jun 24, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Bugzilla references

Uh oh!

PetarKirov commented Jun 24, 2017

Uh oh!

wilzbach commented Jun 24, 2017

Uh oh!

wilzbach commented Jun 24, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CyberShadow commented Jun 25, 2017

Uh oh!

PetarKirov commented Jun 25, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wilzbach commented Jun 25, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wilzbach commented Jun 27, 2017

Uh oh!

CyberShadow commented Jun 27, 2017

Uh oh!

wilzbach commented Jun 27, 2017

Uh oh!

wilzbach commented Jun 30, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

CyberShadow commented Jun 21, 2017 •

edited

Loading

PetarKirov Jun 24, 2017 •

edited

Loading

PetarKirov Jun 24, 2017 •

edited

Loading

PetarKirov commented Jun 24, 2017 •

edited

Loading

dlang-bot commented Jun 24, 2017 •

edited

Loading

PetarKirov commented Jun 25, 2017 •

edited

Loading