Skip to content

Conversation

chrisdavidmills
Copy link
Contributor

Description

Chrome 140 adds support for the scroll-target-group property; see https://chromestatus.com/feature/5189126177161216.

This PR adds documentation and examples for this property, and links all the things.

Motivation

Additional details

Related issues and pull requests

@chrisdavidmills chrisdavidmills requested a review from a team as a code owner August 22, 2025 13:34
@chrisdavidmills chrisdavidmills requested review from estelle and removed request for a team August 22, 2025 13:34
@github-actions github-actions bot added the Content:CSS Cascading Style Sheets docs label Aug 22, 2025
@chrisdavidmills chrisdavidmills changed the title Add docs for scroll-target-group Technical review: Add docs for scroll-target-group Aug 22, 2025
@github-actions github-actions bot added the size/m [PR only] 51-500 LoC changed label Aug 22, 2025
Copy link
Contributor

github-actions bot commented Aug 22, 2025

Preview URLs (6 pages)
Flaws (1)

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

URL: /en-US/docs/Web/CSS/CSS_overflow/CSS_carousels
Title: Creating CSS carousels
Flaw count: 1

  • unknown:
    • Error parsing /shared-assets/images/diagrams/css/carousels/carousel.svg: No such file or directory (os error 2)

(comment last updated: 2025-09-04 07:33:30)

@danielsakhapov
Copy link

Overall the text and the examples look good and communicate clearly the idea of this property.

Though, instead of saying:

The scroll-target-group CSS property specifies whether a container element is a scroll marker group.

Could you use a definition from the spec:

The scroll-target-group property specifies whether the element is a scroll marker group container.

And change this wording throughout the rest of the text as well, as this property doesn't really make an element a scroll marker group (not even sure you can say it about an element).

And also:

Note: A scroll marker group can be created from an existing element container using scroll-target-group.

Maybe instead explicitly say that you can create it from an existing set of html anchor elements + container?

@flackr Could you have a quick look, please.

@chrisdavidmills chrisdavidmills requested a review from a team as a code owner August 26, 2025 09:14
@chrisdavidmills chrisdavidmills requested review from dipikabh and removed request for a team August 26, 2025 09:14
@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 Aug 26, 2025
@chrisdavidmills
Copy link
Contributor Author

Overall the text and the examples look good and communicate clearly the idea of this property.

Though, instead of saying:

The scroll-target-group CSS property specifies whether a container element is a scroll marker group.

Could you use a definition from the spec:

The scroll-target-group property specifies whether the element is a scroll marker group container.

And change this wording throughout the rest of the text as well, as this property doesn't really make an element a scroll marker group (not even sure you can say it about an element).

OK, so basically the issue is that we are creating a container for the scroll marker group, not the scroll marker group itself (as this is just the group of scroll markers collected together inside the container)? I've updated the wording throughout. This was an issue on some of the existing CSS carousel content too.

And also:

Note: A scroll marker group can be created from an existing element container using scroll-target-group.

Maybe instead explicitly say that you can create it from an existing set of html anchor elements + container?

OK, makes sense. I've changed such instances to say that anchor elements are needed inside the container, as well as the container.

@chrisdavidmills
Copy link
Contributor Author

Thanks for the review, @danielsakhapov. Let me know if you spot any other issues that need attention.

@estelle estelle removed their request for review August 29, 2025 09:07
Copy link
Contributor

github-actions bot commented Sep 4, 2025

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

@github-actions github-actions bot added the merge conflicts 🚧 [PR only] label Sep 4, 2025
@github-actions github-actions bot removed the merge conflicts 🚧 [PR only] label Sep 4, 2025
@chrisdavidmills chrisdavidmills changed the title Technical review: Add docs for scroll-target-group Editorial review: Add docs for scroll-target-group Sep 4, 2025
Copy link

@flackr flackr left a comment

Choose a reason for hiding this comment

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

This looks good to me as well, thanks for looping me in!

@chrisdavidmills
Copy link
Contributor Author

This looks good to me as well, thanks for looping me in!

@flackr awesome, thanks for giving it a look!

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.

3 participants