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-gaps-1] Initial editor's draft. #10393 #11115

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

kbabbitt
Copy link
Contributor

Major changes from the unofficial draft:

  • Defer gap decoration areas out of Level 1
  • Drop the direction-relative properties
  • Generalize *-rule-break to apply to all container types
  • Remove the join keyword from *-rule-outset and fold its behavior into *-rule-break
  • Define algorithms for:
    • Determining gap decoration endpoints
    • Expanding repeaters in color/style/width properties

@alisonmaher
Copy link
Collaborator

Drop the direction-relative properties

I can't remember, was this based on feedback from the working group?

@kbabbitt
Copy link
Contributor Author

kbabbitt commented Nov 4, 2024

Formatted version of draft can be found here: https://kbabbitt.github.io/css-gap-decorations/pr-11115/Overview.html

@kbabbitt
Copy link
Contributor Author

kbabbitt commented Nov 4, 2024

Drop the direction-relative properties

I can't remember, was this based on feedback from the working group?

It was based on feedback from the issue discussion.

#10393 (comment)

  • (possibly?) add main-rule and cross-rule for Flexbox (personally I'd prefer to just use row-rule and column-rule)

#10393 (comment)

Me too. There is no main-gap or cross-gap property, so I don't think we need main-rule or cross-rule either.

Copy link

@KurtCattiSchmidt KurtCattiSchmidt left a comment

Choose a reason for hiding this comment

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

Looks great overall, mostly just nits

css-gaps-1/Overview.bs Outdated Show resolved Hide resolved
css-gaps-1/Overview.bs Outdated Show resolved Hide resolved
css-gaps-1/Overview.bs Outdated Show resolved Hide resolved
css-gaps-1/Overview.bs Outdated Show resolved Hide resolved
css-gaps-1/Overview.bs Show resolved Hide resolved
css-gaps-1/Overview.bs Outdated Show resolved Hide resolved
Copy link
Collaborator

@alisonmaher alisonmaher left a comment

Choose a reason for hiding this comment

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

Overall, looks good to me, as well, with a few suggestions below.

css-gaps-1/Overview.bs Show resolved Hide resolved
css-gaps-1/Overview.bs Show resolved Hide resolved
css-gaps-1/Overview.bs Outdated Show resolved Hide resolved
css-gaps-1/Overview.bs Outdated Show resolved Hide resolved
css-gaps-1/Overview.bs Outdated Show resolved Hide resolved
css-gaps-1/Overview.bs Outdated Show resolved Hide resolved
css-gaps-1/Overview.bs Show resolved Hide resolved
css-gaps-1/Overview.bs Outdated Show resolved Hide resolved
css-gaps-1/Overview.bs Outdated Show resolved Hide resolved
css-gaps-1/Overview.bs Show resolved Hide resolved
@kbabbitt
Copy link
Contributor Author

kbabbitt commented Nov 5, 2024

@alisonmaher @KurtCattiSchmidt Thanks for the comments! I've pushed some changes with updates to the text; will work on making more visuals.

@kbabbitt
Copy link
Contributor Author

kbabbitt commented Nov 9, 2024

Now updated with more visuals, and I've pushed an update to the formatted copy at https://kbabbitt.github.io/css-gap-decorations/pr-11115/Overview.html

Ready for another look, thanks!

edit: actually, missed one comment, will continue working on that.

edit^2: last comment addressed.

Copy link
Collaborator

@alisonmaher alisonmaher left a comment

Choose a reason for hiding this comment

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

Changes lgtm, thanks for adding all the visuals. This helped clarify all the questions I had.

The only other thoughts I had were if we should add an open issue for how to handle fragmentation and how to handle subgrid?

Otherwise, this looks good to me, thanks Kevin!

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.

3 participants