Skip to content

Fix emojis not showing in commit messages#5168

Merged
techknowlogick merged 3 commits into
go-gitea:masterfrom
jamesa:emoji-in-commit-msg
Oct 29, 2018
Merged

Fix emojis not showing in commit messages#5168
techknowlogick merged 3 commits into
go-gitea:masterfrom
jamesa:emoji-in-commit-msg

Conversation

@jamesa
Copy link
Copy Markdown
Contributor

@jamesa jamesa commented Oct 24, 2018

Fixes #5150

This fixes the issue where emojis were not showing up in the commit messages in the file tree, and on the commit view.

I was looking at two approaches for this:

  1. Modify the code that generates the commit message links
  2. Modify the frontend code that calls emojify

I went with the second approach and would like some feedback on that. I went this route instead of modifying the code that generates the commit link, since it looked like I would need to make a new processor in modules/markup/html.go to facilitate adding the has-emoji class to these links. And that seemed like overkill for this small use case.

It seems natural for emojify to work on children of the has-emoji elements, but it doesn't. (Deprecated, and only used to work by ID). So this will look at the children of each has-emoji element and insert emojis into any children that are links.

I only added emojis to links because I was worried about potential performance impacts if an element has a lot of children. If you think that's still a concern though then maybe we should go with the first approach?

Thanks for your consideration and feedback!

Fixes #5150

Signed-off-by: James Anderson <james@jamesa.me>
@jamesa
Copy link
Copy Markdown
Contributor Author

jamesa commented Oct 24, 2018

Forgot to add screenshots:
screen shot 2018-10-24 at 3 29 29 pm
screen shot 2018-10-24 at 3 29 43 pm

@bkcsoft bkcsoft added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Oct 24, 2018
@codecov-io
Copy link
Copy Markdown

codecov-io commented Oct 24, 2018

Codecov Report

❗ No coverage uploaded for pull request base (master@d4e6278). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##             master   #5168   +/-   ##
========================================
  Coverage          ?   37.5%           
========================================
  Files             ?     309           
  Lines             ?   45827           
  Branches          ?       0           
========================================
  Hits              ?   17187           
  Misses            ?   26177           
  Partials          ?    2463

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 d4e6278...52496fc. Read the comment docs.

@lunny lunny added the type/enhancement An improvement of existing functionality label Oct 25, 2018
@lunny lunny added this to the 1.7.0 milestone Oct 25, 2018
@bkcsoft bkcsoft added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Oct 29, 2018
@bkcsoft bkcsoft added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Oct 29, 2018
@techknowlogick techknowlogick merged commit 1ceae07 into go-gitea:master Oct 29, 2018
@jamesa jamesa deleted the emoji-in-commit-msg branch October 30, 2018 01:00
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/enhancement An improvement of existing functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Emojis not rendering in Repo view or Commit view

6 participants