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-anchor-position] position-try: inset-area() does not reflect CSSWG resolution #10320

Open
fantasai opened this issue May 13, 2024 · 3 comments

Comments

@fantasai
Copy link
Collaborator

fantasai commented May 13, 2024

The minutes of the CSSWG resolution to add position-try include a resolution to add the inset-area syntax into the position-try proposal.

However, what got edited in was not what was discussed (adding <'inset-area'> as an option in the try list), but a new inset-area() function. This is inconsistent both with how we import syntaxes across properties in general (we don't wrap them in a function) and with the WG resolution as most reasonably interpreted (given the logged discussion, which explicitly noted the syntax proposal prior to the resolution).

@tabatkins
Copy link
Member

I don't read the minutes as being particularly specific about the syntax. Notably, the syntax suggestion was introduced by a comment in the IRC discussion, not discussed in the thread at all, and unless I'm missing a comment, there was no discussion from anyone else about the syntax specifically during the meeting. We've similarly interpreted this sort of resolution somewhat loosely wrt precise syntax in the past, once the editors dig in and decide it would be better written slightly differently, so long as the addition agrees with the spirit of the resolution.

That's what happened here - once I got to writing it in, I found the inset-area syntax to not be particularly clear to include directly, and thought the wrapping function made it read much better. I'm also slightly concerned about future extensibility here - if we add anything else that we want to make inline-able, we'll run into similar issues.

We don't anticipate any issues in changing this to be directly embedding the syntax, I just think we shouldn't do it because I think the current syntax reads better and seems more safely extensible.

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed [css-anchor-position] position-try: inset-area() does not reflect CSSWG resolution, and agreed to the following:

  • RESOLVED: position-try: none | [ [<dashed-ident> || <try-tactic>] | <'inset-area'> ]# i.e. remove the inset-area()
The full IRC log of that discussion <keithamus> fantasai: when we discussed position-try proposal we talked about inlining inset area syntax in the list of things to try
<keithamus> ... for a lot of simpler cases you only need to set inset area
<keithamus> ... you wouldnt need a position try rule and manage namespacing etc
<keithamus> ... edited in the spec is a new inset-area function
<keithamus> ... not generally how we pull syntax from one area into another
<keithamus> ... we've never created a function with a syntax that maps to that property
<keithamus> ... not necessary here
<TabAtkins> q+
<keithamus> ... I think the change should be reverted. inset-area value space should be in the position-try list as discussed when we resolved on this
<keithamus> TabAtkins: as I said in the issue - the precise syntax wasn't significantly discussed
<keithamus> ... it was introduced in an IRC comment. Not meaningfully picked over
<keithamus> ... the resolution was weak on syntax
<keithamus> ... editors have had similar leeway
<keithamus> fantasai: not really. if it's ambiguous you should come back to the WG
<keithamus> TabAtkins: in any case it was ambiguous so I found it not to be the most readable. I found a decent change we'd want to inline more stuff in the future
<Rossen4> ack TabAtkins
<keithamus> ... the syntax space might overlap with future extensions
<keithamus> ... so that with the readability right now... I thought the function made it much easier to understand
<keithamus> ... the function, dashed ident... three distinct syntaxes
<fantasai> position-try: top, bottom, left span-bottom, right span-bottom; seems perfectly readable to me
<keithamus> ... the flip keywords, the inset-area function, the dashed ident naming the position try block
<keithamus> ... I'm comfortable with other syntaxes, making it clear the inset-area keywords are specifying that. Maybe the keyword, or area, or whatever
<keithamus> ... I'd like something to indicate what the value is doing
<keithamus> fantasai: I think we haven't ever done this before. Inlining the syntax is perfectly reasonable
<keithamus> ... I'm not sure why listing a bunch of values is unreadable
<keithamus> ... they're not generally combined with the other aspects
<keithamus> ... if we want to intro alignment keywords later we can disambiguate
<keithamus> ... .e.g with a slash
<keithamus> ... this syntax is unprecidented. I'd prefer not to introduce it
<kizu> q+
<Rossen4> ack kizu
<keithamus> kizu: I think I incline towards fantasai's proposal. Later alignment with slash will sound better. Especially if we want to allow using these properties in the try block
<keithamus> ... if you have some specific case
<keithamus> ... new syntax that is unprecedented, authors could later think other properties could have the same thing
<keithamus> ... maybe a separate issue if we want to use it in more properties
<keithamus> TabAtkins: I can live with it
<TabAtkins> (I'm not sure what you mean by "allow in the try block" fwiw, kizu. inset-area *is* allowed in the try block
<TabAtkins> )
<fantasai> PROPOSED: position-try: none | [ [<dashed-ident> || <try-tactic>] | <'inset-area'> ]# i.e. remove the inset-area()
<keithamus> RESOLVED: position-try: none | [ [<dashed-ident> || <try-tactic>] | <'inset-area'> ]# i.e. remove the inset-area()
<kizu> (TabAtkins, I meant self-alignment properties, but I forgot that they're already allowed, yes)

@jwatt
Copy link

jwatt commented Jun 2, 2024

@tabatkins Note that the second paragraph of the Anchor-Based Positioning section still refers to "The inset-area function".

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