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

handleDecorations to check whether table is undefined or null #166

Merged

Conversation

ianmcateer
Copy link
Contributor

@ianmcateer ianmcateer commented May 18, 2022

this is to address the issue raised here: #141

  1. With column resizing enabled, highlight two or more cells.
  2. Without releasing your left mouse button and while your cursor is close enough to a column border to trigger a resize cursor, press the Delete key to delete the cells, alternatively press ctrl + z to undo history if the keymap is enabled

Observe the following error:

Uncaught (in promise) TypeError: Cannot read property 'type' of undefined
    at computeMap (index.es.js:273)
    at Function.get (index.es.js:268)
    at handleDecorations (index.es.js:3074)
    at Plugin.decorations (index.es.js:2811)
    at eval (index.es.js:6526)
    at EditorView.someProp (index.es.js:6806)
    at viewDecorations (index.es.js:6525)
    at EditorView.updateStateInner (index.es.js:6682)
    at EditorView.updateState (index.es.js:6652)
    at Editor.dispatchTransaction (tiptap.esm.js:1176)

This is because pluginState.activeHandle is not being cleared/reset to -1 when its corresponding node is removed. Also because handleDecorations in columnresizing.js is not checking that table is not null/undefined before passing it to TableMap.get(table).

issue.mp4

@ianmcateer
Copy link
Contributor Author

@torifat any chance you could take a look at this please?

@torifat
Copy link
Collaborator

torifat commented May 19, 2022

Hi @ianmcateer, the change LGTM. Is it possible to add a test for the change?

@ianmcateer
Copy link
Contributor Author

Hi @ianmcateer, the change LGTM. Is it possible to add a test for the change?

ty @torifat for getting back- so i've been trying to add a test for the handleDecorations function but it looks like the current test files are in commonJS and are only importing/testing functions/classes from the dist folder, so those methods that are distributed/exported by the package. handleDecorations isnt avilable in /dist so when i try to import the it in the test file i see this

import {handleDecorations} from("../src/columnresizing")
^^^^^^

SyntaxError: Cannot use import statement outside a module

any guidance here? 🙏

@RatoX
Copy link
Collaborator

RatoX commented May 25, 2022

Hey @ianmcateer

I am working to migrate this repository to use TypeScript soon read more. Could you rebase your branch and fix this pr?

Btw, you should be able to import the code using ESM now.

@ianmcateer ianmcateer force-pushed the ianmcateer/fix_handleDecorations_error branch from c0fffea to 2b98bfd Compare June 7, 2022 02:48
@ianmcateer
Copy link
Contributor Author

@RatoX sorry for the late response, i've added a simple test case if you could have a look over? thanks, importing using ESM works fine now

@ocavue ocavue merged commit 23c79ae into ProseMirror:master Jul 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants