HAR-10208 - Trackable id by falsaadehh · Pull Request #788 · superdoc-dev/superdoc

falsaadehh · 2025-08-19T13:16:02Z

No description provided.

…o trackable_ids

VladaHarbour · 2025-08-19T14:40:51Z

@falsaadehh @harbournick looks good to me!
The only thing I would like to mention is that I noticed that all block nodes of the same type have the same id. I'm not aware of the ticket details but just wondering whether this is desired behavior

Copilot

Pull Request Overview

This PR adds trackable block IDs to various node types in the super-editor by implementing a sdBlockId attribute system. The purpose is to enable tracking and manipulation of block-level nodes through unique identifiers.

Key changes:

Adds sdBlockId attribute to multiple node types (Table, DocumentSection, ShapeTextbox, etc.)
Creates a new BlockNode extension with commands and helpers for block manipulation
Implements a generateBlockUniqueId utility function for creating unique identifiers

Reviewed Changes

Copilot reviewed 13 out of 14 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
`packages/super-editor/src/extensions/table/table.js`	Adds `sdBlockId` attribute with HTML parsing/rendering
`packages/super-editor/src/extensions/structured-content/document-section.js`	Adds `sdBlockId` attribute with HTML parsing/rendering
`packages/super-editor/src/extensions/shape-textbox/shape-textbox.js`	Adds `sdBlockId` attribute with HTML parsing/rendering
`packages/super-editor/src/extensions/shape-container/shape-container.js`	Adds `sdBlockId` attribute with HTML parsing/rendering
`packages/super-editor/src/extensions/paragraph/paragraph.js`	Adds `sdBlockId` attribute with HTML parsing/rendering
`packages/super-editor/src/extensions/ordered-list/ordered-list.js`	Adds `sdBlockId` attribute with HTML parsing/rendering
`packages/super-editor/src/extensions/heading/heading.js`	Adds `sdBlockId` attribute with different implementation pattern
`packages/super-editor/src/extensions/bullet-list/bullet-list.js`	Adds `sdBlockId` attribute with HTML parsing/rendering
`packages/super-editor/src/extensions/block-node/block-node.js`	New extension providing commands and helpers for block manipulation
`packages/super-editor/src/core/utilities/sdBlockUniqueId.js`	Utility function for generating unique block IDs

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

falsaadehh · 2025-08-20T10:33:58Z

@VladaHarbour can you give this another look now?
@harbournick I tried to implement this without a plugin but it wasn't a good solution, if default return the id then it's executed once per node type, hence multiple nodes of same type will have the same id even if i have keepOnSplit: false, also it wasn't an elegant solution overall. if i'm using it then i need to use parseHTML instead of parseDOM to make sure the id is in the node not only in the dom which also is not a good practice here
based on this conversation the best way to do it via appendTransaction
https://discuss.prosemirror.net/t/how-i-can-attach-attribute-with-dynamic-value-when-new-paragraph-is-inserted/751/3
let me know what you think

VladaHarbour

Hi @falsaadehh! I think that onCreate hook of BlockNode extension is extra and we can proceed with appendTransaction which is executed right after onCreate every time we face block node. We can reduce the number of calculations. @harbournick can you please confirm my point
But what is more, I found one lists related issue. Every paragraph added after newly inserted list has the same id which is weird. Also @harbournick do you think every list item(which is a separate list) should have the same or different id? For now it's the same if we create new list but different if we upload a document

Screen.Recording.2025-08-20.at.15.51.25.mov

falsaadehh · 2025-08-20T13:15:57Z

Hi @falsaadehh! I think that onCreate hook of BlockNode extension is extra and we can proceed with appendTransaction which is executed right after onCreate every time we face block node. We can reduce the number of calculations. @harbournick can you please confirm my point But what is more, I found one lists related issue. Every paragraph added after newly inserted list has the same id which is weird. Also @harbournick do you think every list item(which is a separate list) should have the same or different id? For now it's the same if we create new list but different if we upload a document

Screen.Recording.2025-08-20.at.15.51.25.mov

if onCreate is removed then uploaded documents won't have unique ids wrapped around their nodes. for lists having the same unique id I think it's better if i move unique id to list item or not add unique ids to lists at all since it has many attributes that identify it and I think that will fix the issue for the following paragraph having same id as will @harbournick should we remove lists from our block node types?

harbournick

Great call using appendTransaction here! Please see my comments and #793

harbournick · 2025-08-21T04:21:25Z

One more comment: let's please add some unit tests for each of the helpers and commands

…hen first loaded (#793) * chore: update content block * fix: create utility fns for block node, add tests, add hasInitialized for first pass

falsaadehh · 2025-08-21T11:55:46Z

done @harbournick @VladaHarbour considered all the review comments here

harbournick

Just two super minor fixes but otherwise looks great!

harbournick · 2025-08-21T16:12:29Z

+
+const { findChildren } = helpers;
+const SD_BLOCK_ID_ATTRIBUTE_NAME = 'sdBlockId';
+export const BlockNodePluginKey = new PluginKey('blockNdePlugin');


Let's fix typo in blockNdePlugin

harbournick · 2025-08-21T16:13:13Z

+const SD_BLOCK_ID_ATTRIBUTE_NAME = 'sdBlockId';
+export const BlockNodePluginKey = new PluginKey('blockNdePlugin');
+export const BlockNode = Extension.create({
+  name: 'BlockNode',


let's go with blockNode for consistency (lower case b) in the actual name key

falsaadehh · 2025-08-22T10:36:10Z

Just two super minor fixes but otherwise looks great!

done

harbournick

LGTM

chittolinag · 2025-08-22T15:08:45Z

I see the unit tests are failing. Is this expected @harbournick @falsaadehh ?

harbournick · 2025-08-22T17:41:26Z

I see the unit tests are failing. Is this expected @harbournick @falsaadehh ?

@chittolinag great catch! you're right - by changing the name of the block we introduced a namespace issue. And that's why we have unit tests!
pushed a commit to fix

falsaadehh added 2 commits August 19, 2025 14:15

feat: adding node block unique ids

256759a

Merge branch 'develop' of github.com:Harbour-Enterprises/SuperDoc int…

5e6ca71

…o trackable_ids

falsaadehh marked this pull request as draft August 19, 2025 13:16

falsaadehh requested review from VladaHarbour, artem-harbour and harbournick and removed request for VladaHarbour, artem-harbour and harbournick August 19, 2025 13:16

feat: add trackable ids block commands

6ab167f

falsaadehh requested review from VladaHarbour and harbournick August 19, 2025 13:51

falsaadehh marked this pull request as ready for review August 19, 2025 13:51

falsaadehh changed the title ~~WIP - Trackable id - HAR-10208~~ Trackable id - HAR-10208 Aug 19, 2025

falsaadehh changed the title ~~Trackable id - HAR-10208~~ HAR-10208 - Trackable id Aug 19, 2025

VladaHarbour reviewed Aug 19, 2025

View reviewed changes

Comment thread packages/super-editor/src/extensions/heading/heading.js

harbournick requested review from VladaHarbour and Copilot August 19, 2025 14:45

Copilot AI reviewed Aug 19, 2025

View reviewed changes

Comment thread packages/super-editor/src/extensions/heading/heading.js Outdated

Comment thread packages/super-editor/src/extensions/heading/heading.js Outdated

Comment thread packages/super-editor/src/core/utilities/sdBlockUniqueId.js Outdated

falsaadehh added 3 commits August 20, 2025 11:21

fix: same id for nodes of same type

c9695f3

fix: same id for nodes of same type

8e44f6f

fix: same id for nodes of same type

7d82c55

fix: same id for nodes of same type

a4b06bf

VladaHarbour requested changes Aug 20, 2025

View reviewed changes

harbournick assigned falsaadehh Aug 21, 2025

harbournick added the ready for review label Aug 21, 2025

harbournick requested changes Aug 21, 2025

View reviewed changes

harbournick and others added 3 commits August 21, 2025 11:10

Feature: Block node IDs utilities, add hasInitialized to run plugin w…

467ff8c

…hen first loaded (#793) * chore: update content block * fix: create utility fns for block node, add tests, add hasInitialized for first pass

fix: consider code review comments

9019fa1

test: adding block-node commands and helpers tests

ce22284

harbournick requested changes Aug 21, 2025

View reviewed changes

harbournick added the changes requested label Aug 21, 2025

fix: change plugin name

5c7780b

harbournick approved these changes Aug 22, 2025

View reviewed changes

chittolinag and others added 3 commits August 22, 2025 12:50

Merge branch 'develop' into trackable_ids

9b5c7bc

Merge branch 'develop' into trackable_ids

58c56ea

chore: fix helpers blockNode capitalization namespace issue

ef0ac43

harbournick merged commit 7a696c6 into develop Aug 22, 2025
5 of 7 checks passed

harbournick deleted the trackable_ids branch August 22, 2025 17:42

harbournick removed the changes requested label Aug 22, 2025

chittolinag restored the trackable_ids branch August 22, 2025 18:41

chittolinag deleted the trackable_ids branch August 22, 2025 18:55

Conversation

falsaadehh commented Aug 19, 2025

Uh oh!

VladaHarbour commented Aug 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

falsaadehh commented Aug 20, 2025

Uh oh!

VladaHarbour left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

falsaadehh commented Aug 20, 2025

Uh oh!

harbournick left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

harbournick commented Aug 21, 2025

Uh oh!

falsaadehh commented Aug 21, 2025

Uh oh!

harbournick left a comment

Choose a reason for hiding this comment

Uh oh!

harbournick Aug 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

harbournick Aug 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

falsaadehh commented Aug 22, 2025

Uh oh!

harbournick left a comment

Choose a reason for hiding this comment

Uh oh!

chittolinag commented Aug 22, 2025

Uh oh!

harbournick commented Aug 22, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

VladaHarbour commented Aug 19, 2025 •

edited

Loading

VladaHarbour left a comment •

edited

Loading

harbournick Aug 21, 2025 •

edited

Loading

harbournick Aug 21, 2025 •

edited

Loading