Skip to content

[css-values] Cyclic definitions with relative units #2115

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

Closed
Loirooriol opened this issue Dec 17, 2017 · 10 comments
Closed

[css-values] Cyclic definitions with relative units #2115

Loirooriol opened this issue Dec 17, 2017 · 10 comments

Comments

@Loirooriol
Copy link
Contributor

CSS Values 4 introduces the lh unit. How exactly does the following resolve?

* {
  line-height: 2em;
  font-size: 3lh;
}

Or using registerProperty from CSS Properties and Values,

CSS.registerProperty({
  name: "--prop",
  syntax: "<length>",
  initialValue: "0"
});
* {
  --prop: 2em;
  font-size: var(--prop);
}

You can use the codes above to make the computed value of font-size depend on the computed value of another property that depends on the computed value of font-size.

I think you should generalize this:

When used in the value of the font-size property (or the line-height property, for the lh and rlh units) on the element they refer to, these units refer to the computed font metrics of the parent element

into this:

For each element, create a directed dependency graph, containing nodes for font-size, line-height and any other property which uses some font-relative unit. For each use of one of these units unit in a property prop on the element unit refers to, add an edge between prop and line-height if unit is lh or rlh, or between prop and font-size otherwise. Edges are possible from a property to itself. If there is some cycle in the dependency graph, each font-relative unit that added an edge in that cycle refers to the computed font metrics of the parent element (or the computed font metrics corresponding to the initial values of the font property, if the element has no parent).

Then,

parent {
  line-height: 2px;
  font-size: 3px;
}
child {
  line-height: 5em; /* 5 * 3px = 15px, em refers to parent */
  font-size: 7lh; /* 7 * 2px = 14px, lh refers to parent */
}
/*  line-height ──────┐
    ▲                 ▼
    └──────────── font-size  */
parent {
  line-height: 2px;
  font-size: 3px;
}
child {
  line-height: 5px;
  --prop: max(1em, 1lh); /* max(3px, 5px) = 5px, em refers to parent, lh does not */
  font-size: var(--prop); /* 5px */
}
/*  ┌────── --prop ──────┐
    ▼          ▲         ▼
line-height    └──── font-size  */

Another approach could be resolving font-size and line-height first, by making relevant font-relative units and var() functions retrieve the value from the parent.

parent {
  --prop: 2px;
  font-size: 3px;
}
child {
  font-size: var(--prop); /* 2px, --prop refers to parent */
  --prop: 5em; /* 5 * 1px = 5px */
}
/*  --prop ──────┐
       ▲         ▼
       └──── font-size  */

But I don't think CSS Values should mess with CSS Variables by altering what var() does.

@frivoal
Copy link
Collaborator

frivoal commented Dec 18, 2017

I think there's a simpler solution, and it is the one I intended when I proposed the text, but it looks like that got lost along the way, or the the initial wording may have been ambiguous as well (this commit seems to be the culprit to me: 95f91ae).

I think lh should always resolve to the metrics of the parent when used in either font-size and in line-height, not just when used in line-height.

This rules out setting the line-height to something manually to some absolute value, and then setting the font-size on the same element to a function of that, which your approach would enable, but I don't think there's a use case for that, and always going for the parent is much simpler.

So, it seems that a sufficient fix would be to change/clarify:

When used in the value of the font-size property (or the line-height property, for the lh and rlh units) on the element they refer to, these units [...]

into

When used in the value of the font-size property (and additionally for the lh and rlh units, in the the line-height property) on the element they refer to, these units [...]

@Loirooriol
Copy link
Contributor Author

@frivoal Seems reasonable, but I think this does not solve the problem when using CSS Variables that compute to a <length>.

@Nadya678
Copy link

Definition of max recurency cycles if var is used? If next request to calculate then write 0 instead of calculating n times?

@frivoal
Copy link
Collaborator

frivoal commented Dec 18, 2017

@Loirooriol I think the problem of cycles in CSS Variables that compute to a <length> should be filed on https://drafts.css-houdini.org/css-properties-values-api/ rather than here, as that's the spec that introduces the problem. font relative units are fine on their own (if we include the lh fix discussed above), and custom properties don't break it when they're not typed, as they're just tokens that get expanded in place. It's the calculation of computed values introduced in that spec that introduces the loop. It looks like there's already an issue file on that spec to deal with that question: w3c/css-houdini-drafts#315

So I think we should limit the discussion in this issue to resolving the lh cycle, and I think what I proposed above works fine.

@frivoal frivoal self-assigned this Dec 18, 2017
@css-meeting-bot
Copy link
Member

The Working Group just discussed [css-values] Cyclic definitions with relative units, and agreed to the following resolutions:

  • RESOLVED: define lh unit resolution to be that of the parent when the lh unit is spec on line height or font size
The full IRC log of that discussion <dael> Topic: [css-values] Cyclic definitions with relative units
<dael> github: https://github.com//issues/2115
<fantasai> fantasai^: Styling the table as all 'display inline' would have no effect on whether these pseudo-classes matched
<dael> florian: There's already a bit of prose that says if you try to use the em usit in font size property or any usit that depends on font size in the font size property we say you fetch from the parent to avoid a loop.
<dael> florian: There was a poorly owrded way to attach this to lh when defining line height. TH emis-phased part meant we still have a problem if trying to do line height in terms of font size.
<dael> florian: What I tried to do is say all font dependant units go to the parent and in addition lh also goes tot he parent if you use it in line height. That breaks the loops.
<dael> florian: We could have a less blunt approach, but I don't think there's use cases for the more subtile variants.
<dael> florian: Precise wording is in the issue.
<dael> fantasai: Works for me.
<dael> myles: I agree we shouldn't be in the business of trying to do a complex dependency analysis.
<dael> Rossen_: I'm waiting for others to have a moment to consider.
<dael> dbaron: The proposal is that lh goes to the parent if used in font size or line height.
<dael> florian: Yes.
<dael> dbaron: Okay.
<dael> ericwilligers: Other things like height?
<dael> florian: No, there's no loop.
<dael> ericwilligers: I know there's no loop, but seems confusing if lh appears 5 times and means 5 different things.
<dael> ??: we have that with ems
<dael> ericwilligers: Don't want to make it worse.
<dael> florian: We need to keep toehr cases working because that's the point of hte lh unit. Only sensible option would be to ban it from line ehight.
<dael> Rossen_: But for the same reason we should have banned em from font.
<dael> florian: And we didn't
<dael> Rossen_: And I think anyone that used em in fonts understands how they did it.
<dael> Rossen_: So, obj to define lh unit resolution to be that of the aprent when the lh unit is spec on line height or font size?
<myles> +1
<dbaron> The cases that go to the parent here are probably cases that can be safely recommended against.
<dael> RESOLVED: define lh unit resolution to be that of the parent when the lh unit is spec on line height or font size
<dael> dbaron: Should the spec have a not saying we suggest not using that behavior? Suggest it's poor practice to use it that way
<dael> myles: Is there one for em?
<dael> dbaron: I don't htink so.
<dael> florian: If we had a plh unit I would say it is, but since we don't I'm not sure it's bad practice.
<dael> Rossen_: I'm fine with either note or no note. If dbaron you feel it's really nec?
<dael> dbaron: I don't. It's just a thought.

@frivoal
Copy link
Collaborator

frivoal commented Jan 4, 2018

PR for tests over here: web-platform-tests/wpt#8886

@fantasai
Copy link
Collaborator

fantasai commented Jan 4, 2018

Reopening, because I think this needs further edits.

@fantasai fantasai reopened this Jan 4, 2018
@frivoal
Copy link
Collaborator

frivoal commented Jan 5, 2018

@fantasai What did I miss?

@frivoal
Copy link
Collaborator

frivoal commented Jan 31, 2018

ping @fantasai. Could you say what you think is missing here?

@frivoal
Copy link
Collaborator

frivoal commented Mar 29, 2018

ping @fantasai

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

5 participants