Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[css-conditional-5] Draft spec for scroll-state() #6402 #10798

Merged
merged 6 commits into from
Sep 4, 2024
Merged

[css-conditional-5] Draft spec for scroll-state() #6402 #10798

merged 6 commits into from
Sep 4, 2024

Conversation

lilles
Copy link
Member

@lilles lilles commented Aug 28, 2024

Per resolution in [1], add scroll-state() as a query function to query sticky, snap, and overflow scroll states with a new container-type for scroll-state queries.

[1] #6402 (comment)

[css-conditional-5] Draft spec for scroll-state() w3c#6402
6b08d4a 
Per resolution in [1], add scroll-state() as a query function to query
sticky, snap, and overflow scroll states with a new container-type for
scroll-state queries.

[1] w3c#6402 (comment)
@lilles lilles requested a review from dbaron August 28, 2024 11:11
dbaron
dbaron reviewed Aug 28, 2024
View reviewed changes
Copy link
Member

@dbaron dbaron 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 reasonable. One big question at the top (about whether it's supposed to be in this file at all), and then a bunch of specific comments on the text.

css-conditional-5/Overview.bs Show resolved Hide resolved
css-conditional-5/Overview.bs Outdated Show resolved Hide resolved
css-conditional-5/Overview.bs Show resolved Hide resolved
css-conditional-5/Overview.bs Outdated Show resolved Hide resolved
css-conditional-5/Overview.bs Outdated Show resolved Hide resolved
css-conditional-5/Overview.bs Outdated Show resolved Hide resolved
css-conditional-5/Overview.bs Outdated Show resolved Hide resolved
css-conditional-5/Overview.bs Outdated Show resolved Hide resolved
css-conditional-5/Overview.bs Outdated Show resolved Hide resolved
css-conditional-5/Overview.bs Show resolved Hide resolved
@lilles lilles requested a review from fantasai August 29, 2024 08:49
svgeesus
svgeesus approved these changes Sep 2, 2024
View reviewed changes
Copy link
Contributor

@svgeesus svgeesus left a comment

Choose a reason for hiding this comment

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

Overall approve, as I prefer to see good PR merged sooner and details worked out later. But I will wait for one of the other editors to do the merge. Not all of the @dbaron comments have been resolved.

@lilles
Copy link
Member Author

lilles commented Sep 3, 2024

Overall approve, as I prefer to see good PR merged sooner and details worked out later. But I will wait for one of the other editors to do the merge. Not all of the @dbaron comments have been resolved.

Thanks!

@svgeesus
Copy link
Contributor

svgeesus commented Sep 3, 2024

@dbaron are you now satisfied with the responses to your helpful comments on this PR? If so please r+

dbaron
dbaron approved these changes Sep 3, 2024
View reviewed changes
Copy link
Member

@dbaron dbaron left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I left four new comments (two of which show up directly above, and two of which only show up in the middle of the earlier threads), but I'm fine with this landing either in its current state or with an attempt to address those.

@lilles lilles merged commit 9468285 into w3c:main Sep 4, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dbaron dbaron dbaron approved these changes

@svgeesus svgeesus svgeesus approved these changes

@fantasai fantasai Awaiting requested review from fantasai

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@lilles @svgeesus @dbaron