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-multicol] Pass at incorporating the new model plus clarification in the issue re naming #3999

Merged
merged 4 commits into from
Jun 15, 2019

Conversation

rachelandrew
Copy link
Contributor

#1072

Given the fiddliness of this I have opened a PR. I'm just looking at this first part now, with regard to the inclusion of the new model plus the naming we discussed. If we can get this right I'll go through the spec looking for any naming problems that fall out of it. I'll address the parts that deal with spanners specifically separately. I wanted to sort all these anonymous boxes first.

  1. I think we essentially add an extra container (2 in @frivoal's writeup in the issue) when we do this, I've attempted to write that in. We end up with multiple multicol rows in the case of pagination (these contain the box 1 in @frivoal's writeup) and spanners. In future we might then add the concept or rows being created in the block dimension by height restricting the row.

  2. This means a bit of a change to the text below that about gaps, and clarification that gaps are the same across all rows and lines in the container, even if we have multiple rows, or a spanner creates multiple lines.

Please let me know any thoughts.

The gap is the same size in all rows and lines in the container.
@rachelandrew
Copy link
Contributor Author

@frivoal updated based on our discussion today.

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.

First set of review comments...

css-multicol-1/Overview.bs Outdated Show resolved Hide resolved
css-multicol-1/Overview.bs Outdated Show resolved Hide resolved
css-multicol-1/Overview.bs Outdated Show resolved Hide resolved
css-multicol-1/Overview.bs Outdated Show resolved Hide resolved
css-multicol-1/Overview.bs Outdated Show resolved Hide resolved
css-multicol-1/Overview.bs Outdated Show resolved Hide resolved
@rachelandrew rachelandrew merged commit 5afdd93 into master Jun 15, 2019
@rachelandrew rachelandrew deleted the multicol-rows-lines-edits branch June 16, 2019 08:26
@syncbot syncbot restored the multicol-rows-lines-edits branch September 16, 2019 07:54
@plinss plinss deleted the multicol-rows-lines-edits branch September 16, 2019 08:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants