Skip to content

Fix broken playgrounds (and some improvements) in CSS positioning tutorial #39879

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

steffahn
Copy link

Description

Fix broken code examples on https://developer.mozilla.org/en-US/docs/Learn_web_development/Core/CSS_layout/Positioning. And add some improvements.

Motivation

Look e.g. at the current status around https://developer.mozilla.org/en-US/docs/Learn_web_development/Core/CSS_layout/Positioning#introducing_top_bottom_left_and_right, the linked playground there contains code such as

top: 30px;
left: 30px;

body {
  width: 500px;
  margin: 0 auto;
}

p {
  background: aqua;
  border: 3px solid blue;
  padding: 10px;
  margin: 10px;
}

span {
  background: red;
  border: 1px solid black;
}

.positioned {
  position: relative;
  background: yellow;
  top: 30px;
  left: 30px;
}

because more code snippets are aggregated into this than what the author anticipated. And this is then of course broken (and results in the body { … } rule to be ignored entirely).

Also, there are multiple places where a textual description of a very interesting and very significant for understandingposition intermediate state without any visualization of what happens.

Thus I have added 3 such visualizations / playground states, which look as follows:

A displayed version of the static code example

Bildschirmfoto_20250610_035221

The not-yet z-index flipped state, for visual contrast

Bildschirmfoto_20250610_035301

The not-yet margin-top-fixed state for fixed positioning, also for visual contrast

Bildschirmfoto_20250610_035313

Additional details

Additionally there are also some minor tweaks here and there. E.g. a 0px becoming 0 for consistency with the snippet that was already displayed; some margin-based height extension for sticky examples using 100vh so now it works on the page and also on the linked playground. And probably more.

I probably overdid it a bit with the de-duplicating of shared parts of code snippets. But I've iterated on this long enough now, so I'll just submit the PR now. One user-facing benefit of this piecework is that now also these shortened snippets have gained links to the corresponding playgrounds, e.g.:
Bildschirmfoto_20250610_035935

Also, the newlines (or lack therof) for some of the generated playgrounds without added things such as single empty lines at the end, or double empty lines at the beginning of some of the hidden code blocks was a bit tricky to work with/around1.

Footnotes

  1. especially since you guys’s prettier tooling tended to somewhat silently simply erase all this work from history as soon as one wants to finish their commit. (And I really do mean erase since the way it's in a pre-commit hook which appears to then also add the changes to the work tree, and the staging area and the current commit means the original version of how things were is relatively thoroughly fully eliminated :-/]

@steffahn steffahn requested a review from a team as a code owner June 10, 2025 02:06
@steffahn steffahn requested review from chrisdavidmills and removed request for a team June 10, 2025 02:06
@github-actions github-actions bot added Content:Learn Learning area docs size/m [PR only] 51-500 LoC changed labels Jun 10, 2025
@steffahn steffahn force-pushed the positioning_tutorial branch from 17a1d7f to d72f06f Compare June 10, 2025 02:19
@steffahn steffahn changed the title Positioning tutorial Fix broken playgrounds (and some improvements) in CSS positioning tutorial Jun 10, 2025
Copy link
Contributor

Copy link
Contributor

@chrisdavidmills chrisdavidmills left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi there @steffahn, and thank you for the significant amount of time and effort you've put into this. On long complex pages like this one, it is always painful when we end up with code examples bleeding into one another because of the heading nesting not being quite what was expected.

This page was written before we implemented the live-sample___sample-name code fence syntax. For new pages, I use it all the time to avoid just such problems.

I read through the page in detail, and what you've done works nicely. I've only made one specific comment where I noticed that what was being rendered in one live sample didn't seem quite right.

However, I have got a more general comment to made — I think you have gone a bit far with the deduplication of code — putting most of the code in one giant chunk near the top of the page will make it quite hard for future page editors to understand and maintain. Do you think you can re-duplicate the main chunks back to the sections they are relevant to?

Obviously, I still want you to keep the live-sample___sample-name syntax, to avoid the code leakage, as that is super useful.

Let me know what you think, and thanks again!

background: yellow;
top: 30px;
left: 30px;
}
```

{{ EmbedLiveSample('Setting_position_absolute', '100%', 450) }}
If you now save and refresh, you should see something like so:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code shown in the next live sample isn't quite right. The positioned box says "Now I'm absolutely positioned relative to the element, not the element!", but it shouldn't, as we haven't got to that stage of changing the positioning context yet.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh that's a good catch!!; I missed that! I only saw the "Now I'm absolutely positioned…" start and thought it applied to both. (On the currently live version it does instead still retain the “By default we span 100” text, which was also wrong, and inconsistent with the linked version at https://mdn.github.io/learning-area/css/css-layout/positioning/3_absolute-positioning.html .

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, yeah, we should probably update those versions too, when the dust settles.

```

If you now save and refresh, you should see something like so:
<!-- prettier-ignore-start -->
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't have to do this. It's fine for the concatenated code to have nonsensical formatting, but the displayed blocks should have sane indentation (i.e. not start at non-zero)

@steffahn
Copy link
Author

However, I have got a more general comment to made — I think you have gone a bit far with the deduplication of code — putting most of the code in one giant chunk near the top of the page will make it quite hard for future page editors to understand and maintain. Do you think you can re-duplicate the main chunks back to the sections they are relevant to?

Yeah, I'll give it another go to re-simplify a bit.

@bsmth bsmth added the awaiting response Awaiting for author to address review/feedback label Jul 10, 2025
@Josh-Cena
Copy link
Member

@steffahn This is still waiting for your update

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting response Awaiting for author to address review/feedback Content:Learn Learning area docs size/m [PR only] 51-500 LoC changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants