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

feat: allow :emoji: in repository list descriptions #449

Merged
merged 2 commits into from
Apr 11, 2023

Conversation

JoshuaKGoldberg
Copy link
Contributor

@JoshuaKGoldberg JoshuaKGoldberg commented Feb 22, 2023

What type of PR is this? (check all applicable)

  • πŸ• Feature
  • πŸ› Bug Fix
  • πŸ“ Documentation Update
  • 🎨 Style
  • πŸ§‘β€πŸ’» Code Refactor
  • πŸ”₯ Performance Improvements
  • βœ… Test
  • πŸ€– Build
  • πŸ” CI
  • πŸ“¦ Chore (Release)
  • ⏩ Revert

Description

Uses node-emoji to "emojify" repository descriptions. Strings like "a :fire: :pizza: !" get emojis converted to outputs like " a πŸ”₯ πŸ• !".

Related Tickets & Documents

Fixes #370

Mobile & Desktop Screenshots/Recordings

The hero search shows two examples when searching for ossu:

Screenshot of hero search with two repos for ossue, both with starting colon emoji names replaced by actual emojis

For older repos below, I added olderData[0].description = ':tada: ' + olderData[0].description in RecentRepoListWrap.tsx's useEffect:

Screenshot of 'Older' repository list showing the πŸŽ‰ emoji at the start of a first entry's description

Added tests?

  • πŸ‘ yes
  • πŸ™… no, because they aren't needed (there are no .tsx file tests yet)
  • πŸ™‹ no, because I need help

Added to documentation?

  • πŸ“œ README.md
  • πŸ““ docs.opensauced.pizza
  • πŸ• dev.to/opensauced
  • πŸ“• storybook
  • πŸ™… no documentation needed

[optional] Are there any post-deployment tasks we need to perform?

[optional] What gif best describes this PR or how it makes you feel?

Donkey from Shrek smiling at the camera charmingly

@netlify
Copy link

netlify bot commented Feb 22, 2023

βœ… Deploy Preview for hot-sauced-ui ready!

Name Link
πŸ”¨ Latest commit fc37ee2
πŸ” Latest deploy log https://app.netlify.com/sites/hot-sauced-ui/deploys/63f6a173abfd52000819669e
😎 Deploy Preview https://deploy-preview-449--hot-sauced-ui.netlify.app/
πŸ“± Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

PR Compliance Checks

Thank you for your Pull Request! We have run several checks on this pull request in order to make sure it's suitable for merging into this project. The results are listed in the following section.

Watched Files

This pull request modifies specific files that require careful review by the maintainers.

Files Matched

  • npm-shrinkwrap.json
  • package.json

@takanome-dev
Copy link
Contributor

takanome-dev commented Feb 22, 2023

Looks great πŸ•

Did you test it for the search results as well?
Here is what I got from the deploy preview

emojify

@0-vortex
Copy link
Contributor

Hey, thanks for opening the PR. Have a couple of questions/concerns before reviewing (big πŸ‘)

  1. @takanome-dev the effort of hooking the emoji translator into every frontend bit of code seems pointless, it would be best to implement the description translator in the client hook to have it available everywhere
  2. @JoshuaKGoldberg have some maintenance concerns about node-emoji, but the biggest blocker is that it doesn't look like it's doing purely GFM emojis, curious what your thoughts are on using a GFM only emoji translator like node-github-emoji

@JoshuaKGoldberg
Copy link
Contributor Author

best to implement the description translator in the client hook

Oh, is that something you'd like me to do in this PR?

doesn't look like it's doing purely GFM emojis

Sorry, I'm not following - is it that you want to make sure it does nonstandard GitHub emojis such as :shipit: :shipit: too?

@0-vortex
Copy link
Contributor

Oh, is that something you'd like me to do in this PR?

If you are willing to edit the description in multiple places (like the search dropdown) then it would require more effort - my suggestion was to limit that effort for this PR, somehow

doesn't look like it's doing purely GFM emojis

Sorry, I'm not following - is it that you want to make sure it does nonstandard GitHub emojis such as :shipit: :shipit: too?

Just wanted to make sure it has parity with GitHub readmes, from a first time viewers' perspective, node-emoji looks like it's doing more than just GFM πŸ˜…

@JoshuaKGoldberg
Copy link
Contributor Author

the search dropdown

D'oh! I had been looking at the wrong place 🀦 just pushed fc37ee2 to apply to the searched repo card too.

limit that effort for this PR

Yeah, that's a good question. In theory the backend could normalize PR descriptions to have actual emojis rather than :name:s. But then they'd be different from the repos themselves. Not sure.

At any rate, the two codepaths that end up in this repo's emojify(description)s don't seem the same to me. But if you see a way to deduplicate I'd love that! πŸ˜„

node-emoji

Yeah I haven't found a library that works better. node-emoji seems to be in-and-out of maintenance and I never ended up pushing omnidan/node-emoji#113 through. Definitely open to suggestions here!

Copy link
Member

@bdougie bdougie left a comment

Choose a reason for hiding this comment

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

Let's ship what we have here and come back with this is live in insights to have parity.

@bdougie bdougie merged commit 7e4a601 into open-sauced:beta Apr 11, 2023
github-actions bot pushed a commit that referenced this pull request Apr 11, 2023
## [2.31.0-beta.10](v2.31.0-beta.9...v2.31.0-beta.10) (2023-04-11)

### πŸ• Features

* allow :emoji: in repository list descriptions ([#449](#449)) ([7e4a601](7e4a601)), closes [#370](#370)
@github-actions
Copy link

πŸŽ‰ This PR is included in version 2.31.0-beta.10 πŸŽ‰

The release is available on:

Your semantic-release bot πŸ“¦πŸš€

@JoshuaKGoldberg JoshuaKGoldberg deleted the repo-list-emoji-description branch April 11, 2023 20:36
This was referenced May 15, 2023
bdougie added a commit that referenced this pull request May 16, 2023
* feat: check whether repo is starred for nav UI (#431)

* chore(minor): release 2.31.0-beta.1 on beta channel [skip ci]

## [2.31.0-beta.1](v2.30.0...v2.31.0-beta.1) (2023-01-05)

### πŸ• Features

* check whether repo is starred for nav UI ([#431](#431)) ([bca5833](bca5833))

* refactor:  Add test for dayjs `getDateFromNow` method (#390)

* feat: create hook for dayjs

* test: add test for getDateFromNow method

* refactor: update avatar component to use new lib function

* refactor: update to dynamic date string

* chore: cleanup

Co-authored-by: OGBONNA SUNDAY <62995161+SunGoldTech@users.noreply.github.com>

* chore(patch): release 2.31.0-beta.2 on beta channel [skip ci]

## [2.31.0-beta.2](v2.31.0-beta.1...v2.31.0-beta.2) (2023-01-05)

### πŸ§‘β€πŸ’» Code Refactoring

*  Add test for dayjs `getDateFromNow` method ([#390](#390)) ([e5fd935](e5fd935))

* fix: remove height from footer to prevent socials from being truncated (#436)

closes #435

* chore(patch): release 2.31.0-beta.3 on beta channel [skip ci]

## [2.31.0-beta.3](v2.31.0-beta.2...v2.31.0-beta.3) (2023-01-05)

### πŸ› Bug Fixes

* remove height from footer to prevent socials from being truncated ([#436](#436)) ([149fbf0](149fbf0)), closes [#435](#435)

* chore: used ts-prune to find unused code

* fix: correct mobile view secondary nav so it wraps on two lines (#440)

closes #438

* chore(patch): release 2.31.0-beta.4 on beta channel [skip ci]

## [2.31.0-beta.4](v2.31.0-beta.3...v2.31.0-beta.4) (2023-01-06)

### πŸ› Bug Fixes

* correct mobile view secondary nav so it wraps on two lines ([#440](#440)) ([8545ce2](8545ce2)), closes [#438](#438)

* npm run format

* Add npx to ts-prune

* chore(patch): release 2.31.0-beta.5 on beta channel [skip ci]

## [2.31.0-beta.5](v2.31.0-beta.4...v2.31.0-beta.5) (2023-01-09)

### πŸ€– Build System

* used ts-prune to find unused code ([9738c56](9738c56)), closes [#432](#432)

* docs(readme): fix typo and unformatted link (#447)

* chore(minor): release 2.31.0-beta.6 on beta channel [skip ci]

## [2.31.0-beta.6](v2.31.0-beta.5...v2.31.0-beta.6) (2023-02-11)

### πŸ“ Documentation

* **readme:** fix typo and unformatted link ([#447](#447)) ([3ffd929](3ffd929))

* fix: remove discord webhook (#441)

* chore(patch): release 2.31.0-beta.7 on beta channel [skip ci]

## [2.31.0-beta.7](v2.31.0-beta.6...v2.31.0-beta.7) (2023-02-21)

### πŸ› Bug Fixes

* remove discord webhook ([#441](#441)) ([eae0ac0](eae0ac0))

* fix: update the status URL in the footer (#454)

closes #453

* fix: triage.yml (#458)

* fix: triage.yml

* Update .github/workflows/triage.yml

* Update .github/workflows/triage.yml

* chore(patch): release 2.31.0-beta.8 on beta channel [skip ci]

## [2.31.0-beta.8](v2.31.0-beta.7...v2.31.0-beta.8) (2023-03-24)

### πŸ› Bug Fixes

* triage.yml ([#458](#458)) ([aec6ba8](aec6ba8))
* update the status URL in the footer ([#454](#454)) ([8381d16](8381d16)), closes [#453](#453)

* fix: make year dynamic in footer (#462)

closes #455

* chore(patch): release 2.31.0-beta.9 on beta channel [skip ci]

## [2.31.0-beta.9](v2.31.0-beta.8...v2.31.0-beta.9) (2023-04-04)

### πŸ› Bug Fixes

* make year dynamic in footer ([#462](#462)) ([9b75f36](9b75f36)), closes [#455](#455)

* feat: allow :emoji: in repository list descriptions (#449)

Fixes #370

* chore(minor): release 2.31.0-beta.10 on beta channel [skip ci]

## [2.31.0-beta.10](v2.31.0-beta.9...v2.31.0-beta.10) (2023-04-11)

### πŸ• Features

* allow :emoji: in repository list descriptions ([#449](#449)) ([7e4a601](7e4a601)), closes [#370](#370)

---------

Co-authored-by: Josh Goldberg <git@joshuakgoldberg.com>
Co-authored-by: OGBONNA SUNDAY <62995161+OgDev-01@users.noreply.github.com>
Co-authored-by: OGBONNA SUNDAY <62995161+SunGoldTech@users.noreply.github.com>
Co-authored-by: Sebastian Gugulski <43274289+Themesses@users.noreply.github.com>
Co-authored-by: Matthew <mtfoley.mae@gmail.com>
Co-authored-by: Leonardo Montini <lion.48m@gmail.com>
Co-authored-by: Ryan Rowse <ryan.rowse@gmail.com>
Co-authored-by: Anush <anushshetty90@gmail.com>
Co-authored-by: Brian Douglas <bdougie@users.noreply.github.com>
Co-authored-by: S A G A R <110724849+tmsagarofficial@users.noreply.github.com>
github-actions bot pushed a commit that referenced this pull request May 16, 2023
## [2.31.0](v2.30.0...v2.31.0) (2023-05-16)

### πŸ§‘β€πŸ’» Code Refactoring

*  Add test for dayjs `getDateFromNow` method ([#390](#390)) ([e5fd935](e5fd935))

### πŸ“ Documentation

* **readme:** fix typo and unformatted link ([#447](#447)) ([3ffd929](3ffd929))

### πŸ› Bug Fixes

* correct mobile view secondary nav so it wraps on two lines ([#440](#440)) ([8545ce2](8545ce2)), closes [#438](#438)
* make year dynamic in footer ([#462](#462)) ([9b75f36](9b75f36)), closes [#455](#455)
* remove discord webhook ([#441](#441)) ([eae0ac0](eae0ac0))
* remove height from footer to prevent socials from being truncated ([#436](#436)) ([149fbf0](149fbf0)), closes [#435](#435)
* triage.yml ([#458](#458)) ([aec6ba8](aec6ba8))
* update the status URL in the footer ([#454](#454)) ([8381d16](8381d16)), closes [#453](#453)

### πŸ€– Build System

* bump node-emoji to 2 ([#465](#465)) ([b1f7f20](b1f7f20))
* used ts-prune to find unused code ([9738c56](9738c56)), closes [#432](#432)

### πŸ• Features

* allow :emoji: in repository list descriptions ([#449](#449)) ([7e4a601](7e4a601)), closes [#370](#370)
* check whether repo is starred for nav UI ([#431](#431)) ([bca5833](bca5833))
* Close search pop-up when escape is pressed ([#445](#445)) ([96e3072](96e3072))
@github-actions
Copy link

πŸŽ‰ This PR is included in version 2.31.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
None yet
Development

Successfully merging this pull request may close these issues.

Bug: markdown emoji do not render in summary lists
4 participants