[go: up one dir, main page]
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.

Sign up for GitHub

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

Jump to bottom

Remove textLength as a global SVG attribute #23135

Merged
merged 1 commit into from
Jun 17, 2024
Merged

Remove textLength as a global SVG attribute #23135

merged 1 commit into from
Jun 17, 2024

Conversation

foolip
Copy link
Collaborator
@foolip foolip commented May 20, 2024 •
edited
Loading

It can only be used on SVG <text>, <tspan>, and <textPath>, and
previously <tref> which has been removed now.

@github-actions github-actions bot added the data:svg 🖌️ Compat data for SVG features. https://developer.mozilla.org/docs/Web/SVG label May 20, 2024
@foolip foolip requested a review from queengooborg May 20, 2024 09:25
@Elchi3 Elchi3 added the needs content update 📝 This PR needs a corresponding update to mdn/content to update the documentation label May 27, 2024
Elchi3
Elchi3 reviewed May 27, 2024
View reviewed changes
Copy link
Member
@Elchi3 Elchi3 left a comment

Choose a reason for hiding this comment

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

This would break the MDN page at https://developer.mozilla.org/docs/Web/SVG/Attribute/textLength

We usually try to have a content PR before merging. I'm assuming this data is available in BCD elsewhere?

queengooborg
queengooborg requested changes Jun 5, 2024
View reviewed changes
Copy link
Collaborator
@queengooborg queengooborg left a comment

Choose a reason for hiding this comment

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

This attribute is added under the <text> element in BCD, but we need to add it to the other three elements as well.

foolip added a commit to foolip/content that referenced this pull request Jun 5, 2024
@foolip
Use svg.elements.text.textLength for textLength compat data
d1f2cc8 
This unblocks mdn/browser-compat-data#23135.
@foolip foolip mentioned this pull request Jun 5, 2024
Merged
@foolip
Copy link
Collaborator Author
foolip commented Jun 5, 2024

I've sent mdn/content#33939 to update MDN.

Rumyra pushed a commit to mdn/content that referenced this pull request Jun 5, 2024
@foolip
Use svg.elements.text.textLength for textLength compat data (#33939)
827631e 
This unblocks mdn/browser-compat-data#23135.
@foolip
Remove textLength as a global SVG attribute
c70925c 
It can only be used on SVG <text>, <tspan>, and <textPath>, and
previously <tref> which has been removed now.
@foolip
Copy link
Collaborator Author
foolip commented Jun 5, 2024

And now I've updated this PR to also add the attribute to <textPath>. I don't think we should add it to <tref> because that's already removed from Safari and just waiting for enough time to pass before we remove the whole entry.

@Elchi3 Elchi3 removed the needs content update 📝 This PR needs a corresponding update to mdn/content to update the documentation label Jun 17, 2024
Elchi3
Elchi3 approved these changes Jun 17, 2024
View reviewed changes
Copy link
Member
@Elchi3 Elchi3 left a comment

Choose a reason for hiding this comment

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

Thanks! 👍

@Elchi3 Elchi3 merged commit 35faa27 into mdn:main Jun 17, 2024
6 checks passed
@WontonSam WontonSam mentioned this pull request Jul 19, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@queengooborg queengooborg queengooborg requested changes

@Elchi3 Elchi3 Elchi3 approved these changes

Assignees
No one assigned
Labels
data:svg 🖌️ Compat data for SVG features. https://developer.mozilla.org/docs/Web/SVG
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@foolip @Elchi3 @queengooborg