-
Notifications
You must be signed in to change notification settings - Fork 22.5k
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
Add live sample specification #5773
Conversation
Preview URLsFlawsNone! 🎉 External URLsURL: No new external URLs (this comment was updated 2021-06-25 18:56:19.891464) |
<p>To create a live sample, writers use the <code>\{{EmbedLiveSample}}</code> KumaScript macro.</p> | ||
|
||
<p>To find the code blocks that should belong to a live sample, we consider that page headings (H2-H6) implicitly divide up the document into nested sections. So, for example:</p> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is overall really clear.
FWIW I found this diagram unhelpul/confusing at this point in the text. Might be better to move it just above the <h3>Examples</h3>
heading (and do the associated rewording).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I could see that. I would prefer not to move it later because this idea that headings section the document is fundamental to the algorithm here. But I think Daniel is going to have some suggestions for improving my ASCII art so I am optimistic this will get better :).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wbamberg stop raising expectations! 😆
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, now that I've built the suspense long enough, what do you think of something like this:
Topic (H1) 🔍 Final search scope
├── Introductory prose content ┃
│
├── Syntax (H2#Syntax) 🙅♀️ Unsearched
│ ├── Syntax code block ┆
│ │ ┆
│ ├── Parameters (H3#Parameters) ┆
│ │ └── Parameters prose content ┆
│ │ ┆
│ └── Return value (H3#Return) ┆
│ └── Return value prose content ┆
│
└── Examples (H2#Examples) 🔍 2nd search scope
└── Short example (H3#Example1) 🔍 1st search scope
├── Example prose content ┃
├── Example code block ┃
├── More prose ┃
└── {{EmbedLiveSample}} ┃
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really like the tree view here, I think it helps a lot to show the nesting. I think I might prefer to have the heading tag first, to make that more obvious, and I'm not sure we need to show the ID as well (since in Markdown it will always be the same, and in fact it looks wrong to have Short example (H3#Example1)
here).
I wanted the ASCII art to show just the sections, independent of the search algorithm, at least at first. I thought it would be helpful just to introduce this way of understanding how we can see a document as sectioned by headers, before talking about the algorithm. So I'm less keen on the right hand column here.
Or maybe we could show just the left hand part in the first diagram, and then talk about it, then talk through the algorithm, then have a second diagram showing both parts?
In the right-hand column as well, there is some strangeness between "Final search scope" and "Unsearched". In this document, with our algorithm, the H1 scope, "Introductory prose content", and even "Examples (H2#Examples)" are not searched, because the algorithm finds code blocks under "Short example (H3#Example1)". So it will actually stop there.
If you had this:
Topic (H1)
├── Introductory prose content
│
├── Syntax (H2#Syntax)
│ ├── Syntax code block
│ │
│ ├── Parameters (H3#Parameters)
│ │ └── Parameters prose content
│ │
│ └── Return value (H3#Return)
│ └── Return value prose content
│
└── Examples (H2#Examples)
└── H3 HTML
├── HTML code block
└── H3 JavaScript
├── JS code block
└── H3 Result
└── {{EmbedLiveSample}}
...then we would search Examples (H2#Examples) . If we had this weird thing:
Topic (H1)
├── Introductory prose content
└── H2 HTML
| └── HTML code block
│
├── Syntax (H2#Syntax)
│ │
│ ├── Parameters (H3#Parameters)
│ │ └── Parameters prose content
│ │
│ └── Return value (H3#Return)
│ └── Return value prose content
│
└── Examples (H2#Examples)
└── H3 Result
└── {{EmbedLiveSample}}
...then you would search the entire document.
I don't think there's any situation in which you would search at the root H1 and not search one of its children, which seems to be implied by "Final search scope" and "Unsearched".
I guess this kind of thing is why it seems helpful to introduce this in parts, describing the sectioning first and then the algorithm.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've had a go at updating this PR kind of along these lines.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think there's any situation in which you would search at the root H1 and not search one of its children, which seems to be implied by "Final search scope" and "Unsearched".
OK, this was a really good exercise. The problem was not with the diagram exactly, but with my understanding of how the algorithm worked. I'm happy with the new diagrams (except for the loss of emoji). ✅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just pushed 74e8d70, which shows both first and second search scopes. I did try adding the 🔍 but the one for "First search scope" broke the alignment of the |
for "Second (final) search scope". I'd be happy to add it if you know a way around that.
The problem was not with the diagram exactly, but with my understanding of how the algorithm worked.
So you were thinking that sibling sections would not get searched, or...? I'm a little worried that the algorithm wasn't clear enough to avoid a misunderstanding, either that the explanation isn't very good or that it's just too hard to understand.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd be happy to add it if you know a way around that.
Don't worry about that. The emoji bit was just some unsuccessful humor
The explanation isn't very good or that it's just too hard to understand.
No, this is not the case! I didn't read the original very closely and applied my own wishful thinking to the algorithm. I think it's a bit permissive for the algorithm to allow cousin sections to be searched. But forbidding cousin sections is a more complex algorithm, both as an implementation and as something to reason about as an author (particularly in a case where you've placed a live sample code block in a cousin section unintentionally).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the description is a lot clearer with this visualisation, so thanks for your comment.
unsuccessful humor
The best kind of humo(u)r :).
I think it's a bit permissive for the algorithm to allow cousin sections to be searched. But forbidding cousin sections is a more complex algorithm, both as an implementation and as something to reason about as an author (particularly in a case where you've placed a live sample code block in a cousin section unintentionally).
Yeah, exactly. I agree on both counts there. Cousin sections would be a weird thing to do and I think it would be very rare. But forbidding it would make the rule harder to understand.
@ddbeck , @hamishwillee , is this good to merge or are there any other changes needed? |
@wbamberg Just FYI the page displays for a few seconds then stops rendering in Firefox, Chrome, Edge: https://pr5773.content.dev.mdn.mozit.cloud/en-US/docs/MDN/Contribute/Markdown_in_MDN Dunno why. Will look at the content now. |
@@ -360,6 +360,187 @@ <h2>Heading IDs</h2> | |||
|
|||
<h2>Live samples</h2> | |||
|
|||
<p>In MDN writers can create "live samples" in which code blocks (HTML, CSS, JavaScript) in the page are built into a document that's displayed in the page in an iframe. In this way authors can show code and its output together, and the displayed code is the code used to generate the output so there's no risk of the code and the output getting out of sync.</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"built into a document" - do you mean document or something else "like code editor" etc?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps link then.
@@ -360,6 +360,187 @@ <h2>Heading IDs</h2> | |||
|
|||
<h2>Live samples</h2> | |||
|
|||
<p>In MDN writers can create "live samples" in which code blocks (HTML, CSS, JavaScript) in the page are built into a document that's displayed in the page in an iframe. In this way authors can show code and its output together, and the displayed code is the code used to generate the output so there's no risk of the code and the output getting out of sync.</p> | |||
|
|||
<p>To create a live sample, writers use the <code>\{{EmbedLiveSample}}</code> KumaScript macro.</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove KumaScript ?
└── \{{EmbedLiveSample}} | ||
</pre> | ||
|
||
<p>In this example there are eight sections, one for each heading. The sections defined by the H2 headings contain the sections defined by the H3 headings under them, and the top-level section defined by the H1 heading contains the entire document.</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe remove this - the treeview above is clear enough it isn't actually needed. All it did was make me wonder if H1 was actually a section, and therefore whether we had 8 or 7 sections.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead, you could perhaps summarise the "rule" you detail below. What I mean is a sentence that says something like "The macro determines what HTML and Javascript files to use based on the relative nesting levels of code blocks and the macro"
<ol> | ||
<li>Define the <em>immediate section</em> as the most-nested section that directly contains the macro call itself.</li> | ||
<li>Look for any code blocks in the immediate section. If you find any, they are the code blocks to use. Stop looking.</li> | ||
<li>If you don't find any, go to the next level up, and look for any code blocks in that section. If you find any, they are the code blocks to use. Stop looking.</li> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Slight ambiguity is that I might think "in immediate section" meaning just at this level, and not at this level and in any subsections of this level. So perhaps "Look for any code blocks in the immediate section (including any sub-sections)"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note, this is absolutely captured by the example below, so just a minor point.
<p>For example, in the case above, the macro call is in <code>H3 Result</code>, so we would: | ||
<ol> | ||
<li>look in <code>H2 Examples > H3 Result</code>, and not find any code blocks</li> | ||
<li>go up to <code>H2 Examples</code>, and look there</li> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In support of https://github.com/mdn/content/pull/5773/files#r668347562 perhaps ", and look there" should be ", and look in that section (and all subsections)
<li>the one under <code>H3#Example2</code> contains <code>JS-code-block-2</code> and <code>CSS-code-block-2</code></li> | ||
</ul> | ||
|
||
<p>Another common pattern is where the macro call wants to use code blocks in a sibling section:</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"the macro call wants to use code blocks" > "the macro call uses code blocks"
|
||
<p>Another common pattern is where the macro call wants to use code blocks in a sibling section:</p> | ||
|
||
<pre class="brush: html"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW Change from outline style to HTML style example. I prefer consistency (and outline style)
@wbamberg I've put a bunch of minor comments that I think would be improvements. The operative word is "minor". This is really useful and I would take no offence if you just merged. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple of questions about the definition of section and immediate section, though not strictly blocking. Thank you, Will!
<p>Given this model, the rule for finding the code blocks to include in a live sample is:</p> | ||
|
||
<ol> | ||
<li>Define the <em>immediate section</em> as the most-nested section that directly contains the macro call itself.</li> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have a definition of section, separately of the immediate section? It might add a lot of clarity to describe what a section is exactly.
Based on the examples, I think it means the content following a heading, up to but not including the next same-level heading or the end of the page, whichever comes first. Though this raises another question: does this proposal imply that every page will have an H1? Will that be written out in Markdown sources? Or do we have an implicit section for "any content preceding the first H2" or "the entirety of the page, including any content preceding the first heading"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have a definition of section, separately of the immediate section? It might add a lot of clarity to describe what a section is exactly.
Based on the examples, I think it means the content following a heading, up to but not including the next same-level heading or the end of the page, whichever comes first.
Almost, but more like "the content following a heading, up to but not including the next same-or-higher-level heading or the end of the page, whichever comes first. "
Though this raises another question: does this proposal imply that every page will have an H1? Will that be written out in Markdown sources? Or do we have an implicit section for "any content preceding the first H2" or "the entirety of the page, including any content preceding the first heading"?
Yari makes the (front matter) title of the page into the H1 heading, and all top-level headings in content are I think expected to be H2.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost, but more like "the content following a heading, up to but not including the next same-or-higher-level heading or the end of the page, whichever comes first. "
Right! 👍
Yari makes the (front matter) title of the page into the H1 heading, and all top-level headings in content are I think expected to be H2.
In that case, for authors, there's an implicit H1 (and content that it's a part of of), which we should probably mention so they can understand that the algorithm is about the structure of the HTML page generated out of Markdown?
I’m not sure what this PR is waiting on currently, but given that it was last updated on July 16th, it seems like it might benefit from some attention at getting it moved along (and getting the merge conflict resolved) |
Closing this because MD conversion will rot it. I'll file a new PR to document live samples. |
This updates the "MDN in Markdown" page with a specification for handling live samples, as discussed in #3548.
It doesn't really belong in the Markdown spec and should perhaps eventually move somewhere else, but it seems useful to keep all this together for now.