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

docs(css): Fx116 - syntax updates for offset-path and ray() #28348

Merged
merged 10 commits into from
Aug 17, 2023

Conversation

dipikabh
Copy link
Contributor

@dipikabh dipikabh commented Aug 2, 2023

1. offset-path

Previous syntax

offset-path: none | ray( [ <angle> && <size> && contain? ] ) | <path()> | <url> | [ <basic-shape> || <geometry-box> ]

New syntax

offset-path: none | <offset-path> || <coord-box>
where:
<offset-path> = <ray()> | <url> | <basic-shape>
ray( <angle> && <size>? && contain? && [at <position>]? )

2. ray()

Previous syntax

ray( <angle> && <ray-size>? && contain? )

New syntax

ray( <angle> && <size>? && contain? && [at <position>]? )

Related issues and pull requests

Doc trackers:

Bugs:

To Dos

@dipikabh dipikabh requested a review from a team as a code owner August 2, 2023 19:11
@dipikabh dipikabh requested review from chrisdavidmills and removed request for a team August 2, 2023 19:11
@github-actions github-actions bot added the Content:CSS Cascading Style Sheets docs label Aug 2, 2023
@dipikabh dipikabh requested review from estelle and removed request for chrisdavidmills August 2, 2023 19:11
@github-actions
Copy link
Contributor

github-actions bot commented Aug 2, 2023

Preview URLs (7 pages)
External URLs (4)

URL: /en-US/docs/Web/CSS/offset-path
Title: offset-path

(comment last updated: 2023-08-17 02:45:59)


In this example, various `offset-anchor` and `offset-path: ray()` values are applied to a box and results are displayed side-by-side for comparison. One box border is highlighted to demonstrate different ray starting points and box orientations. After a box is positioned at the ray's starting point, it is oriented in the direction of the specified ray angle.
In this example, various `offset-anchor` and `offset-path: ray()` values are applied to a box. Results are displayed side-by-side for comparison. The element's containing block is depicted with a dashed border. One border of the box is highlighted to demonstrate different ray starting points and box orientations. After a box is positioned at the ray's starting point, it is oriented in the direction of the specified ray angle. If `offset-position` is not specified, the default offset starting position of a ray is the top-left corner of the element's containing block.

Choose a reason for hiding this comment

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

Note: the spec got updated recently for the initial value of offset-position. (w3c/fxtf-drafts#522)
The initial value of offset-position is normal now (https://bugzilla.mozilla.org/show_bug.cgi?id=1846817), so if we don't specify offset-position, and ray() doesn't specify at <position>, the default starting position will be at center of the containing block.
https://drafts.fxtf.org/motion-1/#valdef-ray-at-position

I am working on this new change now, so perhaps you have to create a new PR for it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for flagging this, @BorisChiou. I've added the doc keyword to that bug so that it is on our radar when the fix comes in for us to update the docs. From the way it is currently working, ray() is using offset-position: auto when no at <position> and offset-position are specified. I'll update this later after your fix is in.

On a related note, the docs are updated for the new keyword for offset-position: https://developer.mozilla.org/en-US/docs/Web/CSS/offset-position#values. But I should add there that normal is (50% 50%) of the containing block.

Appreciate you taking a look at the changes in this PR.

Copy link
Member

@estelle estelle left a comment

Choose a reason for hiding this comment

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

Only got thru <box>, which i am suggesting we change to <box-edge>, since box is said to be the same as visual box and is used in the syntax, so can't have two definitions, especially in one page. Also, the spec uses box-edge

files/en-us/web/css/box_value/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/box_value/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/box_value/index.md Outdated Show resolved Hide resolved
Comment on lines 37 to 39
- `<coord-box>`
- : Refers to the coordinate box used for positioning and sizing an element within its parent container. It is used to control how content flows around the edges of the box. It excludes the margin area.

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- `<coord-box>`
- : Refers to the coordinate box used for positioning and sizing an element within its parent container. It is used to control how content flows around the edges of the box. It excludes the margin area.
- `<coord-box>`
- : Refers to the coordinate box used for positioning and sizing an element within its parent container. It is used to control how content flows around the edges of the box. It excludes the margin area. This value type is used for the {{cssxref("mask-origin")}} property
- `<geometry-box>`
- : Defines the reference box for the basic shape or, if specified by itself, it causes the edges of the specified box, including any corner shaping (such as a {{cssxref("border-radius")}}), to be the clipping path. This value type is used for the {{cssxref("mask-clip")}} property and the SVG {{SVGAttr("clip-path")}} attribute.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for adding <geometry-box>! I somehow completely missed adding an entry for it.

Btw, shouldn't mask-origin be listed with <geometry-box>?
I've changed the property listed with <coord-box> to offset-path (as per the new syntax, also updated as part of this PR).

files/en-us/web/css/box_value/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/box_value/index.md Outdated Show resolved Hide resolved
@github-actions github-actions bot added the merge conflicts 🚧 [PR only] label Aug 3, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Aug 3, 2023

This pull request has merge conflicts that must be resolved before it can be merged.

Copy link
Member

@estelle estelle left a comment

Choose a reason for hiding this comment

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

reviewed some more

files/en-us/web/css/offset-path/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/offset-path/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/offset-path/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/offset-path/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/offset-path/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/offset-path/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/offset-path/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/offset-path/index.md Outdated Show resolved Hide resolved
Copy link
Member

@estelle estelle 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 more comments

files/en-us/web/css/box-edge/index.md Outdated Show resolved Hide resolved
Comment on lines 17 to 21
<visual-box> = content-box | padding-box | border-box /* also referred to as <box> */
<layout-box> = <box> | margin-box /* also referred to as <shape-box> */
<paint-box> = <box> | fill-box | stroke-box
<coord-box> = <box> | fill-box | stroke-box | view-box
<geometry-box> = <layout-box> | fill-box | stroke-box | view-box
Copy link
Member

Choose a reason for hiding this comment

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

not wanting to hold anything up with this question, but...

we use <box> on 17, 18, and 19. It is introduced in line 16 as a comment.
we use <layout-box> on 21, which is the name on 18, with the comment of 18 introducing <shape-box>, but we're not using <shape-box>

thinking this may be a way of updating this:

Suggested change
<visual-box> = content-box | padding-box | border-box /* also referred to as <box> */
<layout-box> = <box> | margin-box /* also referred to as <shape-box> */
<paint-box> = <box> | fill-box | stroke-box
<coord-box> = <box> | fill-box | stroke-box | view-box
<geometry-box> = <layout-box> | fill-box | stroke-box | view-box
<visual-box> = content-box | padding-box | border-box /* the three <box> values */
<layout-box> = <box> | margin-box
<paint-box> = <box> | fill-box | stroke-box
<coord-box> = <box> | fill-box | stroke-box | view-box
<geometry-box> = <layout-box> | fill-box | stroke-box | view-box

and then adding the "also referred to as " under layout box in the values section.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's an excellent observation!

  • The reason I've mentioned <shape-box> in the syntax is because it occurs in the shape-outside property.
    It is defined in the Shapes module spec as:
    <shape-box> = <box> | margin-box (https://drafts.csswg.org/css-shapes/#typedef-shape-box)
    (but the Box module spec defines the data type as <layout-box>, so keeping that as is here...)
  • The clip-path formal syntax refers to <geometry-box>, <shape-box>, and <box>. Another reason why I wanted to define what's <shape-box> and <box>.
  • I believe I've seen occurrences of <box> in a few other "Formal syntax" sections.
  • If we define these alternate terms here, we can point from those property pages to the <box-edge> page in the future and update their value descriptions as well.

How about for <geometry-box>, we use <shape-box> instead of <layout-box>? (Because <shape-box> is more commonly used, but there are no occurrences of <layout-box> in content..yet!)

Updated text:

<visual-box> = content-box | padding-box | border-box /* the three <box> values */
<layout-box> = <box> | margin-box /* the <shape-box> values */
<paint-box> = <box> | fill-box | stroke-box
<coord-box> = <box> | fill-box | stroke-box | view-box
<geometry-box> = <shape-box> | fill-box | stroke-box | view-box

files/en-us/web/css/box-edge/index.md Outdated Show resolved Hide resolved

- `<paint-box>`

- : Refers to the area within the layout box that is used to render the content visually. This includes the area where the element's background and borders are painted. It excludes the margin area.
Copy link
Member

Choose a reason for hiding this comment

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

still looking for a property that uses this... no luck. Do you have know how we can look at a web ref dump?

I am thinking that since in SVG, border-box is treated as stroke-box and padding-box is treated as fill-box, is paint-box just a synonym for visual box?

Maybe @fantasai can give us insight?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you have know how we can look at a web ref dump?

No idea but it would be good to learn how to.

I guess we keep this description as is for now and update when we have more info

files/en-us/web/css/box-edge/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/offset-path/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/offset-path/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/offset-path/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/offset-path/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/offset-path/index.md Outdated Show resolved Hide resolved
@dipikabh
Copy link
Contributor Author

Thanks for the review, @estelle! I've incorporated most of your suggested changes.

A few outstanding comments:

@dipikabh dipikabh requested a review from estelle August 17, 2023 02:45
Copy link
Member

@estelle estelle left a comment

Choose a reason for hiding this comment

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

why not?

@estelle estelle merged commit 3a1ad1f into mdn:main Aug 17, 2023
6 checks passed
@dipikabh
Copy link
Contributor Author

Thank you! Let me take up the <box-edge cleanups in a new PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:CSS Cascading Style Sheets docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants