Fix usages of <div class="hidden"> containing code blocks#7922
Conversation
This comment has been minimized.
This comment has been minimized.
wbamberg
left a comment
wbamberg
left a comment
There was a problem hiding this comment.
Thanks so much for this @meowmeowmeowcat !
I did have some comments about some of the more difficult cases here, which we will need to look at before we can convert to Markdown. I did consider leaving this PR open in case you wanted to try to tackle some of them here. But since I'm going to be away for the first part of next week, I wouldn't be able to re-review it until I get back. So since there's nothing I can find that's actually wrong in this PR and it's a great step towards making the Web/API docs Markdownable, I'll merge it as it is.
Thanks again for this excellent contribution!
|
|
||
| <div class="hidden"> | ||
| <h6 id="Second_demo">Second demo</h6> | ||
| <div id="Second_demo"> |
There was a problem hiding this comment.
In Markdown we won't be able to do <div id="Second_demo">, so in a case like this it would be better to use a heading ID for the EmbedLiveSample call. Couldn't we just use the existing heading IDs here (like "Boundaries", "Acceleration", "Trailing effect")?
|
|
||
| <div class="hidden"> | ||
| <h6 id="Playable_code">Playable code</h6> | ||
| <div id="Playable_code"> |
There was a problem hiding this comment.
This is another case where we will not be able to do this in Markdown. But this case is more complicated, because it's using the hidden <h6> to select a particular bit of the code to use in the live sample, and ignore the other code blocks.
I don't like this example much - what's nice about live samples is that they show you the code and show you the result of that code. But this example shows some code but then uses some different code in a hidden block to product the output.
It does this of course because it wants to make the example editable. I'd be tempted to stop doing that, and just make this a normal live sample. But we could always do this in a follow-up, so I'm OK to keep this change as it is for now.
|
|
||
| <div class="hidden"> | ||
| <h6 id="Playable_code">Playable code</h6> | ||
| <div id="Playable_code"> |
There was a problem hiding this comment.
This is a similar case, except here it seems like the API isn't supported in Chrome or Firefox, so the example doesn't even work. And the API is deprecated so it doesn't seem likely that support would be added. Perhaps we should just make this one a static code block?
| <h6 id="Playable_code">Playable code</h6> | ||
|
|
||
| <pre class="brush: html"><canvas id="canvas" width="400" height="200" class="playable-canvas"></canvas> | ||
| <pre class="brush: html hidden"><canvas id="canvas" width="400" height="200" class="playable-canvas"></canvas> |
There was a problem hiding this comment.
This is weird, because the example is including the hidden code blocks but then not using them - instead the EmbedLiveSample call is referring to a different page (specifically to https://developer.mozilla.org/en-US/docs/Web/API/Canvas_API/Tutorial/Applying_styles_and_colors#a_demo_of_the_miterlimit_property). So I think we could just delete this hidden <div> and nothing would change...
| <div id="Playable_code"> | ||
|
|
||
| <pre class="brush: html"><canvas id="canvas" width="400" height="200" class="playable-canvas"> | ||
| <pre class="brush: html hidden"><canvas id="canvas" width="400" height="200" class="playable-canvas"> |
There was a problem hiding this comment.
Another tricky case where the code that is shown isn't actually used. Again I might consider making this a normal live sample. Or we could leave it as it is for this PR, and come back to it later.
|
|
||
| <div class="hidden" id="example2"> | ||
| <pre class="brush: js">CSS.paintWorklet.addModule('https://mdn.github.io/houdini-examples/cssPaint/intro/02partTwo/header-highlight.js');</pre> | ||
| <div id="example2"> |
There was a problem hiding this comment.
Another tricky case. This one also has a <div id="paintapi"> further up the page, and <div id=example3/4/5> below. Here the author's using <div> to select bits of code to include independent of the heading structure, which won't work in Markdown.
I think this page needs complete restructuring to work in Markdown, perhaps in which we create explicit sections for each example, with its own heading like "Live example".
Again I think it would be OK to deal with this in a follow-up PR if you like.
|
|
||
| <div class="hidden" id="registered"> | ||
| <pre class="brush: html"><button class="registered">Background Registered</button> | ||
| <div id="registered"> |
There was a problem hiding this comment.
Again we still need to remove this <div> for things to work in Markdown.
This one is. weird though, because the CSS block inside the <div> is exactly the same as the one above it, except it also has styles for the button. Couldn't we just:
- remove the duplicated styles from the hidden block, so it only contains the button styles
- remove the
<div id="registered"> - make the EmbedLiveSample call use the heading ID
|
|
||
| <div class="hidden"> | ||
| <h4 id="Hidden_example">Hidden example</h4> | ||
| <div id="Hidden_example"> |
There was a problem hiding this comment.
This is another difficult case, where I guess the visible HTML can't be used in the live sample because it contains the markers for the whitespace.
I think we should defer this for a later PR.
|
@wbamberg Thanks for your review! I'll open another PR for a follow-up on your suggestions. |
2 participants
Fixes #7901