-
Notifications
You must be signed in to change notification settings - Fork 509
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
fix(build): fix iframe title + div ids #11344
base: main
Are you sure you want to change the base?
Conversation
- Don't mututate the DOM in collectClosestCode. We'll use the same title as with regual live samples. - Strip suffix from iframe height height="400px" => height="400". - Don't add ids to divs?! Currnet we add ids to notes by accident: https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_grid_layout/Masonry_layout#sect1
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.
LGTM, but the warning message could be clearer.
Also: It would be nice for our translators to run a full build once, gather all the warnings, and put them as a checklist in an issue in translated-content, so that they have a chance to look at the affected live samples.
@@ -16,7 +16,7 @@ <h2 id="Flexbox">Flexbox</h2> | |||
and also tests that prerequisites are handled as part of the rendering | |||
and live-sample-building process. | |||
--> | |||
<div id="Flex_1">{{ Page("Learn/CSS/CSS_layout/Introduction/Flex/Stuff", "Flex_1") }}</div> | |||
<section id="Flex_1">{{ Page("Learn/CSS/CSS_layout/Introduction/Flex/Stuff", "Flex_1") }}</section> |
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.
@fiji-flo Do you know whether there are any occurrences of <div id="...">
outside of code blocks in content or translated-content that might have been added intentionally and are used to link to them?
Does rari check that the targets of anchor links are actually defined? I believe yari does not, but this would probably be useful, as links to anchors that don't exist are essentially dead links.
This pull request has merge conflicts that must be resolved before it can be merged. |
This pull request has merge conflicts that must be resolved before it can be merged. |
Summary
How did you test this change?
Locally