Skip to content

Add scrollIntoView() container option #40588

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

Merged
merged 5 commits into from
Aug 5, 2025

Conversation

aguingand
Copy link
Contributor

@aguingand aguingand commented Aug 1, 2025

Description

Documentation of the new Element.scrollintoView() container option.

Motivation

The container option has been specified & scheduled to ship in Chrome 140.

Additional details

https://drafts.csswg.org/cssom-view/#dom-scrollintoviewoptions-container
https://cr-status.appspot.com/feature/5100036528275456?gate=5140738792488960

Related issues

mdn/browser-compat-data#27463

@aguingand aguingand requested a review from a team as a code owner August 1, 2025 10:38
@aguingand aguingand requested review from sideshowbarker and removed request for a team August 1, 2025 10:39
@github-actions github-actions bot added the Content:WebAPI Web API docs label Aug 1, 2025
@github-actions github-actions bot added the size/s [PR only] 6-50 LoC changed label Aug 1, 2025
@aguingand
Copy link
Contributor Author

I don't know if there should be more details to the option description

@github-actions github-actions bot added size/m [PR only] 51-500 LoC changed and removed size/s [PR only] 6-50 LoC changed labels Aug 1, 2025
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Copy link
Contributor

github-actions bot commented Aug 1, 2025

Preview URLs

(comment last updated: 2025-08-05 08:26:02)

Copy link
Member

@sideshowbarker sideshowbarker left a comment

Choose a reason for hiding this comment

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

Howdy. The documentation you added for the container option is great, but can you say why you added quotes around all the option values? And why you didn’t also add any quotes around the option names?

Are you maybe following the example of a how some similar content on another existing page is formatted?

I’d personally prefer that we not put quotes around any of them. And I’m not sure if the MDN style guide provides any guidance on this. But at least I don’t think we have any linter that actually enforces that style.

But maybe this style (putting quotes around the option values but not the option names) is how most MDN pages are styled.

If so, I guess maybe the logic for that would be, the option values are strings but the option names aren’t? Regardless, from just that, I don’t personally think it follows that it should be necessary or useful to quote the option values — especially not in a case like this page, where all the options are limited to an enumerated set of values, and can’t be any arbitrary string.

@aguingand
Copy link
Contributor Author

I don't think there should be quotes either, @Josh-Cena I will remove them (but keep the reordering you made)

@github-actions github-actions bot added size/s [PR only] 6-50 LoC changed and removed size/m [PR only] 51-500 LoC changed labels Aug 5, 2025
@Josh-Cena
Copy link
Member

Josh-Cena commented Aug 5, 2025

We indeed don't have written guidelines, but I'm saying "yes" in https://github.com/orgs/mdn/discussions/779. You are welcome to give opinions for "no". smooth is an identifier and corresponds to { behavior: smooth }; "smooth" is a string and corresponds to { behavior: "smooth" }. Otherwise, false, 1, or NodeFilter.NONE would be ambiguous.

@aguingand
Copy link
Contributor Author

aguingand commented Aug 5, 2025

I guess it should be a separate PR that updates all string values, for now other pages seems to be unquoted (e.g. https://developer.mozilla.org/en-US/docs/Web/API/Element/scrollTo, https://developer.mozilla.org/en-US/docs/Web/API/Element/attachShadow#open...)

@Josh-Cena
Copy link
Member

Alright, okay to keep it in this form

@sideshowbarker sideshowbarker merged commit e4ac7f2 into mdn:main Aug 5, 2025
8 checks passed
@sideshowbarker
Copy link
Member

@aguingand, 👍 and congrats on landing your first docs change here — welcome aboard 🎉

@aguingand
Copy link
Contributor Author

@sideshowbarker Thank you ! Long live MDN !

@aguingand
Copy link
Contributor Author

@sideshowbarker I think BCD PR should be merged too (mdn/browser-compat-data#27463)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:WebAPI Web API docs size/s [PR only] 6-50 LoC changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants