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.

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

[css-scroll-snap] scroll-snap-align values assigned backwards #2232

Closed
fantasai opened this issue Jan 28, 2018 · 6 comments
Closed

[css-scroll-snap] scroll-snap-align values assigned backwards #2232

fantasai opened this issue Jan 28, 2018 · 6 comments

Comments

@fantasai
Copy link
Collaborator

We decided that the first keyword should affect the block axis, the second the inline axis: and defined it correctly for the place-* shorthands of the alignment properties, but seem to have specified it backwards for css-scroll-snap. :(

@majido
Copy link
Contributor

Filed Chromium issue to fix this before we ship. However Safari is shipping the scroll-snap-align with inline/block order so there is interop risk here.

Is there any indication that they will switch as well? otherwise we need to evaluate the interop risk here.

@majido
Copy link
Contributor

majido commented Jun 25, 2018

Originally posted here.

I ran a query against httparchive database to better understand the compat risk. My exact query is below if anyone wants to replicate [1].

Only 4 were using the scroll-snap-align with two arguments. This is out of 450k pages so usage and breakage is very low AFAICT. This gives me more confidence to make the switch. We should ensure Safari is willing to make the switch to avoid interop issues.

Here is the result:

bodies_page match bodies_url
http://www.scrimba.com/ scroll-snap-align:center none; https://scrimba.com/assets/cached-client-fab8900c04b0f8f240d73f6c6a561770.css
http://www.participate.com/ scroll-snap-align: 0 50px; https://www.participate.com/webpack/style-fe837b4781471be76fc4b0b20871f197.css
http://www.zikinf.com/ scroll-snap-align:center none; https://www.zikinf.com//_css/default/wk/annonces.1513606238.css
http://www.chuzefitness.com/ scroll-snap-align: none start; https://chuzefitness.com/wp-content/themes/chuze-reflex/css/theme.css?ver=1.5.5

Note that one of the cases is actually using it incorrectly.

[1] BigQuery query:

SELECT bodies.page, REGEXP_EXTRACT(bodies.body, r'(scroll-snap-align:\s*\w+?\s+\w+?\s*;)') as match, bodies.url, 
  FROM [httparchive:har.2018_02_01_chrome_requests_bodies]
  AS bodies
JOIN (
  SELECT 
    page, url,
    JSON_EXTRACT(payload, '$._contentType') AS contentType
  FROM [httparchive:har.2018_02_01_chrome_requests]
) AS requests ON requests.url=bodies.url AND requests.page=bodies.page
WHERE (requests.contentType CONTAINS 'html' OR requests.contentType CONTAINS 'css')
AND REGEXP_MATCH(body, r'(scroll-snap-align:\s*\w+?\s+\w+?\s*;)')
LIMIT 10000

@majido
Copy link
Contributor

@smfr do you have any opinion on this change one way or another? It seems a safe change to me so I plan to match the spec in Chromium implementation.

@fantasai
Copy link
Collaborator Author

@majido
Copy link
Contributor

FWIW, I filed a webkit bug since Safari is still shipping the old ordering:

@rachelandrew
Copy link
Contributor

I've also added a note to the MDN docs for scroll-snap-align

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants