-
Notifications
You must be signed in to change notification settings - Fork 3k
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: support Spanish emojis #20828
feat: support Spanish emojis #20828
Changes from 11 commits
d38f775
08496e8
51ac473
07b59e3
8c18175
91839ed
d9835f9
b309e68
d3778b2
23bb285
99b8343
f742447
2551336
61be178
d9820a1
e9eccaf
fe1d94b
1977161
ac4a5c2
a32d5a3
746c5af
221c763
fe06885
555c8b4
680fc05
2dc64dd
fc90352
18646ed
7ef92fc
2ab63d5
11f77ac
2dbe1ed
45dec32
361f3d9
29070e3
70bc730
353ace4
71110cb
4bed2dd
0df9868
70cce1e
467f49f
a878f3a
f9192ee
d7d91b8
bf8e80e
6461f53
cb4e9cf
a3cd2c9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -1331,20 +1331,20 @@ const CONST = { | |||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
QUICK_REACTIONS: [ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
name: '+1', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
shortcode: {en: '+1', es: '+1'}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
code: '👍', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
types: ['👍🏿', '👍🏾', '👍🏽', '👍🏼', '👍🏻'], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
types: ['👍🏻', '👍🏼', '👍🏽', '👍🏾', '👍🏿'], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. NAB but why orders changed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am also confused. As far as I can tell the types are already displayed in that order, so I don't understand why we need to change this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As you can see below, skintone numbering is weird. I think this change is reasonable. Lines 56 to 81 in 83ab016
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please make sure that this order change doesn't cause any regressions even if it's minor. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah. I'm sure |
||||||||||||||||||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
name: 'heart', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
shortcode: {en: 'heart', es: 'corazón'}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
code: '❤️', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
name: 'joy', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
shortcode: {en: 'joy', es: 'alegría'}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
code: '😂', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
name: 'fire', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
shortcode: {en: 'fire', es: 'fuego'}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
code: '🔥', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -12,7 +12,7 @@ const propTypes = { | |||||||||||||||||||||||||||
/** The emojis consisting emoji code and indices that the icons should link to */ | ||||||||||||||||||||||||||||
headerEmojis: PropTypes.arrayOf( | ||||||||||||||||||||||||||||
PropTypes.shape({ | ||||||||||||||||||||||||||||
code: PropTypes.string.isRequired, | ||||||||||||||||||||||||||||
name: PropTypes.string.isRequired, | ||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here. I don't see any reason of code -> name There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Headers has no code unlike emojis. It has only name and icon. I wanted to add meanings to the properties There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We already have localized strings here: Lines 1221 to 1233 in edf529e
We can still use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll change the headers back to original Spanish. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @stitesExpensify do you agree with this new structure? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should think about extension of supported languages, not about original structure. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm confused @s-alves10 what is the benefit of this change? In the future we're going to add other files like |
||||||||||||||||||||||||||||
index: PropTypes.number.isRequired, | ||||||||||||||||||||||||||||
icon: PropTypes.func.isRequired, | ||||||||||||||||||||||||||||
}), | ||||||||||||||||||||||||||||
|
@@ -27,7 +27,7 @@ function CategoryShortcutBar(props) { | |||||||||||||||||||||||||||
icon={headerEmoji.icon} | ||||||||||||||||||||||||||||
onPress={() => props.onPress(headerEmoji.index)} | ||||||||||||||||||||||||||||
key={`categoryShortcut${i}`} | ||||||||||||||||||||||||||||
code={headerEmoji.code} | ||||||||||||||||||||||||||||
name={headerEmoji.name} | ||||||||||||||||||||||||||||
/> | ||||||||||||||||||||||||||||
))} | ||||||||||||||||||||||||||||
</View> | ||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,37 +1,56 @@ | ||
import _ from 'underscore'; | ||
import emojis from '../../assets/emojis'; | ||
import Trie from './Trie'; | ||
import Timing from './actions/Timing'; | ||
import CONST from '../CONST'; | ||
|
||
Timing.start(CONST.TIMING.TRIE_INITIALIZATION); | ||
|
||
const emojisTrie = new Trie(); | ||
const supportedLanguages = ['en', 'es']; | ||
|
||
// Inserting all emojis into the Trie object | ||
for (let i = 0; i < emojis.length; i++) { | ||
if (emojis[i].name) { | ||
const node = emojisTrie.search(emojis[i].name); | ||
function createTrie(lang = 'en') { | ||
const trie = new Trie(); | ||
|
||
// Inserting all emojis into the Trie object | ||
_.forEach(emojis, (item) => { | ||
if (item.header) { | ||
return; | ||
} | ||
|
||
const shortcode = item.shortcode[lang]; | ||
if (!shortcode) { | ||
return; | ||
} | ||
|
||
const node = trie.search(shortcode); | ||
if (!node) { | ||
emojisTrie.add(emojis[i].name, {code: emojis[i].code, types: emojis[i].types, suggestions: []}); | ||
trie.add(shortcode, {code: item.code, types: item.types, shortcode: item.shortcode, suggestions: []}); | ||
} else { | ||
emojisTrie.update(emojis[i].name, {code: emojis[i].code, types: emojis[i].types, suggestions: node.metaData.suggestions}); | ||
trie.update(shortcode, {code: item.code, types: item.types, shortcode: item.shortcode, suggestions: node.metaData.suggestions}); | ||
} | ||
|
||
if (emojis[i].keywords) { | ||
for (let j = 0; j < emojis[i].keywords.length; j++) { | ||
const keywordNode = emojisTrie.search(emojis[i].keywords[j]); | ||
if (!keywordNode) { | ||
emojisTrie.add(emojis[i].keywords[j], {suggestions: [{code: emojis[i].code, types: emojis[i].types, name: emojis[i].name}]}); | ||
} else { | ||
emojisTrie.update(emojis[i].keywords[j], { | ||
...keywordNode.metaData, | ||
suggestions: [...keywordNode.metaData.suggestions, {code: emojis[i].code, types: emojis[i].types, name: emojis[i].name}], | ||
}); | ||
} | ||
const keywords = item.keywords[lang]; | ||
for (let j = 0; j < keywords.length; j++) { | ||
const keywordNode = trie.search(keywords[j]); | ||
if (!keywordNode) { | ||
trie.add(keywords[j], {suggestions: [{code: item.code, types: item.types, name: shortcode, shortcode: item.shortcode}]}); | ||
} else { | ||
trie.update(keywords[j], { | ||
...keywordNode.metaData, | ||
suggestions: [...keywordNode.metaData.suggestions, {code: item.code, types: item.types, name: shortcode, shortcode: item.shortcode}], | ||
}); | ||
} | ||
} | ||
} | ||
}); | ||
|
||
return trie; | ||
} | ||
|
||
const emojiTrie = {}; | ||
_.forEach(supportedLanguages, (lang) => { | ||
emojiTrie[lang] = createTrie(lang); | ||
}); | ||
|
||
Timing.end(CONST.TIMING.TRIE_INITIALIZATION); | ||
dummy-1111 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
export default emojisTrie; | ||
export default emojiTrie; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you stick to original structure? I don't see any reason why
name
should be changed toshortcode
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is for clarity. They are not names of emojis, just short codes of them. I mentioned this in issue before
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Strictly, they are "short name", not code.
If I follow you, code = '😄', short code = 'smile',
Why
code
length should be smaller thanshort code
length?As we don't support name for now, we can just use
name
instead ofshort name
.So let's keep original structure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that the name should always be in english. In the database we save the reactions like this:
{"emoji":"heart","users":[{"accountID":8505565,"skinTone":-1}...]
If we start changing that then you would be able to react with the same emoji in both english and spanish.