-
-
Notifications
You must be signed in to change notification settings - Fork 379
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
Feat. Add Emoji Picker Option #854
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@hunxjunedo is attempting to deploy a commit to the TypeCell Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great stuff @hunxjunedo! Nice that you got the keyboard navigation to work. I left a number of comments which I think will be helpful to make the code clean and a bit more generic.
packages/react/src/components/SuggestionMenu/SuggestionMenuController.tsx
Outdated
Show resolved
Hide resolved
packages/react/src/components/SuggestionMenu/hooks/useSuggestionMenuKeyboardNavigation.ts
Outdated
Show resolved
Hide resolved
@flipvh wdyt of this? You can see a preview here: https://blocknote-git-fork-hunxjunedo-main-typecell.vercel.app/basic/minimal The UX is very much based on Notion. There are some things to be improved (i.e.: dark / light mode), but I think the basics look great. wdyt? |
Acknowledged the comments, updating on that soon 👍 |
I like it too! Maybe in a second round, the use of emoji option in the command menu can be made more intuitive. Now it just renders a |
@flipvh if I don't get you wrong, do you mean that the emoji option in the slash menu doesn't open the Emoji Menu ? Because I've tested it and can be seen in the demo that it works fine, is that a browser issue ? |
@YousefED it's been almost a week, what's the progress of this PR ? Are we moving towards merging it ? Is this a low-priority ? |
I'll look into this PR tomorrow so we can try to finish it up and merge👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Overall works & looks great - in addition to the review comments, there are a couple of broader changes that I think we still need:
- Option to disable emoji menu in
BlockNoteDefaultUI
, like with the slash menu. - Apply the fix to open the emoji menu programmatically, see here, and remove code for the simulated input approach.
- General refactor to decouple the emoji menu from the suggestion menu.
- Move the slash menu item to
defaultSlashMenuItems
/reactDefaultSlashMenuItems
. - Like @YousefED suggested, we should separate the
useSuggestionMenuKeyboardNavigation
into 2 hooks, the second beinguseGridSuggestionMenuKeyboardNavigation
. Likewise theSuggestionMenuController
andSuggestionMenuWrapper
should take agrid: boolean
prop to swap between them. - We should also rename the
EmojiMenu
component to a more generalGridSuggestionMenu
, that way we can hook it up to thecomponentsContext
later (don't worry about the components context thing for now, we'll sort it out later).
- Move the slash menu item to
packages/react/src/components/SuggestionMenu/SuggestionMenuController.tsx
Outdated
Show resolved
Hide resolved
So I implemented the fix to opening the menu and started with the refactor, this should give you a good starting point to work off for the remaining changes👍 |
@matthewlipski thanks for taking charge and the valuable reviews. It's night here in my time zone, I'll do what's necessary tomorrow morning. |
@matthewlipski I've addressed the requested changes, can you please finalize. |
/claim #808
fixes #808
this pull request adds the emoji picker option which is accessible by both the trigger character (:) and the slash menu.
Recordings
Screencast.from.19-06-2024.20.11.04.webm
2024-06-19.20-21-45.mp4
Approach
I started with a custom suggestion menu as suggested, with custom component and
getItems
function. The function fetches all the emojis from the Emojimart at initial render, and then returns the emojis asitems
on every search. This approach may sound unfamiliar as using emoji-mart as the picker would have been preferable, however the main issue with almost all emoji-pickers is that either they don't allow filtering out the emojis shown, or even if indirectly allowed (like emoji-mart allows to hide some emoijs ), they don't update dynamically to reflect those changes. Therefore I was left with an option of creating a custom picker, which worked pretty fine. I then proceeded to add the functionality in theuseSuggestionMenuKeyboardNavigation.ts
file to handle all sorts of navigation forward and backward arrows if it's for theemojiMenu
.Secondly, the task was to make this menu accessible by the slash-menu, therefore I added a custom option to slash menu for emojis, which was similar to other options except that this option doesn't insert anything into the editor, instead it just simulates a
keydown
when clicked, which is listened by theonKeyDown
function specified in the pluginprops
in thesuggestionPlugin.ts
file, which then carries the logic and opens theemojiMenu
Tests
showing document Json
and the one withmentions menu