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.

Sign up for GitHub

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

Jump to bottom

Change heading error #2868

Closed
vinicius73 opened this issue Sep 6, 2022 · 4 comments · Fixed by #2914
Closed

Change heading error #2868

vinicius73 opened this issue Sep 6, 2022 · 4 comments · Fixed by #2914
Assignees
@vinicius73
Labels
2. developing bug Something isn't working
Milestone
Nextcloud 25

Comments

@vinicius73
Copy link
Member

Related to: #2841

Peek.2022-09-06.15-15.mp4
@vinicius73 vinicius73 added this to the Nextcloud 25 milestone Sep 6, 2022
@vinicius73 vinicius73 self-assigned this Sep 6, 2022
@vinicius73
Copy link
Member Author

@susnux I will revert #2841 until I find a solution for this.
It seems related to the addNodeView and toggleNode in https://github.com/ueberdosis/tiptap/blob/5bcfb322ceb30575430bd8797f2787d65de80f2c/packages/extension-heading/src/heading.ts#L81

vinicius73 pushed a commit that referenced this issue Sep 8, 2022
⏪️ Revert "Merge pull request #2841 from nextcloud/feature/heading-an…
998aa72 
…chors" ultil #2868 be fixed

This reverts commit 0f2c48e, reversing
changes made to 9973d1f.
@vinicius73 vinicius73 mentioned this issue Sep 8, 2022
Merged
vinicius73 pushed a commit that referenced this issue Sep 8, 2022
⏪️ Revert "Merge pull request #2841 from nextcloud/feature/heading-an…
c363e46 
…chors" until #2868 be fixed

This reverts commit 0f2c48e, reversing
changes made to 9973d1f.

Signed-off-by: Vinicius Reis <[email protected]>
@vinicius73 vinicius73 mentioned this issue Sep 8, 2022
Closed
@susnux
Copy link
Contributor

susnux commented Sep 9, 2022

The issue is caused by the VueNodeViewRenderer, I have implemented the node view using plain js and that version is not affected by the issue.

vinicius73 pushed a commit that referenced this issue Sep 9, 2022
⏪️ Revert "Merge pull request #2841 from nextcloud/feature/heading-an…
f59361c 
…chors" until #2868 be fixed

This reverts commit 0f2c48e, reversing
changes made to 9973d1f.

Signed-off-by: Vinicius Reis <[email protected]>
@susnux
Copy link
Contributor

susnux commented Sep 12, 2022

@vinicius73 I think I found a solution, have a look at https://github.com/nextcloud/text/tree/feat/page-anchors

It seems the problem is prosemirror rendering the children into the contentDOM before the tiptap vue renderer updates the nodeview, which will replace the contentDOM with a new one as the wrapper element changes (e.g. from h2 to h1).

I solved it by enforcing tiptap to remount the NodeView if the level changes. I tested it and now it works, also added some more tests for this issue.

@vinicius73
Copy link
Member Author

Works perfectly @susnux
Please go ahead and open a PR pls.

@susnux susnux mentioned this issue Sep 13, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@vinicius73 vinicius73

Labels
2. developing bug Something isn't working
Projects
None yet
Milestone
Nextcloud 25
Development

Successfully merging a pull request may close this issue.

Feat/page anchors nextcloud/text
2 participants
@vinicius73 @susnux