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

emoji proof of concept #638

Closed
wants to merge 3 commits into from
Closed

emoji proof of concept #638

wants to merge 3 commits into from

Conversation

lukesmurray
Copy link
Collaborator

@lukesmurray lukesmurray commented Apr 20, 2021

Issue

Create an example to replace emoji names between : and : with the corresponding emoji.

What I did

Created a simple proof of concept using emojis from emoji.json.

Next steps.

  • Integrate with combo box example to create emoji dropdown. (might want to wait til combo box is finished)
  • Simplify emoji.json file or write custom script to parse the official unicode file

The committed emoji.json file would need to be minified and simplified before a merge. This PR is just a RFC.

Before I do any of these I wanted to make sure the changes would be useful! If you want to take a different approach feel free to close this PR!

Alternatives

I thought about using the AutoFormat plugin but I would need to change some of the logic to support emoji matching.
For example you probably don't want to pass a list of a couple thousand markup strings to check for formatting, a dict is faster. Also it feels more intuitive to insert the emoji when the second : is typed rather than after : followed by a space.

Checklist

  • The new code matches the existing patterns and styles.
  • The stories still work (run yarn storybook).
  • The stories are updated when relevant: stories for plugins, knobs for options.

@lukesmurray lukesmurray requested a review from zbeyens as a code owner April 20, 2021 18:33
@lukesmurray lukesmurray marked this pull request as draft April 20, 2021 18:39
@codecov
Copy link

codecov bot commented Apr 20, 2021

Codecov Report

Merging #638 (cbd78e5) into next (a9cf427) will decrease coverage by 26.44%.
The diff coverage is 65.48%.

Impacted file tree graph

@@             Coverage Diff             @@
##             next     #638       +/-   ##
===========================================
- Coverage   90.13%   63.69%   -26.45%     
===========================================
  Files         289      159      -130     
  Lines        2838     1837     -1001     
  Branches      460      493       +33     
===========================================
- Hits         2558     1170     -1388     
- Misses        280      663      +383     
- Partials        0        4        +4     
Impacted Files Coverage Δ
packages/common/src/hoc/createNodeHOC.tsx 0.00% <0.00%> (ø)
packages/common/src/hoc/createNodesHOC.tsx 0.00% <0.00%> (ø)
packages/common/src/hoc/withProps.tsx 0.00% <0.00%> (ø)
packages/common/src/queries/isCollapsed.ts 100.00% <ø> (ø)
packages/common/src/queries/isExpanded.ts 100.00% <ø> (ø)
packages/common/src/queries/isFirstChild.ts 100.00% <ø> (ø)
...kages/common/src/transforms/defaultsDeepToNodes.ts 60.00% <ø> (ø)
packages/common/src/transforms/mergeDeepToNodes.ts 100.00% <ø> (ø)
packages/common/src/utils/escapeRegexp.ts 100.00% <ø> (ø)
packages/common/src/utils/getHandler.ts 100.00% <ø> (ø)
... and 520 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a66ca4c...cbd78e5. Read the comment docs.

@dylans
Copy link
Collaborator

dylans commented Apr 20, 2021

I've been thinking of doing something similar for a while, so I'm really excited to see this!

@zbeyens
Copy link
Member

zbeyens commented Apr 20, 2021

Great! Your approach looks good to me, I would also use the combobox for that. We can discuss later on which emoji picker to use as there is many in the field, e.g. https://github.com/missive/emoji-mart. It would be perfect to decouple the picker from the plugin (slate logic).

@lukesmurray
Copy link
Collaborator Author

lukesmurray commented Apr 20, 2021

Thanks for the pointer on emoji-mart. I added the combobox and switched to using emoji-mart as a headless backend for search in the combobox, and I'm now using the emoji-mart index to resolve :colon-strings: to emojis.

The current approach is text between colons is replaced with an emoji, or you can use the combobox which inserts an emoji directly. TBD is skin variant preferences. Emoji-mart uses a global skin variant preference which makes sense from a usability standpoint since the colon syntax is a bit confusing and verbose :man::skin-tone-2: vs :man:.

New Todos

  • Extract from stories into a plugin. I totally get what you mean about decoupling the picker. It looks like emoji mart wants to have control over text input so not sure if it could be integrated fluidly.
  • Currently emoji-mart is installed in the workspace root, that needs to be changed before this PR is merged. Obvious choice is to move the emoji logic to a package and install it there
  • Add skin variant preferences or options to the UI

Here's a gif 🎉

slate-plugins-emojis

@zbeyens
Copy link
Member

zbeyens commented Apr 21, 2021

Great!

  • For the new package, you can use this template.
  • we don't want to lose the editor focus when picking so we indeed need to use a controlled picker (combobox for now)
  • emoji-mart picker can be displayed on a toolbar button click.

Let's move the picker design discussion to #639

@zbeyens
Copy link
Member

zbeyens commented May 9, 2021

Closed as we're deleting the next branch. Please open this against main branch.

@zbeyens zbeyens closed this May 9, 2021
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.

3 participants