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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: Add node emoji to highlight cards #50

Closed
1 of 2 tasks
bdougie opened this issue Jun 6, 2023 · 12 comments 路 Fixed by #63
Closed
1 of 2 tasks

Feature: Add node emoji to highlight cards #50

bdougie opened this issue Jun 6, 2023 · 12 comments 路 Fixed by #63

Comments

@bdougie
Copy link
Member

bdougie commented Jun 6, 2023

Type of feature

馃崟 Feature

Current behavior

Emojis are not being generated in the card preview.

https://insights.opensauced.pizza/feed/171

Screen Shot 2023-06-06 at 10 59 06 AM

Suggested solution

Use node-emoji wrapper for copy. We use it in open-sauced/hot

https://github.com/open-sauced/hot/pulls?q=is%3Apr+emoji+is%3Aclosed

Additional context

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct

Contributing Docs

  • I agree to follow this project's Contribution Docs
@a0m0rajab
Copy link
Contributor

Clicking on the link in the issue gave me this issue:

image

Runtime.UnhandledPromiseRejection: TypeError: fetch failed
    at process.<anonymous> (file:///var/runtime/index.mjs:1189:17)
    at process.emit (node:events:525:35)
    at process.emit (node:domain:489:12)
    at emit (node:internal/process/promises:140:20)
    at processPromiseRejections (node:internal/process/promises:274:27)
    at processTicksAndRejections (node:internal/process/task_queues:97:32)
Runtime.UnhandledPromiseRejection - TypeError: fetch failed
Netlify internal ID: 01H5YKK2BXRAP2QXDVZ283Y20B

Refreshing the page solved the issue.

@a0m0rajab
Copy link
Contributor

This is the result I got when I paste the link on FaceBook
image

@a0m0rajab
Copy link
Contributor

This is the related component in this repo:

${body.length > 108 ? `${body.slice(0, 108)}...` : body}

This is an example of fixing it based on the suggestion in the issue:
https://github.com/open-sauced/hot/pull/449/files

@a0m0rajab
Copy link
Contributor

I worked on this and used node-emoji but it seems not to work, not sure if it's only on my PC, here is the code:
https://github.com/a0m0rajab/opengraph/tree/feat-add-emoji

But one of the things that I got curious about is the HTML/image convertor: the solution might be there:
https://github.com/vercel/satori#dynamically-load-emojis-and-fonts

I will do further testing later.

@a0m0rajab
Copy link
Contributor

Satory/Vercel is using an API/CDN to get the SVG of the emoji:
https://github.com/vercel/satori/blob/2e8dcb486f3dadeb6fc2e8790cb822a72893a21a/playground/pages/index.tsx#L102

We might need to implement something similar:

https://github.com/vercel/satori/blob/2e8dcb486f3dadeb6fc2e8790cb822a72893a21a/playground/utils/twemoji.ts

I will stop here now and check it at another time.

@a0m0rajab a0m0rajab mentioned this issue Jul 22, 2023
19 tasks
@bdougie
Copy link
Member Author

bdougie commented Jul 22, 2023

Clicking on the link in the issue gave me this issue:

image

Runtime.UnhandledPromiseRejection: TypeError: fetch failed
    at process.<anonymous> (file:///var/runtime/index.mjs:1189:17)
    at process.emit (node:events:525:35)
    at process.emit (node:domain:489:12)
    at emit (node:internal/process/promises:140:20)
    at processPromiseRejections (node:internal/process/promises:274:27)
    at processTicksAndRejections (node:internal/process/task_queues:97:32)
Runtime.UnhandledPromiseRejection - TypeError: fetch failed
Netlify internal ID: 01H5YKK2BXRAP2QXDVZ283Y20B

Refreshing the page solved the issue.

This is do to the image generation failing because the data is not ready, I believe. Refreshing hits the cache. We will honestly need to look into using an edge function for all of this in the future.

That work is being explored here. open-sauced/app#1400

I will look at this deeper on Monday. Interesting that nore-emoji did not work in this case.

@a0m0rajab
Copy link
Contributor

Thank you for that! Looking forward to read your review.

@a0m0rajab
Copy link
Contributor

Shower thoughts: This issue could be a great case study for why you need to provide a suggestion to your issue.
The suggestion that you @bdougie provided saved a couple of hours (between 2-3 hours) of trial and error and debugging, since the node-emoji worked on other project it proved to me that the issue is not in the HTML/React part of the code! Otherwise, it would be hard to get out of the rabbit hole.

@bdougie
Copy link
Member Author

bdougie commented Jul 22, 2023

Indeed. I need to get in more issues and make proper suggestions.

Hoping we can start coordinating more triaging with the team.

@a0m0rajab
Copy link
Contributor

I would like to help in case if I can do anything there, when the PR get accepted I am thinking to write a blog about this. Focusing on the value of providing suggested solutions.

@github-actions
Copy link

馃帀 This issue has been resolved in version 2.4.0-beta.1 馃帀

The release is available on:

Your semantic-release bot 馃摝馃殌

@github-actions
Copy link

馃帀 This issue has been resolved in version 2.4.0 馃帀

The release is available on:

Your semantic-release bot 馃摝馃殌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

3 participants