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

fix: Emoji suggestion slash menu does not display #984

Merged
merged 5 commits into from
Sep 6, 2024

Conversation

jkcs
Copy link
Contributor

@jkcs jkcs commented Aug 2, 2024

close #975

Currently unable to use ":" to open emoji, because it will cause ":" not to be cleared. Can we add a new pluginState to mark it?

file: packages/core/src/extensions/SuggestionMenu/SuggestionPlugin.ts

clearQuery = () => {
    if (this.pluginState === undefined) {
      return;
    }

    this.editor._tiptapEditor
      .chain()
      .focus()
      .deleteRange({
        from:
          this.pluginState.queryStartPos! -
          (this.pluginState.fromUserInput
            ? this.pluginState.triggerCharacter!.length
            : 0),
        to: this.editor._tiptapEditor.state.selection.from,
      })
      .run();
  };

Copy link

vercel bot commented Aug 2, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
blocknote ✅ Ready (Inspect) Visit Preview Sep 6, 2024 11:42am
blocknote-website ✅ Ready (Inspect) Visit Preview Sep 6, 2024 11:42am

Copy link

vercel bot commented Aug 2, 2024

@jkcs is attempting to deploy a commit to the TypeCell Team on Vercel.

A member of the Team first needs to authorize it.

@matthewlipski
Copy link
Collaborator

Hi, thanks for this PR! It looks good, but I'm seeing a bug where if you open the emoji picker (with the : key or slash menu), start typing, and then keep hitting backspace, the whole picker briefly flashes after clearing the last character of the query.

@jkcs
Copy link
Contributor Author

jkcs commented Aug 29, 2024

Thank you for your testing and review. I have fixed this bug, pls take another look.

Can we extend a state in the SuggestionPlugin, such as focusInputTriggerCharacter, to achieve a UX similar to Notion, or is it sufficient as it is?

@jkcs jkcs changed the title fix #975 fix: Emoji suggestion slash menu does not display Aug 29, 2024
@matthewlipski
Copy link
Collaborator

The difference in UX is that Notion adds the : character after selection the Emoji slash menu item whereas BlockNote doesn't right?

Hmm, I think in that case we want to extend editor.openSelectionMenu with an optional argument to toggle fromUserInput & insert the trigger character, smth like this:

editor.openSelectionMenu(":", false) -> opens menu same as before, fromUserInput is false
editor.openSelectionMenu(":", true) -> inserts trigger character before opening the menu, fromUserInput is true

That should mean we don't have to change the plugin state. Do you wanna give that a shot?

Copy link
Collaborator

@YousefED YousefED left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing this!

Should we give some more thought about naming conventions? When reviewing, to me payload seems quite generic, and also fromUserInput is not very clear what it does. I know fromUserInput was already there, but maybe we should rename it to make more clear what it does?

cc @matthewlipski

@jkcs
Copy link
Contributor Author

jkcs commented Sep 4, 2024

What do you think of deleteTriggerCharacter and extraData?

@YousefED
Copy link
Collaborator

YousefED commented Sep 4, 2024

What do you think of deleteTriggerCharacter and extraData?

deleteTriggerCharacter first sounds great if that's exactly what it does, way more descriptive.

extraData is just as generic as payload. I guess it's not just the naming in this case, but more the question whether we need a generic "bag" of properties like this, or can we write it more strongly typed?

@matthewlipski
Copy link
Collaborator

Tbh I think we can do with just deleteTriggerCharacter and ignoreMinQueryLength. While only the grid suggestion currently supports minQueryLength and this implies that the regular suggestion menu also uses it, I think minQueryLength is really rarely used anyway outside the default emoji picker so I don't think this is a big concern. Imo if we just change fromUserInput to deleteTriggerCharacter and payload.ignoreQueryLength to ignoreQueryLength, we'll be ready to merge👍

# Conflicts:
#	packages/core/src/editor/BlockNoteEditor.ts
#	packages/core/src/extensions/SuggestionMenu/getDefaultSlashMenuItems.ts
@jkcs
Copy link
Contributor Author

jkcs commented Sep 5, 2024

ready to merge

@YousefED
Copy link
Collaborator

YousefED commented Sep 5, 2024

Great! Btw @jkcs , are you on discord?

@jkcs
Copy link
Contributor Author

jkcs commented Sep 6, 2024

Great! Btw @jkcs , are you on discord?

of course, the name is the same as the github username

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.

Emoji Slash - lack of identification and bug
3 participants