-
Notifications
You must be signed in to change notification settings - Fork 522
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
Clarify scrollMargin details #515
Clarify scrollMargin details #515
Conversation
index.bs
Outdated
@@ -298,6 +298,20 @@ interface IntersectionObserver { | |||
passed to the {{IntersectionObserver}} constructor. If no | |||
{{IntersectionObserverInit/scrollMargin}} was passed to the {{IntersectionObserver}} | |||
constructor, the value of this attribute is "0px 0px 0px 0px". | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, my suggestion was not quite correct. The goal is to make sure that all the language describing how scrollMargin
is applied is reachable from the algorithm description for "Compute the Intersection of a Target Element and the Root". That algorithm description currently refers to the [[scrollMargin]]
slot, as distinct from the scrollMargin
argument to the IntersectionObserver constructor.
I think instead it would make sense to put these two paragraphs back where they were, but add a line like 'To <dfn>apply scroll margin to a scrollport</dfn>' above it. You should also move lines 291-292 ("<b>These offsets are only applied...") to the same place; and then refer to this new definition in the algorithm description for "Compute the Intersection of a Target Element and the Root", instead of referring directly to the [[scrollMargin]]
slot.
Hopefully this makes sense...
So I think these two paragraphs should be moved to the place where the [[scrollMargin]]
slot is defined; and the text on lines 291-292 ("These offsets are only applied when handling...") should also be moved there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me know if this looks better.
3d43a1e
to
3b364c5
Compare
3b364c5
to
a47c2fc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, thanks!
SHA: 33a6211 Reason: push, by szager-chromium Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Thank you Stefan! |
RE: "I think this content, which is currently intermingled with the definition of root intersection rect, should be moved to the definition of scrollMargin"
Preview | Diff