Skip to content

[css-values-5] Introduce sibling-index() and sibling-count() (#4559) #9134

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

Closed
wants to merge 4 commits into from

Conversation

bramus
Copy link
Contributor

@bramus bramus commented Aug 1, 2023

A first, and possible naive, attempt at speccing sibling-index() and sibling-count(). Very much open to feedback on how to do this properly.

@bramus bramus requested review from tabatkins and fantasai August 1, 2023 09:47

Issue: Describe when it actually resolves. Computed Value? Used Value?
Copy link
Member

Choose a reason for hiding this comment

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

Definitely resolves computed-value-time, no reason to wait any longer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added in af5ce47

Getting the position of an element within its siblings: the ''sibling-index()'' function</h3>

The <dfn>sibling-index()</dfn> function resolves to the position
of an element within its group of <a>inclusive siblings</a>.
Copy link
Member

Choose a reason for hiding this comment

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

Might want to clarify that it's the inclusive flat tree siblings, which is what we want (I assume).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Clarified in 30de15b, but not sure if my wording is 100% correct.

Copy link
Member

Choose a reason for hiding this comment

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

We don't use flat tree order for :nth-child(). I think we want :nth-*() and sibling-index() to be consistent?

Copy link
Member

Choose a reason for hiding this comment

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

That's true.

@bramus bramus requested a review from andruud August 29, 2023 11:50
Copy link
Member

@tabatkins tabatkins left a comment

Choose a reason for hiding this comment

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

A couple of nits. Also, I'd add a few small examples to show them off.

██ ██ ██ ██ ██ ██ ██ ████ ██ ██ ██ ██ ██ ██ ██ ████ ██ ██ ██ ████ ██ ██
██ ██ ██ ██ ██ ██ ██ ███ ██ ██ ██ ██ ██ ██ ██ ██ ███ ██ ██ ██ ███ ██ ██
████████ ████████ ████████ ██ ██ ████████ ██ ██ ██ ██████ ███████ ███████ ██ ██ ██ ████ ██ ██ ██████
-->
Copy link
Member

Choose a reason for hiding this comment

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

Nit: Keep these headings as short as possible; if they wrap (as they definitely will on many screens, given how long these lines are) they become unreadable. I'd probably just do "COUNT" or maybe two lines having "INDEX" and "COUNT".


The ''sibling-index()'' and ''sibling-count()'' functions
(the <dfn export for=CSS>element counting functions</dfn>)
allow authors to take these factors into account in ''calc()'' expressions.
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
allow authors to take these factors into account in ''calc()'' expressions.
allow authors to take these factors into account anywhere that allows <<integer>>s,
such as in [=math functions=].

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 light rewrite to avoid implying that these are only valid in calc().)

allow authors to take these factors into account in ''calc()'' expressions.

With these, authors can create things like staggered animation delays
or easily position elements on a circle.
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
or easily position elements on a circle.
or easily position elements evenly around a circle.

Comment on lines +977 to +978
The <dfn>sibling-index()</dfn> function resolves to the position
of an element within its group of <a>inclusive siblings</a>
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 <dfn>sibling-index()</dfn> function resolves to the position
of an element within its group of <a>inclusive siblings</a>
The <dfn>sibling-index()</dfn> function resolves to the <<integer>>
representing the element's index within its list of <a>inclusive siblings</a>

Comment on lines +987 to +988
The <dfn>sibling-count()</dfn> function resolves to the total number of
<a>inclusive siblings</a> in the <a>flat tree</a> an element is part of.
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 <dfn>sibling-count()</dfn> function resolves to the total number of
<a>inclusive siblings</a> in the <a>flat tree</a> an element is part of.
The <dfn>sibling-count()</dfn> function resolves to the <<integer>>
representing the total number of the element's <a>inclusive siblings</a>
in the <a>flat tree</a>.

@Loirooriol
Copy link
Contributor

With 762eace, I guess this can be closed.

@Loirooriol Loirooriol closed this Nov 3, 2023
@bramus
Copy link
Contributor Author

bramus commented Nov 6, 2023

Yes, thanks for closing this one @Loirooriol. I was preoccupied with Other Important Things™ + some time OOO, so never got round to reviewing the suggested changes in this thread.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants