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

isTextSelection always returns false #2979

Closed
2 tasks done
eduludi opened this issue Jul 12, 2022 · 5 comments · Fixed by #3089
Closed
2 tasks done

isTextSelection always returns false #2979

eduludi opened this issue Jul 12, 2022 · 5 comments · Fixed by #3089
Labels
Type: Bug The issue or pullrequest is related to a bug

Comments

@eduludi
Copy link

eduludi commented Jul 12, 2022

What’s the bug you are facing?

The method isTextSelection always return false even when object is a TextSelection

Which browser was this experienced in? Are any special extensions installed?

Chrome, Safari

How can we reproduce the bug on our side?

  1. Import isTextSelection from @tiptap/core

    import { isTextSelection } from '@tiptap/core'
  2. Provide editor.state.selection to it

    const isTextSelected = isTextSelection(editor.state.selection)
  3. Select a text and isTextSelected will be false

Can you provide a CodeSandbox?

https://codesandbox.io/s/determined-gauss-7tnuji?file=/src/App.js

What did you expect to happen?

isTextSelected should return true when text is selected

Anything to add? (optional)

I'm using @tiptap/react v2.0.0-beta.114

Thanks for this awesome library!

Did you update your dependencies?

  • Yes, I’ve updated my dependencies to use the latest version of all packages.

Are you sponsoring us?

  • Yes, I’m a sponsor. 💖
@eduludi eduludi added the Type: Bug The issue or pullrequest is related to a bug label Jul 12, 2022
@eduludi
Copy link
Author

eduludi commented Jul 12, 2022

A workaround is to do what isTextSelected does internally: checking if the selection is a instance of TextSelection (from prosemirror-state)

import { TextSelection } from 'prosemirror-state'

// ...

const hasSelection = editor && !editor.state.selection.empty;
const isTextSelected =  hasSelection &&  editor.state.selection instanceof TextSelection

console.log(isTextSelected) // Will return true when text is selected

So, I'm not sure why the version of isTextSelection imported from @tiptap/core doesn't work...

@jorroll
Copy link

jorroll commented Jul 13, 2022

Just ran into this issue myself. isNodeSelected() also doesn't work. The problem seems to be that tiptap defines isTextSelection with:

function isTextSelection(value) {
    return isObject(value) && value instanceof TextSelection;
}

function isNodeSelection(value) {
    return isObject(value) && value instanceof NodeSelection;
}

function isObject(value) {
    return (value
        && typeof value === 'object'
        && !Array.isArray(value)
        && !isClass(value));
}

function isClass(value) {
    var _a;
    if (((_a = value.constructor) === null || _a === void 0 ? void 0 : _a.toString().substring(0, 5)) !== 'class') {
        return false;
    }
    return true;
}

It's not clear to me why tiptap's version of isTextSelection and isNodeSelection includes the isObject() check since it is unnecessary given the following value instanceof TextSelection check. More problematically though, isObject() includes the !isClass() call (Not sure why that is? I guess it's checking for just a POJO) and since TextSelection is a class instance isClass() is returning true resulting in isObject() returning false.
Solution:

function isTextSelection(value) {
    return value instanceof TextSelection;
}

function isNodeSelection(value) {
    return value instanceof NodeSelection;
}

@kivikakk
Copy link
Contributor

kivikakk commented Aug 16, 2022

It looks like ProseMirror's migration to TypeScript (which happened in prosemirror-state 1.3.4->1.4.0) made this break since they weren't classes proper before then.

That update made its way into Tiptap in #2854, which was first released in @tiptap/core@2.0.0-beta.180, so likely broken since then.

kivikakk added a commit to radiopaedia/tiptap that referenced this issue Aug 16, 2022
Fixes ueberdosis#2979.  Since the ProseMirror TypeScript upgrade, these have
always returned false, since the Selection type tree are all classes
now.
@kivikakk
Copy link
Contributor

PR opened at #3089.

@bdbch
Copy link
Contributor

bdbch commented Aug 22, 2022

Great one @kivikakk - looking forward to merge your PR.

bdbch pushed a commit that referenced this issue Aug 22, 2022
Fixes #2979.  Since the ProseMirror TypeScript upgrade, these have
always returned false, since the Selection type tree are all classes
now.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug The issue or pullrequest is related to a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants