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-color-3] opacity should use <<number>> not <<alphavalue>> “which is syntactically a <<number>>” #3139

Closed
fantasai opened this issue Sep 24, 2018 · 11 comments

Comments

@fantasai
Copy link
Collaborator

The value of opacity looks and behaves exactly as a <<number>>. It should be defined as actually being a <<number>>, so that it's clear. This will hook into future specs like Animations more correctly.

This should just be an editorial change.

@svgeesus
Copy link
Contributor

I think that comment also applies to CSS Color 4 definition of opacity

This would be an erratum on Color 3, but an editorial change on Color 4.

@tabatkins
Copy link
Member

I'm fine with killing <alphavalue>, but note that it's now equivalent to <number> | <percentage>, not just <number>.

@AmeliaBR
Copy link
Contributor

Note that SVG 2 currently references the <alphavalue> type for various properties. So if you decide to kill it, please make sure that an issue (or ideally a PR) gets filed on SVG to match up.

But I do agree that, since the limits on the value are not enforced by the parser, it is grammatically simpler to just use the raw data types. The benefit of having a designated type really only comes in when you start trying to parse the syntax for the color functions, where it helps explain the meaning of the value.

@svgeesus
Copy link
Contributor

The benefit of having a designated type really only comes in when you start trying to parse the syntax for the color functions, where it helps explain the meaning of the value.

Yes, I thought that was the point. I hadn't realized it would somehow affect the animation spec (and don't understand why it would).

@svgeesus
Copy link
Contributor

It seems desirable (for readability, for cross-spec linking and consistency) to say that something is an &lt;alphavalue.. Can't we just (continue to) define the term, and the definition is <number> | <percentage> ? Would that still work for animation to be correctly defined?

@svgeesus
Copy link
Contributor

ping @fantasai @tabatkins @AmeliaBR do we:

  1. remove <alphavalue> and replace it with <number> | <percentage>
  2. define <alphavalue> as <number> | <percentage> (and do what to make animations work)?

I'd like to close off this issue, get Color 4 updated and Color 3 errata'ed.

@svgeesus
Copy link
Contributor

svgeesus commented Sep 1, 2020

re-ping @fantasai @tabatkins @AmeliaBR

@AmeliaBR
Copy link
Contributor

AmeliaBR commented Sep 1, 2020

I don't see any problem with continuing to define <alphavalue> as an intermediate syntax for the specs. Animations should still be fine.

The fact that it is now a union of two types makes it much more useful to continue to have a shorthand syntax.

@tabatkins
Copy link
Member

Agreed; I think it's useful to leave it as a production to ensure consistency.

Animation isn't an issue; animations drill down to the base values of any production to figure out how to animate if needed.

Note, tho, that you can't animate between number and percentage naively, because they can't be combined in calc(). We should probably specially define this to allow animation with them, since they're equivalent - either converting both to a number or both to a percentage. We can reuse the language from https://drafts.csswg.org/css-values/#combine-numbers et al.

@AmeliaBR
Copy link
Contributor

We should probably specially define this to allow animation with them, since they're equivalent

I think that's already handled by saying the percentages compute to numbers, isn't it?

@tabatkins
Copy link
Member

Ah, I missed that. Yes, we're cool then.

@svgeesus svgeesus added css-color-4 Current Work and removed Agenda+ labels Sep 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants