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

Editorial review: CSS anchor positioning 4: sizing and positioning part 2 #33511

Merged
merged 30 commits into from
Jul 2, 2024

Conversation

chrisdavidmills
Copy link
Contributor

@chrisdavidmills chrisdavidmills commented May 9, 2024

Description

CSS Anchor Positioning is set to be released in Chrome 125 (see the associated Chrome Status entry).

This PR (part of a series; see the first PR here) adds the following content:

Do not merge until #33058 is merged.

Motivation

Additional details

Related issues and pull requests

@chrisdavidmills chrisdavidmills requested a review from a team as a code owner May 9, 2024 10:49
@chrisdavidmills chrisdavidmills requested review from estelle and removed request for a team May 9, 2024 10:49
@chrisdavidmills chrisdavidmills marked this pull request as draft May 9, 2024 10:49
@github-actions github-actions bot added Content:CSS Cascading Style Sheets docs size/m [PR only] 51-500 LoC changed labels May 9, 2024
Copy link
Contributor

github-actions bot commented May 9, 2024

Preview URLs (17 pages)
Flaws (6)

Note! 13 documents with no flaws that don't need to be listed. 🎉

URL: /en-US/docs/Web/CSS/anchor-size
Title: anchor-size()
Flaw count: 1

  • macros:
    • /en-US/docs/Web/CSS/inset-area does not exist

URL: /en-US/docs/Web/CSS/anchor-name
Title: anchor-name
Flaw count: 1

  • macros:
    • /en-US/docs/Web/CSS/inset-area does not exist

URL: /en-US/docs/Web/CSS/position-visibility
Title: position-visibility
Flaw count: 3

  • macros:
    • /en-US/docs/Web/CSS/position-try-options does not exist
    • /en-US/docs/Web/CSS/@position-try does not exist
    • /en-US/docs/Web/CSS/inset-area does not exist

URL: /en-US/docs/Web/CSS/anchor
Title: anchor()
Flaw count: 1

  • macros:
    • /en-US/docs/Web/CSS/inset-area does not exist

(comment last updated: 2024-07-02 12:38:10)

@github-actions github-actions bot added size/l [PR only] 501-1000 LoC changed and removed size/m [PR only] 51-500 LoC changed labels May 11, 2024
@chrisdavidmills chrisdavidmills marked this pull request as ready for review May 11, 2024 14:23
Copy link

@mfreed7 mfreed7 left a comment

Choose a reason for hiding this comment

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

LGTM! Nice page on inset-area. I shared with a few more folks to double-check for accuracy. But my read looked pretty good.

files/en-us/web/css/inset-area/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/inset-area/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/inset-area/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/inset-area/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/inset-area/index.md Outdated Show resolved Hide resolved
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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.

started this review while still in draft. Publishing the four initial comments. this is NOT a full review.

Comment on lines 1 to 3
<mxfile host="app.diagrams.net" modified="2024-05-11T13:43:05.958Z" agent="Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:127.0) Gecko/20100101 Firefox/127.0" etag="L6tp-tvD06_g2Gk0fwGm" version="24.3.1" type="device">
<diagram name="Page-1" id="UW3OLV6GaEz2t2Hav6oM">
<mxGraphModel dx="1809" dy="871" grid="1" gridSize="10" guides="1" tooltips="1" connect="1" arrows="1" fold="1" page="1" pageScale="1" pageWidth="3300" pageHeight="4681" math="0" shadow="0">
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
<mxfile host="app.diagrams.net" modified="2024-05-11T13:43:05.958Z" agent="Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:127.0) Gecko/20100101 Firefox/127.0" etag="L6tp-tvD06_g2Gk0fwGm" version="24.3.1" type="device">
<diagram name="Page-1" id="UW3OLV6GaEz2t2Hav6oM">
<mxGraphModel dx="1809" dy="871" grid="1" gridSize="10" guides="1" tooltips="1" connect="1" arrows="1" fold="1" page="1" pageScale="1" pageWidth="3300" pageHeight="4681" math="0" shadow="0">
<mxfile>
<diagram>
<mxGraphModel dx="1809" dy="871" pageWidth="3300" pageHeight="4681">

I don't think this is used, so we can delete the file.

I assume this created the PNG. But, if necessary to keep, can we convert to SVG and run through SVGOMG.

If keeping as drawio, is there an svgomg equivalent for it? None of the ids are used and empty values are, well, empty. rounded=0;whiteSpace=wrap;html=1; are likely all removable. Not sure about as="geometry"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

source file for the diagram. I'm not messing with the code for this ;-)


The **`anchor-size()`** [CSS](/en-US/docs/Web/CSS) [function](/en-US/docs/Web/CSS/CSS_Functions) can be used as a value for an **anchor-positioned** element's [sizing properties](#properties_that_accept_anchor-size_function_values), to size it relative to the dimensions of its associated **anchor element**.

For detailed information on anchor usage, see the [CSS Anchor Positioning](/en-US/docs/Web/CSS/CSS_anchor_positioning) module landing page.
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
For detailed information on anchor usage, see the [CSS Anchor Positioning](/en-US/docs/Web/CSS/CSS_anchor_positioning) module landing page.
For detailed information on anchor usage, see the [CSS Anchor Positioning](/en-US/docs/Web/CSS/CSS_anchor_positioning/Using) guide.

The module page should just be a list of links to everything mentioned in the module. I see in the see also you're creating a guide, so that is likely where we want to link.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Already fixed this.

Comment on lines 38 to 39
max-block-size: anchor-size(height);
max-block-size: anchor-size(--my-anchor block);
Copy link
Member

Choose a reason for hiding this comment

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

instead of re-using the exact same values on every property page, can we include different values so people see it being used with different values

Suggested change
max-block-size: anchor-size(height);
max-block-size: anchor-size(--my-anchor block);
max-block-size: anchor-size(self-inline);
max-block-size: anchor-size(--anchorNameValue block, 120%);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, I've made some updates


{{CSSRef}}

The **`anchor-size()`** [CSS](/en-US/docs/Web/CSS) [function](/en-US/docs/Web/CSS/CSS_Functions) can be used as a value for an **anchor-positioned** element's [sizing properties](#properties_that_accept_anchor-size_function_values), to size it relative to the dimensions of its associated **anchor element**.
Copy link
Member

Choose a reason for hiding this comment

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

Define anchor element, maybe:

Suggested change
The **`anchor-size()`** [CSS](/en-US/docs/Web/CSS) [function](/en-US/docs/Web/CSS/CSS_Functions) can be used as a value for an **anchor-positioned** element's [sizing properties](#properties_that_accept_anchor-size_function_values), to size it relative to the dimensions of its associated **anchor element**.
The **`anchor-size()`** [CSS](/en-US/docs/Web/CSS) [function](/en-US/docs/Web/CSS/CSS_Functions) can be used as a value for an **anchor-positioned** element's [sizing properties](#properties_that_accept_anchor-size_function_values), to size it relative to the dimensions of its associated **anchor element**, the element you want to size an element relative to.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not convinced this is needed. We've got links for more information for people that are confused as to what this means.

Copy link
Member

Choose a reason for hiding this comment

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

ok. will debold.

@chrisdavidmills chrisdavidmills changed the title Tech review: CSS anchor positioning 4: sizing and positioning part 2 Editorial review: CSS anchor positioning 4: sizing and positioning part 2 May 21, 2024
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.

An <inset-area> data type page should be created.

The `anchor-size()` function's syntax is as follows:

```text
anchor(anchor-element anchor-size, length-percentage)
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
anchor(anchor-element anchor-size, length-percentage)
anchor-size(anchor-element anchor-size, length-percentage)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oops, fixed!

The parameters are:

- `anchor-element` {{optional_inline}}
- : The {{cssxref("anchor-name")}} set on the anchor element you want to size an element relative to. This is optional — if omitted, the positioned element is sized relative to the anchor referenced by its {{cssxref("position-anchor")}} property (this is sometimes referred to as the element's **default anchor**).
Copy link
Member

Choose a reason for hiding this comment

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

same comments as anchor()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

- : The {{cssxref("anchor-name")}} set on the anchor element you want to size an element relative to. This is optional — if omitted, the positioned element is sized relative to the anchor referenced by its {{cssxref("position-anchor")}} property (this is sometimes referred to as the element's **default anchor**).
- `anchor-size`

- : Specifies the dimension of the anchor element that the positioned element will be sized relative to. This can be expressed using the following values:
Copy link
Member

Choose a reason for hiding this comment

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

same comments as anchor()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated


### Return value

Returns a {{cssxref("length")}} value.
Copy link
Member

Choose a reason for hiding this comment

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

same comments as anchor()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

same counter comment as for anchor()


The properties that can take an `anchor-size()` function as a value are as follows:

- Physical sizing properties: {{cssxref("width")}}, {{cssxref("height")}}, {{cssxref("min-width")}}, {{cssxref("min-height")}}, {{cssxref("max-width")}}, and {{cssxref("max-height")}}.
Copy link
Member

Choose a reason for hiding this comment

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

just a list.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated


{{CSSRef}}{{seecompattable}}

The **`inset-area`** [CSS](/en-US/docs/Web/CSS) property enables an **anchor-positioned** element to be positioned relative to the edges of its associated **anchor element** by placing the positioned element on one or more tiles of an implicit 3x3 grid called the **inset-area grid**. This provides an alternative to positioning an element relative to its anchor via inset properties and the {{cssxref("anchor()")}} function.
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
The **`inset-area`** [CSS](/en-US/docs/Web/CSS) property enables an **anchor-positioned** element to be positioned relative to the edges of its associated **anchor element** by placing the positioned element on one or more tiles of an implicit 3x3 grid called the **inset-area grid**. This provides an alternative to positioning an element relative to its anchor via inset properties and the {{cssxref("anchor()")}} function.
The **`inset-area`** [CSS](/en-US/docs/Web/CSS) property enables an **anchor-positioned** element to be positioned relative to the edges of its associated **anchor element** by placing the positioned element on one or more tiles of an implicit 3x3 grid called the **inset-area grid**. This provides an alternative to positioning an element relative to its anchor via {{glossary("inset properties")}} and the {{cssxref("anchor()")}} function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call; added

Comment on lines 19 to 40
/* Examples: Two keywords to place the element in a single specific tile */
/* Physical */
inset-area: top left;
inset-area: bottom right;
/* Logical */
inset-area: start end;
inset-area: center end;
inset-area: block-start center;
inset-area: inline-start block-end;
/* Coordinate-based */
inset-area: x-start y-end;
inset-area: center y-self-end;

/* Examples: Two keywords to span the element across two tiles */
/* Physical */
inset-area: top span-left;
inset-area: span-bottom right;
/* Logical */
inset-area: center span-start;
inset-area: inline-start span-block-end;
/* Coordinate-based */
inset-area: y-start span-x-end;
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
/* Examples: Two keywords to place the element in a single specific tile */
/* Physical */
inset-area: top left;
inset-area: bottom right;
/* Logical */
inset-area: start end;
inset-area: center end;
inset-area: block-start center;
inset-area: inline-start block-end;
/* Coordinate-based */
inset-area: x-start y-end;
inset-area: center y-self-end;
/* Examples: Two keywords to span the element across two tiles */
/* Physical */
inset-area: top span-left;
inset-area: span-bottom right;
/* Logical */
inset-area: center span-start;
inset-area: inline-start span-block-end;
/* Coordinate-based */
inset-area: y-start span-x-end;
/* Two keywords placing the element in a single specific tile */
inset-area: top left;
inset-area: bottom right;
inset-area: start end;
inset-area: center end;
inset-area: block-start center;
inset-area: inline-start block-end;
inset-area: x-start y-end;
inset-area: center y-self-end;
/* Two keywords spanning the element across two tiles */
inset-area: top span-left;
inset-area: span-bottom right;
inset-area: center span-start;
inset-area: inline-start span-block-end;
inset-area: y-start span-x-end;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

/* Coordinate-based */
inset-area: y-start span-x-end;

/* Examples: Two keywords to span the element across three tiles */
Copy link
Member

Choose a reason for hiding this comment

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

how about something like:

/* Two keywords to span the element across three tiles, and their one keyword equivalents */
inset-area: top span-all;
inset-area: top;
inset-area: block-end span-all;
inset-area: block-end;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd rather not make this change, as it might lead readers to think that the implied second keyword is always span-all, when this is not the case. The next syntax section gives some correct single keyword examples, and the implied keyword situations are discussed in depth later on.

Comment on lines 11 to 12

![The inset-area grid, as described below](inset-area.png)
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
![The inset-area grid, as described below](inset-area.png)

Copy link
Contributor Author

@chrisdavidmills chrisdavidmills Jun 5, 2024

Choose a reason for hiding this comment

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

makes sense. Having that up front as well was a bit overkill. Deleted.

inset-area: unset;
```

### Values
Copy link
Member

Choose a reason for hiding this comment

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

This area should be fairly short:

two value only (<inset-area> and none), no intro sentence.
the inset area term should link to a new inset-area data type page that explains all the value, including the image.

Add all the rest under "description.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I disagreed with you about this initially, but on looking at it, I ended up agreeing. There is so much information on this page that it benefitted from being split, and the values are so much easier to explore in the separate page.

So, pages separated, let me know what you think.

@github-actions github-actions bot added size/xl [PR only] >1000 LoC changed and removed size/l [PR only] 501-1000 LoC changed labels Jun 5, 2024
chrisdavidmills and others added 3 commits June 5, 2024 10:46
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@chrisdavidmills chrisdavidmills requested a review from a team as a code owner June 27, 2024 07:26
@github-actions github-actions bot added the Content:HTML Hypertext Markup Language docs label Jun 27, 2024
@github-actions github-actions bot added size/m [PR only] 51-500 LoC changed and removed size/xl [PR only] >1000 LoC changed labels Jul 2, 2024
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.

Yay! 🎉

@estelle estelle merged commit 9591173 into mdn:main Jul 2, 2024
8 checks passed
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 Content:HTML Hypertext Markup Language docs size/m [PR only] 51-500 LoC changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants