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 for data attributes not rendering after pr #4082 #4980

Merged
merged 1 commit into from
Apr 5, 2024

Conversation

moham96
Copy link
Contributor

@moham96 moham96 commented Mar 15, 2024

Please describe your changes

after pr #4082 there is a regression where data attributes don't trickle down to the last render function, this fixes it

How did you accomplish your changes

I passed the html attributes to the last renderHtml by merging

How have you tested your changes

I ran the demo and verified the rendered attributes

How can we verify your changes

run the demo and inspect the mention elements, you should see data attributes render correctly

Remarks

Not sure if this is the right way to accomplish this since it is mutating the options object, the other fix would be to change the type of renderHTML to renderHTML: (props: { options: MentionOptions; node: ProseMirrorNode, HTMLAttributes?:Record<string, any>}) => DOMOutputSpec and pass the merged attributes

Checklist

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

Related issues

#4082

Copy link

netlify bot commented Mar 15, 2024

Deploy Preview for tiptap-embed ready!

Name Link
🔨 Latest commit 1ab920f
🔍 Latest deploy log https://app.netlify.com/sites/tiptap-embed/deploys/65f42bd1117fcd000804bbf7
😎 Deploy Preview https://deploy-preview-4980--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.

@moham96
Copy link
Contributor Author

moham96 commented Mar 15, 2024

The test failing has nothing to do with the change, it is cause by not checking for null in the images example.

@Joroze
Copy link

Joroze commented Mar 30, 2024

@bdbch can this PR get some prioritization for the Mention extension defect?

Personally not urgent but would love to update to the latest TipTap once its fixed!

Thank you

#4845

svenadlung
svenadlung previously approved these changes Apr 5, 2024
Copy link
Contributor

@svenadlung svenadlung left a comment

Choose a reason for hiding this comment

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

LGTM. @bdbch what do you think?

bdbch
bdbch previously approved these changes Apr 5, 2024
@bdbch bdbch changed the base branch from develop to main April 5, 2024 21:54
@bdbch bdbch dismissed stale reviews from svenadlung and themself April 5, 2024 21:54

The base branch was changed.

@bdbch bdbch changed the base branch from main to develop April 5, 2024 21:54
@bdbch bdbch merged commit 2390cf2 into ueberdosis:develop Apr 5, 2024
13 of 14 checks passed
@nperez0111 nperez0111 mentioned this pull request Jul 13, 2024
5 tasks
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.

4 participants