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

should calc-size and interpolate-size features be combined? #2246

Closed
dbaron opened this issue Nov 14, 2024 · 8 comments
Closed

should calc-size and interpolate-size features be combined? #2246

dbaron opened this issue Nov 14, 2024 · 8 comments
Labels
feature definition Creating or defining new features or groups of features. major version required This PR requires a minor version semver release (vX+1.0.0)

Comments

@dbaron
Copy link

dbaron commented Nov 14, 2024

#2110 added a feature for interpolate-size and #2148 added a feature for calc-size. However, these are very closely related things, defined in a single spec section, that will hopefully be shipped at the same time in all engines that support them (and was certainly the case when they both shipped in Chromium).

I'm not quite sure what the standard is for when things should be part of a single WebDX feature... but I would have thought these should be a single feature rather than two separate ones.

cc @vwallen @ddbeck

@captainbrosset
Copy link
Contributor

Thank you, David, for filling this.

For reference, here are the two features in the explorer:

To me, the question is less about whether the features are part of the same spec, or whether they ship at the same time. We need to think about it from a developer stand point:

Would a web developer always use those two features together to achieve a certain scenario? Or, are there cases when they might use them separately, for different things?

@dbaron
Copy link
Author

dbaron commented Nov 15, 2024

Would a web developer always use those two features together to achieve a certain scenario? Or, are there cases when they might use them separately, for different things?

I'm not sure that question makes sense, because I think the best way to think about them is as two interfaces to the same underlying feature: calc-size() is the lower-level primitive and interpolate-size is the much easier-to-use higher-level interface that covers the main use case in a single line (and uses calc-size() underneath).

@captainbrosset
Copy link
Contributor

In which case grouping them makes sense to me.

@ddbeck ddbeck added feature definition Creating or defining new features or groups of features. major version required This PR requires a minor version semver release (vX+1.0.0) labels Nov 15, 2024
@ddbeck
Copy link
Collaborator

ddbeck commented Nov 15, 2024

I'm not opposed to this change, but I think it's likely to be confusing to developers. If we make this change, then we should take some steps to alert the responsible parties that this relationship is poorly communicated. Otherwise, I think we're likely to see a "where's the calc-size() feature?" issue.

What I was thinking

As a reviewer on both PRs, the close relationship between these features was not at all obvious. I considered other groupings and the one proposed here was not one of them. I turned to the spec and docs:

  • The calc-size() section in the spec opens with a comparison with calc(), implying that is the thing it's most-closely related to. Interpolation isn't highlighted until the third subsection, which doesn't exactly scream "this is why this exists."
  • The MDN page for calc-size() opens with a comparison to calc(). That page takes four examples before it reaches one involving animations.

Meanwhile the interpolate-size spec section requires the reader to be pretty eagle-eyed to notice the mention of calc-size(). Likewise on MDN, where the only calc-size() mentions are in a note at the end of the page and a See also bullet.

Finally, the existing features have distinct support data. It doesn't look like they shipped together in Edge.

What we should do next

If we're going to merge these features, then I'd expect the following:

  • Someone to check whether Edge is or isn't shipping interpolate-size. Right now browser compat data says that it's not. This is probably wrong and it'd be great for someone to open a PR to fix it.

    (If it's correct—that Edge opted-out of interpolate-size but not calc-size()—then that seems like a big enough deal to derail this proposal.)

  • An issue opened against the spec, asking to make this relationship explicit:

    calc-size() is the lower-level primitive and interpolate-size is the much easier-to-use higher-level interface

    This would be especially helpful for our spec links. Being able to link to one spec section that mentions both of these things in the same breath would great.

  • An issue opened against MDN, also asking to make the relationship explicit on both pages.

  • The description of calc() to be expanded with "not to be confused with" text.

@captainbrosset
Copy link
Contributor

Someone to check whether Edge is or isn't shipping interpolate-size. Right now browser compat data says that it's not. This is probably wrong and it'd be great for someone to open a PR to fix it.
(If it's correct—that Edge opted-out of interpolate-size but not calc-size()—then that seems like a big enough deal to derail this proposal.)

This is a mistake. interpolate-size seems to work for me in Edge.

I think this is a probably due to the bcd collector somehow. Chris Mills had properly "mirrored" Edge's status in mdn/browser-compat-data#23863 but the PR got closed in favor of this bcd collector one which didn't mirror Edge to Chrome: mdn/browser-compat-data#24210

@dbaron
Copy link
Author

dbaron commented Nov 15, 2024

They're currently in the same spec section already (also see the table of contents for the structure). And the explainer for the feature is perhaps clearer about the relationship than the spec is.

I've been doing a significant part of the work driving these features to being specified and to ship, and basically all of the communication around them (since interpolate-size was added) has talked about them together, from the single spec section and explainer (both linked above), the chromestatus and intents, to the web.dev blog post. Part of the reason for this is that we don't want developers to reach for calc-size() when interpolate-size would address their needs.

So I filed this mainly because I was a bit surprised to see them separate, and also because it's going to be at least a little bit of extra work to correctly tag the WPT tests to the two separate features rather than a single one.

That said, I'm ok with leaving them separate if you think that's better.

@dbaron
Copy link
Author

dbaron commented Nov 15, 2024

That said, an argument for keeping them separate is that it probably is interesting to track their usage levels separately.

@dbaron
Copy link
Author

dbaron commented Nov 15, 2024

I'm going to just go ahead and add the test annotations and use counters for separate features, so I guess I'll close this.

@dbaron dbaron closed this as completed Nov 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature definition Creating or defining new features or groups of features. major version required This PR requires a minor version semver release (vX+1.0.0)
Projects
None yet
Development

No branches or pull requests

3 participants