Skip to content

[scroll-animations-1] Add self keyword #8471

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 4 commits into from
Mar 10, 2023

Conversation

bramus
Copy link
Contributor

@bramus bramus commented Feb 19, 2023

Fixes #8227.

This PR replaces #8390 which was created from the wrong branch.

@bramus bramus requested review from flackr and fantasai February 19, 2023 14:30
Copy link
Collaborator

@fantasai fantasai left a comment

Choose a reason for hiding this comment

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

You need to define what happens when the element's own box isn't a [=scroll container=].

Also you need to specify [=principal box=]. Some elements have multiple boxes.

@bramus
Copy link
Contributor Author

bramus commented Mar 1, 2023

Thanks for the feedback @fantasai; greatly appreciate it. Did some changes which should address your remarks. Happy to take more pointers, as this is all fairly new to me.

@bramus bramus requested a review from fantasai March 1, 2023 21:45
@fantasai
Copy link
Collaborator

fantasai commented Mar 3, 2023

Looks pretty good! The one other thing to fix is the line breaks.
Read https://rhodesmill.org/brandon/2012/one-sentence-per-line/ ; then take a look at the rest of the document to get a sense of how we're formatting the source.

@fantasai fantasai added the scroll-animations-1 Current Work label Mar 6, 2023
@bramus
Copy link
Contributor Author

bramus commented Mar 7, 2023

Interesting link. Makes sense, instead of cutting off mid-sentence when the line length limit is almost reached. Think I got it right now.

@flackr
Copy link
Contributor

flackr commented Mar 8, 2023

You need to define what happens when the element's own box isn't a [=scroll container=].

I would have thought these cases are covered by the text in the ScrollTimeline interface:

If the source of a ScrollTimeline is an element whose principal box does not exist or is not a scroll container, or if there is no scrollable overflow, then the ScrollTimeline is inactive.

Do we define this for scroll-timeline-name as well since that can target a non scroll container?

@bramus bramus force-pushed the scroll-animations-1-self-keyword branch from 078f25f to e085c9b Compare March 9, 2023 12:21
Co-authored-by: fantasai <fantasai.bugs@inkedblade.net>
@bramus
Copy link
Contributor Author

bramus commented Mar 10, 2023

@fantasai I’ve committed the suggested change.

@fantasai fantasai merged commit b60ccdb into w3c:main Mar 10, 2023
@bramus bramus deleted the scroll-animations-1-self-keyword branch December 30, 2024 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[scroll-animations-1] Allow Anonymous Scroll Progress Timelines to target self
3 participants