Adds ability to disable dropCursor on Nodes

jamesthomsondev commented

2021-07-02 06:44:25 +02:00

(Migrated from github.com)

This PR adds a disableClass which can be placed on certain Nodes that you might not want the dropCursor to appear on.

Use case: Imagine a custom atom Node that allows files to be dragged into it from the OS. Without this check to disable the dropCursor it will display the cursor above or below this custom Node, incorrectly indicating that the file will be dropped outside of the Node.

This PR adds a `disableClass` which can be placed on certain Nodes that you might not want the `dropCursor` to appear on. Use case: Imagine a custom atom Node that allows files to be dragged into it from the OS. Without this check to disable the dropCursor it will display the cursor above or below this custom Node, incorrectly indicating that the file will be dropped outside of the Node.

marijnh commented

2021-07-02 08:44:27 +02:00

(Migrated from github.com)

I don't think using DOM classes for this is the best approach here. But I'd be okay with having a node spec property (say disableDropCursor) that has this effect.

I don't think using DOM classes for this is the best approach here. But I'd be okay with having a node spec property (say `disableDropCursor`) that has this effect.

jamesthomsondev commented

2021-07-05 03:51:45 +02:00

(Migrated from github.com)

@marijnh Thanks for your reply. I agree, using DOM classes isn't the best and would be more than happy to adjust this PR to your suggested approach. I'm quite new to ProseMirror and therefore not entirely educated on the entirety of its capabilities in terms of customisation. I believe this is the Node spec you're referring to - my assumption then is that adding a disableDropCursor property would not be something a plugin is capable of doing and therefore would require a change to the Node class itself - please correct me if I'm wrong. Any guidance on a way forward would be appreciated. Thanks!

@marijnh Thanks for your reply. I agree, using DOM classes isn't the best and would be more than happy to adjust this PR to your suggested approach. I'm quite new to ProseMirror and therefore not entirely educated on the entirety of its capabilities in terms of customisation. I believe this is the [Node spec](https://prosemirror.net/docs/ref/#model.NodeSpec) you're referring to - my assumption then is that adding a `disableDropCursor` property would not be something a plugin is capable of doing and therefore would require a change to the `Node` class itself - please correct me if I'm wrong. Any guidance on a way forward would be appreciated. Thanks!

marijnh commented

2021-07-05 10:09:22 +02:00

(Migrated from github.com)

The raw object that the user provides as node spec is stored in NodeType.spec, so it is possible for extensions to recognize new node spec properties without changing the model package.

The raw object that the user provides as node spec is stored in [`NodeType.spec`](https://prosemirror.net/docs/ref/#model.NodeType.spec), so it is possible for extensions to recognize new node spec properties without changing the model package.

👍 1

jamesthomsondev commented

2021-07-06 04:15:09 +02:00

(Migrated from github.com)

@marijnh I update the PR to check against a Node spec property instead. Works well. The only thing I found was it would also disable the drop cursor when dragging around existing Nodes within the document - so if you wanted to re-arrange text, it wouldn't display the drop cursor when a Node with disableDropCursor is its sibling (which makes sense). As my use case is catered towards dragging external sources into the editor, I added a check against the editor state to see if it's actually document nodes being dragged. This makes me wonder if the property disableDropCursor is descriptive enough. Something like disableDropCursorWhenSourceExternal would be more accurate, but also quite verbose.

@marijnh I update the PR to check against a Node spec property instead. Works well. The only thing I found was it would also disable the drop cursor when dragging around existing Nodes within the document - so if you wanted to re-arrange text, it wouldn't display the drop cursor when a Node with `disableDropCursor` is its sibling (which makes sense). As my use case is catered towards dragging external sources into the editor, I added a check against the editor state to see if it's actually document nodes being dragged. This makes me wonder if the property `disableDropCursor` is descriptive enough. Something like `disableDropCursorWhenSourceExternal` would be more accurate, but also quite verbose.

marijnh commented

2021-07-06 09:28:12 +02:00

(Migrated from github.com)

I think pos.inside is a more appropriate position to query (if you drag over the end of a leaf node, pos.pos might point after that node)
Would it be reasonable to make this a predicate function rather than a boolean, so that special-cased stuff like view.dragging doesn't have to be hard-coded? That way you could even inspect the dataTransfer to figure out whether files are being dragged to further refine the behavior. (Could pass the view and the node's resolved position to the function.)

- I think `pos.inside` is a more appropriate position to query (if you drag over the end of a leaf node, `pos.pos` might point _after_ that node) - Would it be reasonable to make this a predicate function rather than a boolean, so that special-cased stuff like `view.dragging` doesn't have to be hard-coded? That way you could even inspect the `dataTransfer` to figure out whether files are being dragged to further refine the behavior. (Could pass the view and the node's resolved position to the function.)

jamesthomsondev commented

2021-07-09 08:28:50 +02:00

(Migrated from github.com)

I like the idea of a predicate function for better flexibility so I've added one as an option, but I may have misunderstood you as it sounds more like the predicate should be returned in some manor with the view and resolved node position.

marijnh commented

2021-07-09 10:57:07 +02:00

(Migrated from github.com)

Ah, what I meant was that a node spec could contain a predicate function. Maybe have a disableDropCursor property that can be either a boolean or a function, and will be called with a view and a resolved position (returning a boolean) when it is a function.

Ah, what I meant was that a node spec could contain a predicate function. Maybe have a `disableDropCursor` property that can be either a boolean or a function, and will be called with a view and a resolved position (returning a boolean) when it is a function.

jamesthomsondev commented

2021-07-13 01:40:57 +02:00

(Migrated from github.com)

Got it, that makes a lot more sense now that I have a fresh head. Updated the code to reflect this.

marijnh commented

2021-07-20 16:53:49 +02:00

(Migrated from github.com)

Thanks. I've merged this and followed up with attached patch, which adds documentation and fixes some corner cases (such as when pos is null) and code style. Could you see if the result works for you?

Rows
Columns

Adds ability to disable dropCursor on Nodes #13