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 3: sizing and positioning part 1 #33493

Merged
merged 45 commits into from
Jun 25, 2024

Conversation

chrisdavidmills
Copy link
Contributor

@chrisdavidmills chrisdavidmills commented May 8, 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:

  • The anchor() function
  • Properties that support anchor() as a value:
    • top
    • left
    • bottom
    • right
    • inset-block-start
    • inset-block-end
    • inset-inline-start
    • inset-inline-end
    • inset-block
    • inset-inline
    • inset
  • The anchor-center value, which is supported on the following properties:
    • justify-self
    • align-self
    • justify-items
    • align-items
    • place-items
    • place-self

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 8, 2024 14:31
@chrisdavidmills chrisdavidmills requested review from bsmth and removed request for a team May 8, 2024 14:31
@github-actions github-actions bot added Content:CSS Cascading Style Sheets docs size/m [PR only] 51-500 LoC changed labels May 8, 2024
Copy link
Contributor

github-actions bot commented May 8, 2024

Preview URLs (20 pages)
Flaws (7)

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

URL: /en-US/docs/Web/CSS/CSS_anchor_positioning/Using
Title: Using CSS anchor positioning
Flaw count: 3

  • macros:
    • /en-US/docs/Web/CSS/inset-area does not exist
  • broken_links:
    • Can't resolve /en-US/docs/Web/CSS/inset-area_value
    • Can't resolve /en-US/docs/Web/CSS/anchor-size

URL: /en-US/docs/Web/CSS/CSS_Functions
Title: CSS value functions
Flaw count: 2

  • macros:
    • /en-US/docs/Web/CSS/anchor-size does not exist
    • /en-US/docs/Web/CSS/inset-area_function does not exist

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

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

(comment last updated: 2024-06-25 12:49:46)

@chrisdavidmills chrisdavidmills changed the title Add anchor() function ref page Tech review: CSS anchor positioning 3: sizing and positioning May 8, 2024
@chrisdavidmills chrisdavidmills marked this pull request as draft May 8, 2024 14:46
@chrisdavidmills chrisdavidmills changed the title Tech review: CSS anchor positioning 3: sizing and positioning Tech review: CSS anchor positioning 3: sizing and positioning part 1 May 9, 2024
@chrisdavidmills chrisdavidmills marked this pull request as ready for review May 9, 2024 10:47
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.

Looks good! Lots of comments, but overall very nice.

files/en-us/web/css/align-items/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/align-self/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/anchor/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/anchor/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/inset-block/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/inset-inline/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/left/index.md Outdated Show resolved Hide resolved
@bsmth bsmth removed their request for review May 13, 2024 11:58
@chrisdavidmills chrisdavidmills changed the title Tech review: CSS anchor positioning 3: sizing and positioning part 1 Editorial review: CSS anchor positioning 3: sizing and positioning part 1 May 20, 2024
@chrisdavidmills chrisdavidmills requested review from estelle and removed request for mfreed7 May 20, 2024 18:01
@chrisdavidmills
Copy link
Contributor Author

Thanks for the tech review, @mfreed7.

@estelle, this one is ready for editorial review too.

estelle

This comment was marked as outdated.

@github-actions github-actions bot removed the merge conflicts 🚧 [PR only] label Jun 17, 2024
@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 Jun 18, 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.

REVIEW OF ALL INSET PROPERTY PAGES:

I reviewed the right inset property page. the suggestions from that page apply to every inset property page:

  1. we should likely add to every inset property page that it is an inset property, linking that term to the glossary. I think the best place is the line in the intro:

Changing "It has no effect on non-positioned elements." to "This {{glossary("inset properties", "inset property")}} has no effect on non-positioned elements." or " {{glossary("Inset properties")}} have no effect on non-positioned elements." or "This property, like all {{glossary("Inset properties")}}, has no effect on non-positioned elements."

The above fact is missing from the logical inset properties.

  1. In the first syntax section, in the intro, move the anchor() examples into the <length> section. Make one of the two examples part of a calc() function.

  2. In the values, the anchor() should be a component within the <lenght> description. It is not a standalone value in the formal syntax for inset properties. Rather, it returns a length. In that description, indicate which <anchor-side> values are valid. This part is unique for each page.

for anchor positioned elements, the {{cssxref("anchor()")}} function returns 0px from the X or X edge of the associated anchor element, depending on whether the function's anchor-side parameter is X or X, respectively.

  1. In the Description section of each inset property, amend the line about absolute and fixed positions:

When position is set to absolute or fixed, the X property specifies the distance between the element's outer margin of X edge and the inner border of the X edge of its containing block. If the positioned element has an associated anchor, ....

Don't add anchor positioning to the see also sections of the inset property pages.

Comment on lines 59 to 60
- : Resolves to a {{cssxref("&lt;length&gt;")}} value relative to the position of the left and/or right edges of an absolutely- or fixed-positioned element's associated **anchor element**. See [CSS anchor positioning](/en-US/docs/Web/CSS/CSS_anchor_positioning).

Copy link
Member

Choose a reason for hiding this comment

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

I was thinking something like this:

  - : Only applicable to [anchor-positioned](/en-US/docs/Web/CSS/CSS_anchor_positioning/Using) elements. Resolves to a {{cssxref("&lt;length&gt;")}} value relative to the position of the left and/or right edges of the element's associated anchor element.

And for all inset property pages we need to:

  • in values, indicate that it's only relevant to PosEls.
  • link to how to create PosEls (rather than the module)

and like 15 other things

but the return value is a <length>, so we should NOT include anchor() within the values section on any inset page.

We should still include anchor() on the page, but with these updates to the page instead of the current updates:

  1. in the non-formal syntax, include the two anchor examples under length. Make one a calc(anchor(x) + 10px) or something similar so it is clear it's a length.

  2. under <length> in the values, where it reads:

<length>

  • : A negative, null, or positive <length> that represents:

for absolutely positioned elements, the distance to the top edge of the containing block.

add a DD (between line 45 and 46 on this page):

  • : for anchor positioned elements, the {{cssxref("anchor()") function returns 0px from the top or bottom edge of the associated anchor element, depending on whether the function's anchor-side parameter is top or bottom, respectively.

do not bold any of the terms as they are linked. We already link to the guide and to the inset portion directly, so no additional links in this section are needed

  • In the description, by "When position is set to absolute or fixed, the X property specifies the distance between the element's outer margin of right edge and the inner border of the right edge of its containing block." update that sentence to include anchor. (line 68 on this page)

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 started to fix all of this stuff, and then realized that we hadn't really understood the exact effect of the <anchor-side> values or tested them thoroughly enough. I then fell down a giant rabbit hole. 4 hours of testing later and some text updates, and I think I understand how it works now. I also think I have done all that you are asking for in this comment.

Before you look through this stuff, have a look at my updated descriptions of the anchor-side values (these weren't right before):

https://pr33493.content.dev.mdn.mozit.cloud/en-US/docs/Web/CSS/anchor#anchor-side

and this compatibility description:

https://pr33493.content.dev.mdn.mozit.cloud/en-US/docs/Web/CSS/anchor#compatibility_of_inset_properties_and_anchor-side_values

I have also made some very small updates to the "Using..." guide that we just published. Fortunately, what we said in there is mostly not affected by this new information coming to light.

Comment on lines 27 to 30
/* anchor() function values */
right: anchor(left);
right: anchor(--myAnchor 50%);

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() function values */
right: anchor(left);
right: anchor(--myAnchor 50%);

Copy link
Member

Choose a reason for hiding this comment

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

move to length (i am in the gui and can't edit that)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

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.

REVIEW OF PLACE/JUSTIFY/ALIGN

The place-x, justify-x, and align-x pages are approved, except for a question on the place-x pages about the divisions of keywords v. positional in the informal sytnax

@@ -24,6 +24,7 @@ This property is a shorthand for the following CSS properties:
/* Keyword values */
place-self: auto center;
place-self: normal start;
place-self: anchor-center;
Copy link
Member

Choose a reason for hiding this comment

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

Should this be under positional alignment? Should we have a "keyword values" separated from "positional" section at all? (same question for place-items)

The spec reads:

The normal value for the self-alignment properties behaves as either start, end, or anchor-center, depending on the positioning of the region, to give a good default alignment for the positioned element. Again, see § 3.1.1 Resolving s for details.

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 think you've got a point here. The values given under keywords all have some component of positional alignment to them. I've added them all into the positional alignment section and gotten rid of the key values section as suggestion, in both pages.

I've put anchor-center at the bottom, in each case.

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.

Review of anchor()


/* property: anchor(anchor-element anchor-side, length-percentage) */
top: anchor(--myAnchor bottom, 50%);
inset-block-end: anchor(--myAnchor start, 200px);
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
inset-block-end: anchor(--myAnchor start, 200px);
inset-block-end: anchor(--myAnchor start, 200px);
left: calc(anchor(--myAnchor right, 0%) + 10px);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

LGTM; added


- `<anchor-element>` {{optional_inline}}

- : The [`anchor-name`](/en-US/docs/Web/CSS/anchor-name) property value of the anchor element you want to position the element's side relative to. This is a `<dashed-ident>` value. If omitted, the element's **default anchor** is used. This is the anchor referenced in its [`position-anchor`](/en-US/docs/Web/CSS/position-anchor) property, or associated with the element via the [`anchor`](/en-US/docs/Web/HTML/Global_attributes/anchor) HTML attribute.
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 [`anchor-name`](/en-US/docs/Web/CSS/anchor-name) property value of the anchor element you want to position the element's side relative to. This is a `<dashed-ident>` value. If omitted, the element's **default anchor** is used. This is the anchor referenced in its [`position-anchor`](/en-US/docs/Web/CSS/position-anchor) property, or associated with the element via the [`anchor`](/en-US/docs/Web/HTML/Global_attributes/anchor) HTML attribute.
- : The [`anchor-name`](/en-US/docs/Web/CSS/anchor-name) property value of the anchor element you want to position the element's side relative to. This is a `<dashed-ident>` value. If omitted, the element's **default anchor**, referenced in its [`position-anchor`](/en-US/docs/Web/CSS/position-anchor) property, or associated with the element via the [`anchor`](/en-US/docs/Web/HTML/Global_attributes/anchor) HTML attribute, is used.

Current version is what I was confused about. It doesn't create an association, but this is not the right spot to mention that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

LGTM; updated

- : Specifies the position relative to a side, or sides, of the anchor. If a physical or logical value is used that is not on the same axis as the inset property on which `anchor()` is set, the fallback value is used. Valid values include:

- `top`
- : The top of the 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.

Suggested change
- : The top of the anchor element.
- : The top of the anchor element. Valid within {{cssxref("top")}}, {{cssxref("bottom")}}, and {{cssxref("inset")}} properties only.

This is thought: should we add this to each value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See updated page. I've added all the compatibility of inset properties and <anchor-side> values stuff into a separate section in the description. It is a bit more nuanced than you think.


The `anchor()` function resolves to a `<length>`, so it can be used within [other CSS functions](/en-US/docs/Web/CSS/CSS_Functions) that accept length values, including {{cssxref("calc()")}}, {{cssxref("clamp()")}}, etc.

The function sets the inset position value as an absolute distance relative to the anchor element by defining the anchor element, the side of the anchor element the positioned element is being positioned relative to, and the distance from that side.
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 function sets the inset position value as an absolute distance relative to the anchor element by defining the anchor element, the side of the anchor element the positioned element is being positioned relative to, and the distance from that side.
The function is only valid within an inset property value set on an absolute or fixed position element. It returns a `<length>` absolute distance value relative to one side of an anchor element. The function defines which side of the anchor element the distance is relative to.

You likely have a better way of saying that it's a length, relative to an anchor, not necessarily the associated anchor, and that it's part of a value, not necessarily the whole value.

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 think yours was mostly fine; I've tried clarifying it a bit:

The function is only valid within an inset property value set on an absolute or fixed position element. It returns a <length> value specifying the distance between the anchor-positioned element side specified by the inset value, and the side of the anchor element specified by the chosen <anchor-side> value.


The function sets the inset position value as an absolute distance relative to the anchor element by defining the anchor element, the side of the anchor element the positioned element is being positioned relative to, and the distance from that side.

If the `<anchor-element>` specified in an `anchor()` function within an inline direction inset property differs from the `<anchor-element>` specified in an `anchor()` function within a block direction inset property, the block direction `<anchor-element>` takes precedence.
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
If the `<anchor-element>` specified in an `anchor()` function within an inline direction inset property differs from the `<anchor-element>` specified in an `anchor()` function within a block direction inset property, the block direction `<anchor-element>` takes precedence.
If the `<anchor-element>` specified in the inline direction inset property's `anchor()` function differs from the one set in the block direction's inset property `anchor()` function, the block direction's `<anchor-element>` takes precedence.

Mine says the same thing; and not sure if it's better. I had to read the original several times. Maybe this one has to be read several times too, or maybe it's easier. Your pick

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've deleted this paragraph, because I actually don't think it makes sense. If you specify different anchor elements in different inset property anchor() functions, the positioned element is associated with all of them, provided the values are valid. It doesn't choose one of them.


#### JavaScript

The JavaScript is adapted from a basic [draggable element script](https://www.w3schools.com/howto/howto_js_draggable.asp) published on W3Schools; for brevity, we won't repeat it here.
Copy link
Member

Choose a reason for hiding this comment

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

we never ever, ever, ever, link to w3schools.

Copy link
Contributor Author

@chrisdavidmills chrisdavidmills Jun 20, 2024

Choose a reason for hiding this comment

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

As I was doing this, I was imagining the ire I would raise in you...I think I got off lightly, all things considered ;-)

Comment on lines 408 to 413
if (document.getElementById(elmnt.id + "header")) {
// if present, the header is where you move the DIV from:
document.getElementById(elmnt.id + "header").onmousedown = dragMouseDown;
} else {
// otherwise, move the DIV from anywhere inside the DIV:
elmnt.onmousedown = dragMouseDown;
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
if (document.getElementById(elmnt.id + "header")) {
// if present, the header is where you move the DIV from:
document.getElementById(elmnt.id + "header").onmousedown = dragMouseDown;
} else {
// otherwise, move the DIV from anywhere inside the DIV:
elmnt.onmousedown = dragMouseDown;
elmnt.onmousedown = dragMouseDown;

there's no header

Copy link
Member

Choose a reason for hiding this comment

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

I am reworking the JS at https://codepen.io/estelle/pen/pomaadP.
i added a tabindex to make them focusable and a focus color to enable keyboard users to see where they are.
submitting this PR likely before i add keyboard actions


#### Result

Note how the positioned element is positioned relative to both the anchor elements. Drag them with the mouse to see how their position, and as a consequence, the area of the positioned element, changes. Also, try scrolling the demo to see how the positions of all the elements are maintained relative to one another on scroll.
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
Note how the positioned element is positioned relative to both the anchor elements. Drag them with the mouse to see how their position, and as a consequence, the area of the positioned element, changes. Also, try scrolling the demo to see how the positions of all the elements are maintained relative to one another on scroll.
The positioned element is positioned relative to both anchor elements. Drag them to see how their position, and as a consequence, the area of the positioned element, changes. Scroll to see how the positions of all the elements are maintained.

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

Comment on lines 454 to 457
> **Note:** This example is a quick, rough proof-of-concept, and not intended to be used in production code. Its main shortcomings are:
>
> 1. The dragging functionality is not robust. Note how the example breaks if you try to drag the anchors past each other horizontally or vertically.
> 2. The example is not keyboard accessible.
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
> **Note:** This example is a quick, rough proof-of-concept, and not intended to be used in production code. Its main shortcomings are:
>
> 1. The dragging functionality is not robust. Note how the example breaks if you try to drag the anchors past each other horizontally or vertically.
> 2. The example is not keyboard accessible.
>{!NOTE]
>This example is a proof-of-concept and not intended to be used in production code. Among its shortcomings, the example breaks if you try to drag the anchors past each other horizontally or vertically, and it is not keyboard accessible.

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


- {{cssxref("position-anchor")}}
- {{cssxref("inset-area")}}
- The {{cssxref("anchor-size()")}} 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 {{cssxref("anchor-size()")}} function
- {{cssxref("anchor-size()")}} 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.

updated


#### JavaScript

The JavaScript is adapted from a basic [draggable element script](https://www.w3schools.com/howto/howto_js_draggable.asp) published on W3Schools; for brevity, we won't repeat it here.
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 JavaScript is adapted from a basic [draggable element script](https://www.w3schools.com/howto/howto_js_draggable.asp) published on W3Schools; for brevity, we won't repeat it here.

use https://codepen.io/estelle/pen/pomaadP instead

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yay, thank you! I really couldn't be bothered to write this myself.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It worked fine, except that the keyboard controls didn't work — the arrow keys are already used to scroll the example window, so they didn't move the anchors. I've fixed this by instead assigning the W/A/S/D keys to up/down/left/right. This is a pretty common set of keyboard controls amongst gamers, so I thought it would be OK.

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.

w00t!

@estelle estelle merged commit 09c431e into mdn:main Jun 25, 2024
8 checks passed
@chrisdavidmills chrisdavidmills deleted the css-anchor-positioning-3 branch June 26, 2024 09:50
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 size/l [PR only] 501-1000 LoC changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants