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

Trix compatibility & allow developpers to setup more replacements on create #66

Merged
merged 8 commits into from
May 20, 2024

Conversation

gael-ian
Copy link
Member

Should fix #65

@jhirn
Copy link

jhirn commented May 17, 2024

Hey @gael-ian.

I was trying to load this branch on my project with import maps but not quite sure how to load the Javascript . This is my first experience with import maps but it's defaults resolving jsdeliver. should I hard code the full path to the gem's install directory (specifically /npm)? I tried to unpin and build from scratch to import via manifest.js but hope you could save me some wrangling.

Any tips?

Comment on lines 61 to 62
// Compatibility with Trix. See #65 on Github.
{ attribute: 'input', delimiters: ['_'] },
Copy link

@jhirn jhirn May 17, 2024

Choose a reason for hiding this comment

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

This doesn't seem limited to <trix-editor> elements.

I know input is a Trix specific html attribute but I'm curious if this could introduce issues with other RTEs or JS plugins which rely on that attribute for another purpose. You'd need a method to opt-out in those cases if this is going to automatically replace input attribute on any html element.

Copy link
Member Author

Choose a reason for hiding this comment

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

You're right, even if chances are small it will match with an input attribute on another tag and find a match with both association name and delimiters in its value and ends doing a wrong replacement.

I changed the Replacement class to support an optional tag argument in its constructor to restrain matches. It should be fine now.

@gael-ian
Copy link
Member Author

@jhirn

Any tips?

Sorry, I don't use importmap myself (I'm not convinced by the idea of having two different tools and pipelines to bundle things that can be as intertwined as JavaScript and CSS) so I can't help you :/

@jhirn
Copy link

jhirn commented May 20, 2024

@jhirn

Any tips?

Sorry, I don't use importmap myself (I'm not convinced by the idea of having two different tools and pipelines to bundle things that can be as intertwined as JavaScript and CSS) so I can't help you :/

I agree with you 100%. First time trying out import maps and it's been kind of a headache.

I can't pull this into my app to test the fix but reading through the code it feels like it would do the trick (and more). Thanks for the feature add!

@gael-ian gael-ian merged commit f7c513c into main May 20, 2024
13 checks passed
@gael-ian gael-ian deleted the 65-trix-compatibility branch May 20, 2024 21:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Cocooned does work properly with ActionText
2 participants