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

feat: Preserve HTML data attributes (from spans to anchors) #42

Merged
merged 2 commits into from Feb 27, 2024

Conversation

pawamoy
Copy link
Member

@pawamoy pawamoy commented Feb 23, 2024

Closes #41

(will squash to add you as co-author @oprypin)

@pawamoy
Copy link
Member Author

pawamoy commented Feb 23, 2024

Ah, adding class to spans will cause issues. That's probably what you meant in your issue comment about still using the attrs HTML parser?

@oprypin
Copy link
Member

oprypin commented Feb 23, 2024

Could you explain to me, I actually don't know what kind of issues this would be. No, I didn't predict this. I only meant if you need to parse the attributes but apparently that's not even needed 🙂

@pawamoy
Copy link
Member Author

pawamoy commented Feb 23, 2024

Well, autorefs sets class=... on anchors, so if someone sets class=... on an autorefs span, anchors will end up with two class attributes. Maybe browsers don't mind though? My initial guess is that it's incorrect and only the last class attribute will be kept, therefore cancelling the autorefs-internal/external classes.

@oprypin
Copy link
Member

oprypin commented Feb 23, 2024

Ah yes for sure multiple class would be super bad

@oprypin
Copy link
Member

oprypin commented Feb 23, 2024

My first instinct is to still double down on regexes and require the class attribute to come first 🤪 (well, second)
But yes the other option is to parse the attributes

@oprypin
Copy link
Member

oprypin commented Feb 23, 2024

The reason that is it's still ok to do this is because all usages should be programmatic and we can still have strict requirements on them

@oprypin
Copy link
Member

oprypin commented Feb 23, 2024

Commit oprypin@10d76d3 works accordingly

@pawamoy
Copy link
Member Author

pawamoy commented Feb 27, 2024

The reason that is it's still ok to do this is because all usages should be programmatic and we can still have strict requirements on them

I've personally written autoref spans manually on a few occurrences 😄
But yeah it's not documented so we can consider it's not public API.

I've pushed your commit on this branch, thanks!

LGTM so will merge once CI passes.

@pawamoy pawamoy merged commit 0c1781d into main Feb 27, 2024
33 checks passed
@pawamoy pawamoy deleted the preserve-data-attributes branch February 27, 2024 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feature: Keep data-attributes of spans
2 participants