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-shapes-1] ellipse() grammar gratuitously inconsistent with radial-gradient() #824

Closed
tabatkins opened this issue Dec 21, 2016 · 14 comments

Comments

@tabatkins
Copy link
Member

The ellipse() grammar defined in Shapes was clearly designed to resemble the grammar of radial-gradient(), but it's different in small but important ways, for no apparent reason.

In particular, the ellipse part of the radial-gradient() grammar is: <extent-keyword> | <length-percentage>{2}. You either say something like "farthest-side", which defines an ellipse, or give lenpers for both axises.

On the other hand, the ellipse() function says [ <extent-keyword> | <length-percentage> ]{2}? - for each axis, you can either provide a keyword or a lenper. So you can do something like ellipse(farthest-side closest-side), to make its width the amount necessary to hit the farthest horizontal side, and its height the amount necessary to hit the closest vertical side. But you can't do ellipse(closest-side).

Given the seniority of the radial-gradient() syntax, and the lack of major issues with it, I think we should converge on that as the grammar for ellipse().

@AmeliaBR
Copy link
Contributor

There's an additional inconsistency that also applies to the circle() function: only -side keywords are allowed, not -corner keywords.

Was there any reason for not allowing circle() and ellipse() shape functions to use closest-corner and farthest-corner to define radii?

@astearns
Copy link
Member

@AmeliaBR closest-corner and farthest-corner did not seem necessary for shape-outside and clip-path, as using those for radii often creates a shape that extends outside the margin box. I'm not against allowing them for consistency's sake, but I don't know that they would get much use.

@AmeliaBR
Copy link
Contributor

@astearns The reference box for sizing a clip-path or shape-outside doesn't need to be the margin-box. A circle sized to the content box's corners could still fit within the margin box. And the shape functions may be adopted for other uses.

But regardless of the strength of use cases, I agree with Tab's original point that consistency is preferred.

@astearns
Copy link
Member

I should have addressed this a few years ago. Sorry for the delay.

Adding the *-corner keywords should be fine.

My guess is it's still possible to change the ellipse() syntax to exactly match radial gradients. If we do that, there will be some breakage. It would be limited to instances of ellipse() where two keywords are used (which is likely very small), and the breakage would be limited to having someone's shape-outside or clip-path no longer apply (both have okay fallbacks).

BUT I think we could change ellipse() to allow either what's currently specified OR something matching a current radial gradient:

<extent-keyword> | [ <extent-keyword> | <length-percentage> ]{2}?

Then there would be no breakage, no one would get a surprise trying to copy a radial gradient ellipse into an ellipse(), we would get the two-keyword case (which is useful for creating ellipses that are the largest that can fit in a reference box), and we could decide whether to extend the radial gradient syntax to match (fully containing the ellipse is less useful there, but still could be good)

@astearns
Copy link
Member

Hmm. If we allowed two keywords and both *-side and *-corner, then we'd have to define what it means when both keywords were *-corner. It's not immediately obvious to me what the resulting ellipse should be in those cases.

@AmeliaBR
Copy link
Contributor

If we allowed two keywords and both *-side and *-corner, then we'd have to define what it means when both keywords were *-corner. It's not immediately obvious to me what the resulting ellipse should be in those cases.

Oh, that's a good point. And may have been the reason the radial-gradient syntax only allows one keyword.

So the natural superset of the two syntaxes is one or two "side" keywords or lengths, or one "corner" keyword. Which is rather more confusing.

@tabatkins
Copy link
Member Author

I can assure you that's not why I wrote radial-gradient the way I did; I just happened to think of the ellipse/circle keywords as an automagical way to write the two sizes.

@astearns
Copy link
Member

astearns commented Oct 25, 2019

Given that closest-corner is defined to create an ellipse with the same aspect-ratio as closest-side that passes through the closest corner point we could do something similar for the two-keyword form:

X-corner Y-corner creates an ellipse with the same aspect-ratio as X-side Y-side that passes through both corners

@jakearchibald
Copy link
Contributor

@astearns

closest-corner and farthest-corner did not seem necessary for shape-outside and clip-path, as using those for radii often creates a shape that extends outside the margin box. I'm not against allowing them for consistency's sake, but I don't know that they would get much use.

I have a use-case!

@keyframes circular-reveal {
  from { clip-path: circle(0 at center); }
  to { clip-path: circle(farthest-corner at center); }
}

Right now this is only possible with JS.

@tabatkins
Copy link
Member Author

Agenda+ at @jakearchibald 's request

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed ellipse() grammar gratuitously inconsistent with radial-gradient(), and agreed to the following:

  • RESOLVED: update the basic shape functions and update radial-gradient
The full IRC log of that discussion <Rossen_> Topic: ellipse() grammar gratuitously inconsistent with radial-gradient()
<Rossen_> github: https://github.com//issues/824
<bramus> Rossen_: last issue
<bramus> astearns: i have one question about latest comment
<bramus> astearns: jake his example is circle not ellipse
<bramus> astearns: we also cover that?
<bramus> TabAtkins: maybe?
<bramus> astearns: does it use nearest/farthest corner?
<bramus> TabAtkins: yes
<bramus> astearns: so proposal would both cover circle and ellipse
<bramus> astearns: looking through the issue
<bramus> astearns: we want to change both circle and ellipse in basic shapes to match radial gradients
<bramus> astearns: and i am fine with that
<bramus> astearns: do we also want to retaint he facility that te basic ellipse function has
<bramus> astearns: which is to define an ellipse that is as large as is possible for a given reference box
<bramus> astearns: if we do this then there will be no breakage with current shape ellipses in use
<bramus> astearns: but we would also wamt to add this to the gradient syntax
<bramus> TabAtkins: yeah
<bramus> astearns: fine going either way
<bramus> Rossen_: lets pick one
<bramus> astearns: do it all
<lea> +1
<bramus> astearns: update the basic shape functions and update radial-gradient
<bramus> Rossen_: opinions?
<bramus> lea: yes, do it!
<bramus> Rossen_: ojbections?
<bramus> slicen
<bramus> Rossen_: resolved
<bramus> RESOLVE: update the basic shape functions and update radial-gradient
<bramus> Rossen_: and that concludes the list of issues
<dbaron> RESOLVED: update the basic shape functions and update radial-gradient

@jakearchibald
Copy link
Contributor

Yay!

@cdoublev
Copy link
Collaborator

cdoublev commented Dec 19, 2023

Follow-up on the recent change.


Is it intentional to not allow mixing an extent keyword and a length/percentage?

That is, <radial-extent>{1,2} | <length-percentage [0,∞]>{1,2} vs. [<radial-extent> | <length-percentage [0,∞]>]{1,2}.


Two-component values remain invalid when specifying circle

What about a single size component value specified with ellipse?

From reading the normative value definition and the prose in CSS Images 3, a single <percentage> is invalid but a single <length> is valid. Browsers follow the value definition in the non-normative note, which disallows a single component value, regardless of its type.

While it would be confusing to allow eg. circle 100% 120%, maybe you can relax this restriction for ellipse? The second value would default to the first one.

Note that it would mean that one of circle, ellipse, or both size component values, should be explicit in the serialization, when a single size component value was specified, because circle 100% does not mean the same as ellipse 100% 100%.

If you prefer to require two size component values with ellipse, maybe this restriction can be hard-coded this way:

<radial-gradient-syntax> = <radial-shape>? [ at <position> ]? , <color-stop-list>
<radial-shape> = [ circle || <radial-size> ] | [ ellipse || <radial-size>{2} ]
<radial-size> = <radial-extent> | <length-percentage [0,∞]>

astearns pushed a commit that referenced this issue Dec 19, 2023
@astearns
Copy link
Member

Can you open a new issue on this followup, @cdoublev?

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

No branches or pull requests

7 participants