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-grid] Error in distribute space to spanned tracks algo #1729

Closed
fantasai opened this Issue Aug 16, 2017 · 5 comments

Comments

Projects
None yet
4 participants
@fantasai
Contributor

fantasai commented Aug 16, 2017

Testcase: http://software.hixie.ch/utilities/js/live-dom-viewer/?saved=5307
Spec: https://www.w3.org/TR/css-grid-1/#distribute-extra-space

The spec currently says

Distribute the space equally to the planned increase of each spanned track with an affected size, freezing tracks as their planned size reaches their growth limits (and continuing to grow the unfrozen tracks as needed).

However, it's unclear about what "distribute the space" means. It's easy to conclude that it should incorporated by adding to the "planned increase", but actually it needs to be incorporated by ensuring the "planned increase" is at least that amount, i.e. by max()ing with the "planned increase".

This is necessary to ensure the spec's stated goal of “(This prevents the size increases from becoming order-dependent.)” The result should be as Chrome currently calculates it: if you have a series of items spanning two tracks each, then the tracks should be equally sized.

@fantasai fantasai added the css-grid-1 label Aug 16, 2017

@fantasai

This comment has been minimized.

Contributor

fantasai commented Aug 16, 2017

Note: This error is a mistranscription of the original Microsoft algorithm, which called for maxxing rather than adding, see description at the top of https://www.w3.org/TR/2014/WD-css-grid-1-20140123/#function-ResolveContentBasedTrackSizingFunctionsForItems

@astearns

This comment has been minimized.

Member

astearns commented Sep 27, 2017

Reopening for testcase need.

@astearns astearns reopened this Sep 27, 2017

@tabatkins tabatkins closed this Sep 29, 2017

@zcorpan

This comment has been minimized.

Member

zcorpan commented Nov 6, 2017

@tabatkins can you point to the testcase in web-platform-tests? If there isn't one, this should be open?

@tabatkins

This comment has been minimized.

Member

tabatkins commented Nov 6, 2017

No, we reversed the "must leave the issue open until it gets a test" decision, because it made things too noisy for differentiating between whether an issue was addressed or not.

(Also, next time you want to ask the same question on a dozen issues, maybe just ask it once and link to the other issues, or do the asking out-of-band? Now I have to decide between responding to every single issue again, or making it look like I'm just leaving you hanging on all the other threads.)

@zcorpan

This comment has been minimized.

Member

zcorpan commented Nov 6, 2017

OK, thanks.

(Yeah, sorry about that. I'll clean it up.)

edit: I've now removed my comment from the other issues.

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