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

Add clipping, shapes and masking #1649

Merged
merged 18 commits into from
Sep 19, 2024
Merged

Add clipping, shapes and masking #1649

merged 18 commits into from
Sep 19, 2024

Conversation

jamesnw
Copy link
Collaborator

@jamesnw jamesnw commented Aug 22, 2024

Targeting the CSS4 "Clipping, shapes & masking" group.

@github-actions github-actions bot added the feature definition Creating or defining new features or groups of features. label Aug 22, 2024
@@ -0,0 +1,2 @@
name: Clipping, shapes and masking
parent: css
Copy link
Contributor

Choose a reason for hiding this comment

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

The potential issue with this group is that it now contains features that apply to both CSS and SVG.
Your features make sense to me, and I'm fine with making them apply to both languages, if others are. But in that case, we need to make this group belong to both CSS and SVG (which is not possible today).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good point- I've been thinking we will need to address the overlap between CSS and SVG more thoroughly at some point. Allowing for multiple parents for a group would be complex, so I'll remove the parent group here for now.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is a bit of a surprising result- these features do have some implementation, and I'm not sure if I'm misreading the BCD data, or if there is a bug in generating this.

@jamesnw jamesnw marked this pull request as ready for review August 23, 2024 17:36
@jamesnw
Copy link
Collaborator Author

jamesnw commented Aug 28, 2024

I just saw #1129, which is blocked with more conversation in #1128.

Copy link
Contributor

@mirisuzanne mirisuzanne left a comment

Choose a reason for hiding this comment

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

I like this way of breaking things out to clarify the support for various parts. Left a few comments on things that I think could be clarified.

@@ -0,0 +1,6 @@
name: Animatable clipping paths
description: Clipping paths in CSS are animated using transitions and CSS animations.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
description: Clipping paths in CSS are animated using transitions and CSS animations.
description: Clipping paths in CSS are animated using CSS transitions and animations.

@@ -0,0 +1,6 @@
name: Animatable clipping paths
description: Clipping paths in CSS are animated using transitions and CSS animations.
spec: https://drafts.fxtf.org/css-masking-1/#the-clip-path
Copy link
Contributor

Choose a reason for hiding this comment

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

The 'current' spec:

Suggested change
spec: https://drafts.fxtf.org/css-masking-1/#the-clip-path
spec: https://www.w3.org/TR/css-masking/#the-clip-path

I would update the other spec links to match.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll defer this until resolution of #1785

@@ -0,0 +1,8 @@
name: Clip path boxes
description: The `fill-box`, `stroke-box`, and `view-box` values for the `clip-path` set the part of the shape that will be used to clip the element.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
description: The `fill-box`, `stroke-box`, and `view-box` values for the `clip-path` set the part of the shape that will be used to clip the element.
description: The `fill-box`, `stroke-box`, and `view-box` values for `clip-path` set an edge of the elements box to use as the clipping shape.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I added an apostrophe in element's but wanted to verify that was the intent.

Copy link
Contributor

Choose a reason for hiding this comment

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

the intent was possessive, but I'm dyslexic. The element has a box. :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I just wanted to confirm it wasn't like attorneys general or something 😆

@@ -0,0 +1,10 @@
name: "`path()` shape"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's do this feature in a separate PR (from synchronous discussion)

Copy link
Collaborator

Choose a reason for hiding this comment

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

To elaborate here: the BCD data is very messy. It has subfeatures to describe support in d, shape-outside, and offset-path, but also represents that information as partial support on path. It ought to do one or the other. We'll need to fix this in BCD.

(Please @ me on that PR, when you open it.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Moved to #1799

Copy link
Collaborator

@ddbeck ddbeck left a comment

Choose a reason for hiding this comment

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

A couple small requests and elaborations on previous requests

features/clip-path-animatable.yml Outdated Show resolved Hide resolved
features/clip-path.yml Outdated Show resolved Hide resolved
features/mask-type.yml Outdated Show resolved Hide resolved
@@ -0,0 +1,10 @@
name: "`path()` shape"
Copy link
Collaborator

Choose a reason for hiding this comment

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

To elaborate here: the BCD data is very messy. It has subfeatures to describe support in d, shape-outside, and offset-path, but also represents that information as partial support on path. It ought to do one or the other. We'll need to fix this in BCD.

(Please @ me on that PR, when you open it.)

@jamesnw jamesnw mentioned this pull request Sep 18, 2024
@jamesnw jamesnw requested a review from ddbeck September 18, 2024 16:12
Copy link
Collaborator

@ddbeck ddbeck left a comment

Choose a reason for hiding this comment

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

There's a typo to fix, otherwise this is ready to go. Thank you!

features/clip-path.yml Outdated Show resolved Hide resolved
@ddbeck ddbeck merged commit 5b96b7f into web-platform-dx:main Sep 19, 2024
3 checks passed
@jamesnw jamesnw mentioned this pull request Oct 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature definition Creating or defining new features or groups of features.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants