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

Text Align Extension: issue with DOM update when multiple types exist and not every exist at the DOM of the editor. #5097

Merged

Conversation

echatzief
Copy link
Contributor

@echatzief echatzief commented Apr 25, 2024

Please describe your changes

I have changed the setTextAlign and unsetTextAlign because if you used multiple types lets say ['heading', 'paragraph'] and at the DOM of the editor we only have a paragraph only no heading. Then when we call setTextAlign then it will go the the every loop and at the first try to updateAttributes for heading it will fail and it will stop there an the paragraph won't update because that how the every loop works. It stops on the first falsy value. Same happens for the unsetTextAlign function

How did you accomplish your changes

I changed the setTextAlign and unsetTextAlign to use a map to go through every update and then validate the updates. That how we avoid the bug described above

How have you tested your changes

Yes. I have tested locally with a Vue3 project

How can we verify your changes

Try the scenario that we described.

Checklist

  • The changes are not breaking the editor
  • Added tests where possible
  • Followed the guidelines
  • Fixed linting issues

Copy link

netlify bot commented Apr 25, 2024

Deploy Preview for tiptap-embed ready!

Name Link
🔨 Latest commit 5461d43
🔍 Latest deploy log https://app.netlify.com/sites/tiptap-embed/deploys/663123d3302a330008911e82
😎 Deploy Preview https://deploy-preview-5097--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.

@echatzief echatzief changed the title fixed issue with blocking update attribute when we have multiple type… Text Align Extension: issue with DOM update when multiple types exist and not every exist at the DOM of the editor. Apr 25, 2024
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 right to me

@bdbch
Copy link
Contributor

bdbch commented May 2, 2024

Nice change. LGTM!

@echatzief
Copy link
Contributor Author

If it looks good then merge it 😀

@bdbch
Copy link
Contributor

bdbch commented May 10, 2024

@echatzief sometimes we wait for another maintainer to also take a look

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants