Skip to content
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

Box sizing migration #2739

Merged
merged 5 commits into from
Jun 8, 2018
Merged

Box sizing migration #2739

merged 5 commits into from
Jun 8, 2018

Conversation

frivoal
Copy link
Collaborator

@frivoal frivoal commented Jun 6, 2018

This PR solves #2458

@frivoal frivoal added css-sizing-3 Current Work css-ui-4 Current Work labels Jun 6, 2018
@frivoal frivoal self-assigned this Jun 6, 2018
Copy link
Collaborator

@fantasai fantasai left a comment

Choose a reason for hiding this comment

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

A few comments, otherwise looks good. Feel free to merge after handling those changes, I don't think it needs re-review.

@@ -53,6 +53,8 @@ Module interactions</h3>
<p>This module extends the 'width', 'height', 'min-width', 'min-height', 'max-width', 'max-height', and 'column-width'
features defined in [[!CSS2]] chapter 10 and in [[!CSS3COL]]

<p>The definition of the 'box-sizing' property in module supersedes the one in [[CSS-UI-3]].
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think I'd probably make this a note?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I made all the requested changes except this one.

If it is a note, then css-sizing does not normatively supersede css-ui, and we have two specs both claiming to the the canonical version. That sounds less than ideal. Merging as is for expediency, but feel free to fix after the fact if you still disagree.

(<a lt="content box">content-box</a> size)
of a <a>box</a>
unless otherwise indicated.

Note: [[CSS-UI-3#box-sizing]] provides an explicit disambiguation of these terms
for the <a href="https://www.w3.org/TR/CSS21/visudet.html">Visual formatting model details</a> section of [[CSS2]].
Copy link
Collaborator

Choose a reason for hiding this comment

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

s/Note:/Refer to/; s/provides/for/

for the <a href="https://www.w3.org/TR/CSS21/visudet.html">Visual formatting model details</a> section of [[CSS2]].

<div class=advisement>To avoid ambiguities,
specification authors should avoid ambiguous terms such as width or height without further qualification,
Copy link
Collaborator

Choose a reason for hiding this comment

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

“should avoid ambiguous uses of terms such as width or height without further qualification”

because sometimes the disambiguation is formed by context and not needed in a specific instance of the term--it's ambiguous usage that's a problem

@@ -140,196 +140,14 @@ which itself replaced and superseded the following:
<a href="https://www.w3.org/TR/2000/WD-css3-userint-20000216">User Interface for CSS3 (16 February 2000)</a> [[CSS-UI-3]]
</ul>

One feature previously defined in [[CSS-UI-3]] has been moved to [[CSS-SIZING-3]] rather than this specification.
Copy link
Collaborator

@fantasai fantasai Jun 7, 2018

Choose a reason for hiding this comment

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

Delete this sentence and the next two headings, mark the following added text as a note, and add empty anchors for all the removed anchors, e.g. via <a id=".."></a> or somesuch. This way links won't be broken, but we also don't have to carry around an empty section.

@frivoal frivoal merged commit 727d9c1 into w3c:master Jun 8, 2018
@frivoal frivoal deleted the box-sizing-migration branch June 8, 2018 03:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants