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-values] attr()'s type system is inconsistent with other things #10437

Closed
andruud opened this issue Jun 12, 2024 · 5 comments
Closed

[css-values] attr()'s type system is inconsistent with other things #10437

andruud opened this issue Jun 12, 2024 · 5 comments

Comments

@andruud
Copy link
Member

andruud commented Jun 12, 2024

As briefly discussed with @tabatkins during CSS Day:

  • In @property, we have "<length>", and similar.
  • We intend to use the same for the parameter types of custom functions (probably).
  • We have a loose plan of dropping the string-part of the syntax string, making it possible to specify <length> directly. (https://github.com/andruud/csswg-drafts/pull/2/files). EDIT: This now exists, but only for css-mixins-1.

Why doesn't attr() use the same way of specifying types? I.e., shouldn't we do attr(data-foo <length>, 0px) instead of attr(data-foo length, 0px)?

EDIT: I accidentally posted the issue template.

@andruud andruud changed the title [css-values] attr() [css-values] attr()'s type system is inconsistent with other things Jun 12, 2024
@andruud
Copy link
Member Author

andruud commented Aug 28, 2024

Agenda+ on behalf of @tursunova.

@Crissov
Copy link
Contributor

Crissov commented Aug 29, 2024

#6408 proposes explicit type conversion functions and #3702 a string parsing function. Both approaches could be used instead of a type hint within attr().

@tursunova
Copy link

#3702 was already closed, since it doesn't bring extra functionality and attr() is easier to read, compare #3702 (comment).

Even if it will be decided to add a conversion function, as per #6408, I don't think we would like to remove <attr-type> from the grammar. So adding angle brackets would make attr() grammar more compatible with other things, like registered custom property syntax types.

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed [css-values] attr()'s type system is inconsistent with other things, and agreed to the following:

  • RESOLVED: Use the <syntax> production that we hae defined in the mixins spec, consistent with what we will be allowing in registered custom properties
The full IRC log of that discussion <kbabbitt> moonira: in attr() grammer we define without angle brackets which is inconsistent with other custom property syntax
<kbabbitt> ... we have a prototype of attr() in Blink which we are going to ship
<kbabbitt> ... propose to resolve this issue by making attr() consistent with @Property using angle brackets
<kbabbitt> TabAtkins: want to make sure all the places use consistent syntax; attr() was designed before we came up with this other stuff
<kbabbitt> astearns: and this is ocnsistent ewith type decision we made earlier
<kbabbitt> TabAtkins: yes
<lea> +1 seems like an obvious win to me
<kbabbitt> astearns: questions?
<chrishtr> q+
<kbabbitt> Proposed: Use the <syntax> production that we hae defined in the mixins spec, consistent with what we will be allowing in registered custom properties
<flackr> +1 sgtm
<kbabbitt> lea: (missed)
<fantasai> +!
<astearns> ack chrishtr
<fantasai> s/!/1/
<astearns> s/(missed)/consistent with custom properties etc.
<kbabbitt> chrishtr: question, this is new syntax for attr() and there's no compatibility risk with existing websites?
<kbabbitt> moonira: I don't think it should be a problem
<kbabbitt> chrishtr: attr exists, but it's just not exposed
<kbabbitt> moonira: existing attr() version doesn't include type syntax
<kbabbitt> chrishtr: what you're planning to ship will have type syntax?
<kbabbitt> moonira: correct
<kbabbitt> chrishtr: so because it's new syntax, there will not be a compat concern?
<kbabbitt> moonira: yes
<kbabbitt> astearns: other commments or questions?
<kbabbitt> hober: you're right, everyone has simple attr() that's it, should be good. but...
<kbabbitt> ... older spec has had other syntax for a long time, authors do like to do the cool thing they hope will work eventually
<kbabbitt> ... so there is potentially a compat risk, there may be things that were invalid before and would have been ignored
<kbabbitt> chrishtr: good point, it's possible but it seems worth trying to me
<kbabbitt> dbaron: I think it might even be less scary to change it just because suddenly implementing a feature if authors have been trying to write it and not test it for a long time is scary
<TabAtkins> we don't *usually* account for people writing invalid stuff ahead of the spec unless we're pretty sure there's actually a problem
<kizu> +1
<kbabbitt> ... changing the syntax right befor eit ships helps with that
<kbabbitt> chrishtr: are there other older features that have not been shipped with similar conditions?
<kbabbitt> tabatkins: no additional locations, just @Property, @function, attr()
<TabAtkins> @Property @function and @tr
<kbabbitt> astearns: objections?
<kbabbitt> RESOLVED: Use the <syntax> production that we hae defined in the mixins spec, consistent with what we will be allowing in registered custom properties
<matthieud> q+
<astearns> ack matthieud
<lea> q+
<kbabbitt> matthieud: for @Property we need to put the syntax in quotes, but for attr() and @function we don't need uqoes?
<kbabbitt> tabatkins: no, @Property will be updated
<kbabbitt> lea: the string versio will strictly be more powerful because you can still do quantifiers and htings like that?
<kbabbitt> tabatkins: no, you'll be able to use th enew syntax prodyction
<kbabbitt> lea: can you use quantifiers, e.g. 0-5?
<kbabbitt> tabatkins: I don't think that's in the grammer but it's not supported for the string version either
<kbabbitt> ... (missed0
<TabAtkins> like <number [0, 5]>

@tabatkins
Copy link
Member

Edits checked in, including moving the <syntax> production over to Values 5 since multiple specs are using it now.

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

No branches or pull requests

5 participants