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: Update to Emoji 15 #270

Merged
merged 1 commit into from
Jun 11, 2023

Conversation

susnux
Copy link
Contributor

@susnux susnux commented May 31, 2023

Update the emoji data to version 15.

@serebrov serebrov changed the base branch from master to susnux-feat-emoji-15 June 1, 2023 12:26
Copy link
Owner

@serebrov serebrov left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! Please check my question.

Also, some tests are failing, but that is expected - emojis changed positions and we need to update test snapshots, as described here, please run npm run jest -- -u to update snapshots.

I changed the base for this PR to a branch instead of master, so I can test it more after merge before pushing to master.

@@ -281,7 +281,7 @@ export default {
let emoji = index.findEmoji(':smile:')
// Note, that position in the emoji sheet is calculated by
// `emoji` object
let style = `background-position: ${emoji.getPosition()}; background-image: url(https://unpkg.com/emoji-datasource-twitter@14.0.0/img/twitter/sheets-256/64.png); width: 24px; height: 24px; display: inline-block; background-size: 5700%`
let style = `background-position: ${emoji.getPosition()}; background-image: url(https://unpkg.com/emoji-datasource-apple@15.0.0/img/apple/sheets-256/64.png); width: 24px; height: 24px; display: inline-block; background-size: 5700%`
Copy link
Owner

Choose a reason for hiding this comment

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

❓ Is the twitter -> apple change functional (something did not work) or just visual (apple looks better?)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can revert this, mostly this was from a different change (twitter emojis are discontinued and do not provide Emoji 15).

Also adjusted the tests for the new emoji tiles

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@susnux
Copy link
Contributor Author

susnux commented Jun 5, 2023

Also, some tests are failing, but that is expected - emojis changed positions and we need to update test snapshots, as described here, please run npm run jest -- -u to update snapshots.

@serebrov I have adjusted the tests for Emoji 15, tests are now running successfully.

@serebrov serebrov merged commit 0ab9062 into serebrov:susnux-feat-emoji-15 Jun 11, 2023
1 of 2 checks passed
@serebrov serebrov mentioned this pull request Jun 11, 2023
@serebrov
Copy link
Owner

@susnux Thanks! I merged your change to a branch. I will continue (test/merge/release) in #273.

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.

None yet

2 participants