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

fix: #5851 - While setting content directly while using CharacterCount with limit is not obeyed #5862

Merged
merged 1 commit into from
Nov 25, 2024

Conversation

gethari
Copy link
Contributor

@gethari gethari commented Nov 22, 2024

Fixes #5851

2024-11-22.at.16.17.40.-.Tan.Cephalopod.mp4

Copy link

netlify bot commented Nov 22, 2024

Deploy Preview for tiptap-embed ready!

Name Link
🔨 Latest commit bbc4743
🔍 Latest deploy log https://app.netlify.com/sites/tiptap-embed/deploys/674197990f35f00008a8fa1d
😎 Deploy Preview https://deploy-preview-5862--tiptap-embed.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@gethari gethari changed the base branch from develop to main November 22, 2024 11:03
@gethari gethari changed the base branch from main to develop November 23, 2024 08:47
@gethari
Copy link
Contributor Author

gethari commented Nov 25, 2024

@nperez0111 can you please let me know your thoughts / review this please ?

Copy link
Contributor

@nperez0111 nperez0111 left a comment

Choose a reason for hiding this comment

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

This looks good! Thanks for the contribution @gethari

@nperez0111 nperez0111 merged commit 14681a1 into ueberdosis:develop Nov 25, 2024
14 checks passed
@nperez0111 nperez0111 mentioned this pull request Nov 26, 2024
5 tasks
@gethari
Copy link
Contributor Author

gethari commented Nov 29, 2024

Hey @nperez0111 , I think I might have caused a side effect because of this, when the charLimit is set, whenever we try to call setContent , its not getting reflected 😢

@nperez0111
Copy link
Contributor

That doesn't sound right to me, can you reproduce in a sandbox & prove that it this.

Most likely, you are running into something a lot of people do, trying to put html in setContent and thinking it will work exactly as you input it. As a test, could you try doing insertContent which takes a different codepath than setContent

@gethari
Copy link
Contributor Author

gethari commented Dec 2, 2024

@nperez0111 please find the Sanbox,

The size of the content inside setContent is 321, so if we set the limit >= the size, this works, but when we use limit of 300, the content is not getting overridden, I guess previously it was just setting the content all-together without obeying the limit, after my appendTransaction, i think this is getting skipped

@nperez0111
Copy link
Contributor

Yea, not sure what is going on here & I don't have time to fix it. Could you open a new issue? What is strange to me is that it feels like appendTransaction should always run so I'm not sure that we need to run the modification in the filter transaction anymore, & just let it be in the appendTransaction instead

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.

[Bug]: While setting content directly while using CharacterCount with limit is not obeyed
2 participants