-
Notifications
You must be signed in to change notification settings - Fork 22.9k
Move "Whitespace" article from DOM to CSS #40523
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
Conversation
This pull request has merge conflicts that must be resolved before it can be merged. |
7ce203f
to
c9751eb
Compare
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.
Hi @Josh-Cena, I haven't completed the review but I wanted to share my thoughts so far.
Overall, I agree with the consolidation and moving the guide under CSS_text.
- Firstly, what do you think about organizing the guide like this:
## What is whitespace?
## What happens to whitespace? (moved this up)
## How does HTML process whitespace? (reworded the title)
## How does CSS process whitespace? (changed to h2)
### Collapsing and transformation
### Trimming and positioning
### Whitespace processing between inline and inline-block elements (changed to h3 and also reworded the title)
## How is whitespace processed in the DOM? (reworded the title)
Currently, the HTML section seems stuck between the "What is whitespace?" and "What happens to whitespace?" sections. So there is a somewhat break in the flow of ideas. Also, grouping those three subsections under a CSS-specific section will help to better convey the direction of the content in those subsections.
- I'd also suggest removing the section "## Collapsing of white space" from white-space-collapse and pointing it to this guide (to keep one place as the source of truth). Then, the "collapse" link in the normal value of white-space can also be linked to this guide. That section "## Collapsing of white space" actually is a good summary of the steps in this guide, so we could maybe port it over at the end of the CSS sections, something like "Whitespace processing steps in a nutshell" or "Summary of whitespace processing"?
I can add a more detailed review of the guide after I hear what you think.
(oh and the new guide link can be added to the sidebar)
files/en-us/web/api/document_object_model/using_the_document_object_model/index.md
Outdated
Show resolved
Hide resolved
I've been looking at this PR too although I've not properly organized my thoughts on it yet. If you like though I am happy to add my comments alongside Dipika's. |
That would be great :) |
487528f
to
43f39fa
Compare
Thanks @dipikabh those are great suggestions |
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.
Sending edits I have so far
> [!NOTE] | ||
> [Firefox DevTools](https://firefox-source-docs.mozilla.org/devtools-user/index.html) have supported highlighting text nodes since version 52, making it easier to see exactly what nodes whitespace characters are contained within. Pure whitespace nodes are marked with a "whitespace" label. | ||
> [Firefox DevTools](https://firefox-source-docs.mozilla.org/devtools-user/index.html) supports highlighting text nodes, making it easier to see exactly what nodes whitespace characters are contained within. Pure whitespace nodes are marked with a "whitespace" label. |
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.
> [Firefox DevTools](https://firefox-source-docs.mozilla.org/devtools-user/index.html) supports highlighting text nodes, making it easier to see exactly what nodes whitespace characters are contained within. Pure whitespace nodes are marked with a "whitespace" label. | |
> [Firefox DevTools](https://firefox-source-docs.mozilla.org/devtools-user/index.html) supports highlighting text nodes, making it easier to see exactly which nodes contain whitespace characters. Pure whitespace nodes are marked with a "whitespace" label. |
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.
Not sure this is still true. I don't see the whitespace nodes in the demo: https://firefox-dev.tools/devtools-examples/whitespace-only-demo/index.html.
|
||
### Whitespace in block formatting contexts | ||
In a nutshell, different whitespace characters are collapsed in the following fashion: |
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 a nutshell, different whitespace characters are collapsed in the following fashion: | |
In a nutshell, different whitespace characters are collapsed and transformed in the following fashion: |
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.
Honestly I don't think this "in a nutshell" bit is very useful. It recaps the previous list of steps, but in a less precise and (to me) easy to follow way. Does it add anything else?
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, This is ported from the white-space-collapse
page, and it does present things in a case-by-case way instead of a step-by-step way which may be useful to certain people.
2dba4fb
to
662c2a7
Compare
fa7490b
to
977e5c0
Compare
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.
Few final touch-up suggestions, but leaving my +1.
Great work @Josh-Cena! 👏
Also pinging @wbamberg to see if he'd like to add anything more.
</body> | ||
``` | ||
|
||
The three empty lines we now have are not going to occupy any space in the final layout, because they don't contain any visible content. So we'll end up with only two lines taking up space in the page. People viewing the web page see the words "Hello" and "World!" on two separate lines, just as you'd expect two `<div>`s to be laid out. Browsers essentially ignore all of the whitespace that was included in the HTML code. |
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 para needs to be indented
The three empty lines we now have are not going to occupy any space in the final layout, because they don't contain any visible content. So we'll end up with only two lines taking up space in the page. People viewing the web page see the words "Hello" and "World!" on two separate lines, just as you'd expect two `<div>`s to be laid out. Browsers essentially ignore all of the whitespace that was included in the HTML code. | |
The three empty lines we now have are not going to occupy any space in the final layout, because they don't contain any visible content. So we'll end up with only two lines taking up space in the page. People viewing the web page see the words "Hello" and "World!" on two separate lines, just as you'd expect two `<div>`s to be laid out. Browsers essentially ignore all of the whitespace that was included in the HTML code. |
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.
Again, this is wrapping up the whole algorithm and saying what the final result is, and it is not a step of the processing 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.
It's still talking about the example we've been discussing in the steps though. Even if wrapping up to say the result, I think it is a continuation of the previous context.
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.
Each step in the list is a job to be done and corresponds to one step in the browser. This paragraph isn't really a job to be done, but rather the explanation of the end effect. Putting this under the list would make it slightly harder to compare with the spec.
|
||
- `preserve` and `break-spaces`: The whole algorithm is skipped except for step 3, so no whitespace collapsing or transformation happens. | ||
- `preserve-spaces`: The whole algorithm is skipped, so whitespace characters at the start and end of lines are preserved. | ||
- `preserve-breaks`: the algorithm is the same as `collapse`. |
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.
- `preserve-breaks`: the algorithm is the same as `collapse`. | |
- `preserve-breaks`: The algorithm is the same as [collapsing](#collapsing_and_transformation)). |
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 doesn't make sense. The "trimming algorithm" and the "collapsing algorithm" are two algorithms that happen in different phases. What I'm saying here is that the trimming algorithm is unmodified from the basic version with collapse
. Perhaps this could disambiguate:
- `preserve-breaks`: the algorithm is the same as `collapse`. | |
- `preserve-breaks`: The algorithm is the same as that of `collapse`. |
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.
Gotcha. Perhaps we can make it clearer:
preserve-breaks
: The same algorithm is applied as with thecollapse
value.
(https://developer.mozilla.org/en-US/docs/Web/CSS/white-space-collapse#collapse will be pointing to /en-US/docs/Web/CSS/CSS_text/Whitespace#collapsing_and_transformation
btw)
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 white-space-collapse
value affects both collapsing and trimming; the trimming section also starts with the note that "we assume white-space-collapse: collapse
below".
This pull request has merge conflicts that must be resolved before it can be merged. |
Co-authored-by: Dipika Bhattacharya <[email protected]>
Co-authored-by: Dipika Bhattacharya <[email protected]>
Co-authored-by: Dipika Bhattacharya <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Dipika Bhattacharya <[email protected]>
45d4835
to
d331e14
Compare
Don't block on my re-review, if you're happy with this then please feel free to merge. |
@Josh-Cena, there's just two outstanding suggestions. They're not blockers. |
I'd like to merge the current content like this. Thanks both for your reviews! |
Fix #28268.
An overwhelming proportion of this article talks about CSS, and has nothing to do with either DOM or HTML. As such, I think it would be better to just rebrand this article wholesale as one about CSS whitespace processing. This does strand the "Whitespace in DOM" section a bit, but it's short and more like an aside anyway, and we eventually tie back to the "rendered" aspect anyway when talking about selection and
innerText
.cc @yarusome