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

Update scroll to text fragment #2459

Merged
merged 3 commits into from
Dec 18, 2024

Conversation

jamesnw
Copy link
Collaborator

@jamesnw jamesnw commented Dec 17, 2024

See #1007 and #732 for context.

Essentially, the feature itself is not in BCD, so the status in manually set. This adds Firefox support, which makes it baseline low (and matches Caniuse).

Separately, feature detection was not added in Safari, and has been sitting behind a feature flag. It will be enabled by default in Safari at some later date, per WebKit/WebKit#36413.

This adds the keys for feature detection. We'll need to do more cleanup once Safari supports feature detection.

@github-actions github-actions bot added the feature definition Creating or defining new features or groups of features. label Dec 17, 2024
@ddbeck
Copy link
Collaborator

ddbeck commented Dec 18, 2024

Essentially, the feature itself is not in BCD, so the status in manually set. This adds Firefox support, which makes it baseline low (and matches Caniuse).

I think this might actually exist as html.elements.a.text_fragments. It's slightly vague, but the version numbers line up. I'm not sure if you want to add it in this PR or not.

Copy link
Collaborator

@ddbeck ddbeck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh wait I'm undercaffeinated. I think that key solves some problems. Here's an actual review.

Comment on lines 10 to 25
baseline: false
baseline: low
baseline_low_date: 2024-09-30
support:
chrome: "81"
chrome_android: "81"
edge: "83"
firefox: "131"
firefox_android: "131"
safari: "16.1"
safari_ios: "16.1"
# document.fragmentDirective cannot be considered part of this feature due to
# https://webkit.org/b/273466. TODO: Consider partial support for this feature
# depending on https://github.com/WICG/scroll-to-text-fragment/issues/257.
# compat_features:
# - api.Document.fragmentDirective
# - api.FragmentDirective
compat_features:
- api.Document.fragmentDirective
- api.FragmentDirective
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the right thing to do here now is:

  • Add html.elements.a.text_fragments to compat_features
  • Use compute_from: html.elements.a.text_fragments to do the override (instead of the manual one here)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh excellent! That makes things much better, thanks for tracking that down.

@jamesnw jamesnw requested a review from ddbeck December 18, 2024 14:25
@ddbeck ddbeck merged commit 92d6be7 into web-platform-dx:main Dec 18, 2024
3 checks passed
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants