Fix addClassesToSVGElement docs to match its actual merge behavior - #2289
Open
afonsojanu wants to merge 1 commit into
Open
Fix addClassesToSVGElement docs to match its actual merge behavior#2289afonsojanu wants to merge 1 commit into
afonsojanu wants to merge 1 commit into
Conversation
Issue svg#2209 reports that the plugin doesn't clear existing classes before adding the configured ones, even though the docs say it should. Looked at the actual code and the plugin's own test fixture (03, "Should avoid adding existing classes") and the additive behavior is what's intended and tested, going back to when this plugin was written. The docs are just wrong about what the plugin does, not the other way around, so this rewrites the intro line and the caution callout to describe the real, merge-based behavior instead of a replace-based one that was never actually implemented.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2209.
The issue reports that classes already on the outer
<svg>element don't get cleared whenaddClassesToSVGElementruns, and points at the docs, which say the plugin "removes and replaces" existing classes. I checked the actual plugin code and its own test suite (fixture 03, "Should avoid adding existing classes") and the merge/append behavior is what's really implemented and has been covered by a test since this plugin was added, not something that broke recently. So this isn't a code regression, it's the docs describing a replace-style behavior that was never there.Rewrote the intro line and the caution callout in the docs to describe what the plugin actually does: existing classes stay, the configured ones get added alongside them, duplicates are avoided through the Set.
No code changes, so no new tests needed. Ran the full suite locally (520 passing, 3 skipped) to confirm nothing else touches this.