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

Bugfix for empty $oldTag #861

Merged
merged 1 commit into from
Nov 16, 2018
Merged

Bugfix for empty $oldTag #861

merged 1 commit into from
Nov 16, 2018

Conversation

kiewietr
Copy link
Contributor

empty oldTag tags get surrounded by newTag on every keystroke. This resulted in huge content. I dont know what the best solution would be but I added a simple contents().length check and only replace it when there is content inside the tag.

This would occur with tags when using font-awesome for example.

empty oldTag tags get surrounded by newTag on every keystroke. This resulted in huge content. I dont know what the best solution would be but I added a simple contents().length check and only replace it when there is content inside the tag.

This would occur with <i> tags when using font-awesome for example.
@Alex-D
Copy link
Owner

Alex-D commented Oct 12, 2018

Hum, what about: #851
What's the best solution?

The original idea is to delete all empty tags, because there are useless.

@kiewietr
Copy link
Contributor Author

The original idea is to delete all empty tags, because there are useless.

Sorry didnt see that one. I think it's an okay solution if you also have the keepEmptyTags option or some option to exclude certain tags from deletion, because we use font-awesome and these icons do use empty i tags.

<i class="fa fa-check"></i>

@Alex-D
Copy link
Owner

Alex-D commented Oct 14, 2018

I understand, there is a list of tags to not delete here: https://github.com/Alex-D/Trumbowyg/blob/v2.11.1/src/trumbowyg.js#L1074

Maybe we should make it an option?

@Alex-D Alex-D merged commit 2167629 into Alex-D:master Nov 16, 2018
@Alex-D
Copy link
Owner

Alex-D commented Nov 16, 2018

Ok, I add this to a fix I do.

Thanks.

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.

2 participants