Skip to content

Reduce the number of text-wrap features to just 3 #2900

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

foolip
Copy link
Collaborator

@foolip foolip commented Apr 25, 2025

Only text-wrap: balance and text-wrap: pretty are significant new
features, let everything elese be covered by the generic text-wrap
feature.

This changes the Baseline date of text-wrap a little, but it's still
in 2024 and so it doesn't seem worth trying to pin it to the earlier
date by trimming the compute_from list in a way that wouldn't match
the feature decription.

Fixes #2543.

Only `text-wrap: balance` and `text-wrap: pretty` are significant new
features, let everything elese be covered by the generic `text-wrap`
feature.

This changes the Baseline date of `text-wrap` a little, but it's still
in 2024 and so it doesn't seem worth trying to pin it to the earlier
date by trimming the `compute_from` list in a way that wouldn't match
the feature decription.

Fixes web-platform-dx#2543.
@github-actions github-actions bot added the feature definition Creating or defining new features or groups of features. label Apr 25, 2025
@foolip foolip requested a review from ddbeck April 25, 2025 07:43
@ddbeck ddbeck added the major version required This PR requires a minor version semver release (vX+1.0.0) label Apr 25, 2025
@ddbeck
Copy link
Collaborator

ddbeck commented Apr 25, 2025

I'm happy for this to happen at some point, but wiping out features is a semver major event. I don't really want to do that as a one-off for this. So either this is blocked pending a resolution for at least #91 and a resolution on as-yet un-described "inferior feature" capability (i.e., the structure a merged feature becomes).

@foolip
Copy link
Collaborator Author

foolip commented Apr 28, 2025

@ddbeck what's the "inferior feature" capability you have in mind? Here, I'm imagining that all removed features are simply redirected to the merged text-wrap feature.

@ddbeck
Copy link
Collaborator

ddbeck commented Apr 28, 2025

Philip and I chatted about this today and he made an OK argument for the idea that these aren't real merges, but actually a kind of error correction (i.e., we created features that didn't/shouldn't really exist and can use redirects as proposed in #91 instead of something richer/more complex, at least initially).

Given that context, I said I'd look at this again. I had a hard time following all the moving parts here, so I drew a diagram. I think this shows how things are moving around here:

flowchart LR
    text-wrap
    text-wrap-balance
    text-wrap-mode
    text-wrap-nowrap
    text-wrap-pretty
    text-wrap-stable
    text-wrap-style

    text-wrap-stable --> text-wrap

    text-wrap-mode --> text-wrap

    text-wrap-nowrap --> text-wrap

    text-wrap-style -- text-wrap-style: auto
    text-wrap-style: nowrap --> text-wrap
    text-wrap-style -- text-wrap-style: balance --> text-wrap-balance
    text-wrap-style -- text-wrap-style: pretty --> text-wrap-pretty
Loading

I can see Philip's point here. I'd prefer the purist (and purest) version, but simple redirects could work here. I suggest we come back to this as soon as #91 is resolved, which will hopefully be soon.

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

Successfully merging this pull request may close these issues.

Too many text-wrap features
2 participants