993899 - [Keyboard] Emoji layout in keyboard

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Keyboard, defect, P1)

Description

[Omega Feng [:Omega] [:馮於懋]]

As a user, I can input Emoji with built-in Keyboard.

Comment 1

[David Flanagan [:djf]]

We should totally do this.  And I suspect that it would be pretty easy to add new layout pages to our existing layouts. We'd just have to figure out where to squeeze in the mode switch key.

Rudy: what you do you think about this?

Carrie or Juwei: Do you have UX comments about supporting emoji?

Comment 2

[David Flanagan [:djf]]

I'm assuming, however, that our default font supports emoji.  If not, then this becomes a lot harder.

Comment 3

[Raniere Silva]

At the begin of this week I was looking for the keyboard recommendations and found [1]. It is a little old (from 2012) it says in slide 2:

"Tap and hold to access common smileys (or emoji) and punctuation symbols."

Based on the recommendations we don't need to add new layout since it should works like the punctuation symbols.

I want to work on this as part of my GSoC project.

[1] https://wiki.mozilla.org/images/2/20/Keyboard_Specs.pdf

Comment 4

[David Flanagan [:djf]]

As a practical matter, there is probably only room for one or two smilies or emoji characters in the alternate character menu for '.', so this can only be a partial solution. Our Fira font does not seem to include emoji, but if I visit the wikipedia emoji page in the FirefoxOS browser it does display emoji characters, so I guess we support them somehow.

Comment 5

[Raniere Silva]

Attachment: Add emoticon

Comment 6

[Raniere Silva]

My last attachment add support to emoticons. For emoji see https://github.com/r-gaia-cs/gaia/commit/371dc3f099cfba1c59c9af50d21e90b4044cd525.

@David AFAIK our font only support a VERY small number emoji and that's the reason that I submit the patch with emoticons.

Comment 7

[David Flanagan [:djf]]

Comment on attachment 8426522 [details] [review]
Add emoticon

This is interesting as a proof-of-concept, but it is premature to review without first getting thoughts from the keyboard UX team about what is wanted here.

I'd prefer a solution that did not need JS code to make the emoticon key appear: there ought to be a way to just have that in the layout file. Also, this patch probably interacts poorly with the email and url layouts which have other special keys to the left of the spacebar.  (The UI tests app includes input fields of these types so you can check your layouts for urls and email addresses.)

Our Fira sans font does not have emoji in it, but somehow our browser app displays emoji, presumably with a different font.  So if we want to support emoji we may need to use an additional font.  Note that this would mean we would have to use the additional font in every app that used the keyboard or it would be possible to type characters that could not be displayed. So this might be an issue that goes far beyond the keyboard itself.

Comment 8

[David Flanagan [:djf]]

See http://djf.net/smile.html for a test that demonstrates at least one emoji character in our browser.

Comment 9

[Rudy Lu [:rudyl] (inactive after 2015/7/3)]

Hi all,

I think it is reasonable to add one more layout for emoji instead of just embedding those chars into one key's alternative char menu.

If we go this way, then the question would be if we could switch to emoji layout with just the ime switch key (the globe key) or we need to add another mode-switching key for this, as David mentioned in comment 1.

--
Raniere,

Thanks for looking into this, but as you may already, the spec you're reading is an old one, and many things change since then. 
BTW, we have a spec in bug 983043 for keyboard update 2.0. However, it seems this emoji part was not covered yet.

Thanks.

--
++ni Omega, for he is the recent keyboard UX goto-man, AFAIK. :)

Comment 10

[Omega Feng [:Omega] [:馮於懋]]

(In reply to Rudy Lu [:rudyl] from comment #9)
> I think it is reasonable to add one more layout for emoji instead of just
> embedding those chars into one key's alternative char menu.
> If we go this way, then the question would be if we could switch to emoji
> layout with just the ime switch key (the globe key) or we need to add
> another mode-switching key for this, as David mentioned in comment 1.
> ++ni Omega, for he is the recent keyboard UX goto-man, AFAIK. :)

I think it's good idea to treat Emoji as an independent keyboard (use IME switch key) since it has a lot of tabs and controls already.

@Bruce, IIRC we'll target Emoji to v2.1, right?

Comment 12

[Rudy Lu [:rudyl] (inactive after 2015/7/3)]

Just found that we already have emoji support in our font system, so we could implement this.

Comment 13

[Ahmed Nefzaoui [:Nefzaoui]]

Attachment: emoji layout screenshot

Back when the emoji font first landed I did this little experiment. Having a separate layout for emoji may look great, but it could be greater with a little more customizations since it's not just a bunch of letters in 4 or 5 rows & lines.
I'd say a global secondary layout with custom styles (a little different from the usual keyboard styling) and accessed through a smile ":)" button between "space bar" and "." buttons..
We could also give the user the ability to enable/disable it
?

Comment 14

[Rudy Lu [:rudyl] (inactive after 2015/7/3)]

We have a spec update on emoji support,
Please refer to bug 983043 comment 23.

Comment 15

[Raniere Silva]

I took a look at the spec but didn't understand how the user will move into emoji layout and go out of it.

Comment 16

[Tim Guan-tin Chien [:timdream] (please needinfo)]

https://bug983043.bugzilla.mozilla.org/attachment.cgi?id=8452129#22

Comment 17

[Rudy Lu [:rudyl] (inactive after 2015/7/3)]

(In reply to Raniere Silva from comment #15)
> I took a look at the spec but didn't understand how the user will move into
> emoji layout and go out of it.

I think this should live as a separate layout, so you switch to emoji with [IME switching] key and back to other layouts, like English, with the same action, I suppose [IME switching] key on emoji layout is that smiley icon located at the lower left button.

Comment 18

[Raniere Silva]

(In reply to Rudy Lu [:rudyl] from comment #17)
> I think this should live as a separate layout, so you switch to emoji with
> [IME switching] key and back to other layouts, like English, with the same
> action, I suppose [IME switching] key on emoji layout is that smiley icon
> located at the lower left button.

If, for example, you have only English and Emoji layout this will be ok but if you have French, Germany and Emoji layouts this will became a problem since you will need to press more than one key to get the Emoji layout and/or back to the language layout.

Omega, could you get a feedback from UX team for this comment?

Comment 19

[Ahmed Nefzaoui [:Nefzaoui]]

(In reply to Raniere Silva from comment #18)
> (In reply to Rudy Lu [:rudyl] from comment #17)
> > I think this should live as a separate layout, so you switch to emoji with
> > [IME switching] key and back to other layouts, like English, with the same
> > action, I suppose [IME switching] key on emoji layout is that smiley icon
> > located at the lower left button.
> 
> If, for example, you have only English and Emoji layout this will be ok but
> if you have French, Germany and Emoji layouts this will became a problem
> since you will need to press more than one key to get the Emoji layout
> and/or back to the language layout.
> 
> Omega, could you get a feedback from UX team for this comment?

I Agree.. And another suggestion for how this should work is having a separate key between [Space] and [.] keys (or somewhere else easy to access) that acts like a secondary layout just like the [12&] Key.
And It should be accessible through all keyboard layouts..

Comment 20

[Omega Feng [:Omega] [:馮於懋]]

I agree it's easier to switch to Emoji layout with a key in each layout, but that causes some problem:
1) The bottom row of each layout is already cramped. Adding any key there would make UX bad.
2) If we don't make Emoji a separate layout, user will have no chance to use 3rd party keyboard with Built-in Emoji.

To avoid the cons, we decided to use a separate layout for Emoji.

Comment 21

[Carol Huang [:Carol]]

Attachment: 2.1 Emoji Keyboard_Visual spec.zip

Hi,
The attachment is "2.1 Emoji Keyboard_Visual spec"
Please let me know if there's any question about the visual spec.
Thanks!

Comment 22

[Bruce Huang [:bhuang] (inactive)]

(In reply to Omega Feng [:Omega] [:馮於懋] from comment #10)
> 
> @Bruce, IIRC we'll target Emoji to v2.1, right?

Yes, but we'll confirm it after completing the 2.1 blockers.

Comment 23

[Carol Huang [:Carol]]

Attachment: 2.1 Emoji Keyboard_Visual spec_0818.zip

Update Emoji Keyboard Visual spec:
1. Categoery tab width
2. image assets (on/off/pressed)

Comment 24

[Ahmed Nefzaoui [:Nefzaoui]]

The confusing part here is how the smilies/emojis themselves are actually going to be implemented. Are there going to be a script that converts plain text like ":-)" & ":-/" &  ":-(" to the icons provided and vise versa? If so, how fox emojis are constructed? Something like :happyfox: ?

If they're going to be inserted as <img> within html? How are going to deal with the SMS case?

So many details I don't see them being discussed anywhere? :)

Comment 25

[Jan Jongboom [:janjongboom] (inactive)]

emoji's have their own utf8 code.

Comment 26

[Sabrina Smelko]

:Nefzaoui Emoji's follow the Unicode standard as Jan was saying. They're all outlined here: https://mozilla.app.box.com/files/0/f/2098581252

Pavel/Rudy, what's the status on these emojis? What's it looking like at this point? Anything I can do to help? Pavel, last I checked in, I asked what size would be optimal for me to export at. Let me know :)

Comment 27

[Rudy Lu [:rudyl] (inactive after 2015/7/3)]

Pavel will help to work on the font part, and I am going to implement a new keyboard layout for this, but haven't started yet, will send my patch here for feedback when I have WIP.

Comment 28

[howie [:howie]]

Feature does not block, marking feature-b2g 2.2? for scoping.

Comment 29

[Omega Feng [:Omega] [:馮於懋]]

See bug 983043 for UX spec update.

Comment 30

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Mass-unassign feature-b2g: 2.2+ to re-scope 2.2 features.

Comment 31

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Marking 2.2 features for 3.0 considerations.

Comment 32

[Ralph Giles (:rillian)]

Just connecting the dots...Thomas Beduneau has an emoji keyboard in the marketplace. This might serve as a useful starting place, and a solution for users until this bug lands.

https://marketplace.firefox.com/app/emoji-keyboard/
https://github.com/enwin/emoji

The design is similar to Carol's spec, but not identical. It does share the ime switch button not lining up with the same function on roman keyboards, which I find annoying in practice. But not as annoying as not being able to type emoji. :) FWIW.

Comment 33

[Reza Akhavan [:jedireza]]

Hi Ralph. Unfortunately the emoji keyboard you mentioned is close to unusable (I posted yesterday about this in the mailing list). And I'd rather not derail this bug with alternatives. We should have all the pieces needed to ship an emoji keyboard with our own custom artwork. Let's stay focused on landing.

Comment 34

[Reza Akhavan [:jedireza]]

Thomas emailed me after my previous comment. To clarify the unusable symptoms of Thomas's keyboard are only present on my ZC3. It seems to be working well on the Flame.

Hopefully Thomas and I can get those issues worked out for the ZC3. It would still be nice for his keyboard to function well on that device as well.

Again, I'd like us to focus on landing an official FxOS emoji keyboard.

Comment 35

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Reza Akhavan [:jedireza] from comment #34)
> Again, I'd like us to focus on landing an official FxOS emoji keyboard.

Do ping me if you need help to test out what's in the code base currently.

Comment 36

[Reza Akhavan [:jedireza]]

Patryk, Tim and I shared an email thread and we talked about hiding the recent emoji tab to simplify the scope and get this landed. We think it's reasonable to create a follow up bug for adding the recent emoji tab feature later.

I've started working on this. So far I've hid the recent emoji tab and added the emoji keyboard to the Makefile.

You can see the changes I've made so far on my `emoji-keyboard` branch:

https://github.com/mozilla-b2g/gaia/compare/master...jedireza:emoji-keyboard?expand=1

After getting the current keyboard and font on my device I noticed that some emoji are missing. You can see which one's here:

https://gist.github.com/jedireza/c78209d2b4554de5d59e#file-01-missing-md

Thank you Tim for helping me get the current keyboard installed on my device. I kept notes on the dev process so far. You can follow along here:

https://gist.github.com/jedireza/c78209d2b4554de5d59e#file-02-dev-notes-md

This is really close to be shippable. It seems that we just need to resolve the missing emoji. What other road blocks can/will we run into after these changes?

Comment 37

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Reza Akhavan [:jedireza] from comment #36)
> Patryk, Tim and I shared an email thread and we talked about hiding the
> recent emoji tab to simplify the scope and get this landed. We think it's
> reasonable to create a follow up bug for adding the recent emoji tab feature
> later.
> 
> I've started working on this. So far I've hid the recent emoji tab and added
> the emoji keyboard to the Makefile.
> 
> You can see the changes I've made so far on my `emoji-keyboard` branch:
> 
> https://github.com/mozilla-b2g/gaia/compare/master...jedireza:emoji-
> keyboard?expand=1
> 

Please simply remove the button definition instead of comment it out. 
To create the patch to enable it, remove this tag to make it ship by default instead

https://github.com/jedireza/gaia/blob/d42afee5f90839eb1fef37c88adadc5d6823c02a/apps/keyboard/js/layouts/emoji.js#L5-L8

keyboard_test.js would need some adjustment too.

> After getting the current keyboard and font on my device I noticed that some
> emoji are missing. You can see which one's here:
> 
> https://gist.github.com/jedireza/c78209d2b4554de5d59e#file-01-missing-md
> 
> Thank you Tim for helping me get the current keyboard installed on my
> device. I kept notes on the dev process so far. You can follow along here:
> 
> https://gist.github.com/jedireza/c78209d2b4554de5d59e#file-02-dev-notes-md
> 
> This is really close to be shippable. It seems that we just need to resolve
> the missing emoji. What other road blocks can/will we run into after these
> changes?

The missing smiley might be a font priority issue. We would need to tweak that in a Gecko pref instead but that might not be the best approach. We should loop :jfkthame if that's indeed the case.

Thank you for taking the initiative!

Comment 38

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Tim Guan-tin Chien [:timdream] (slow response; please ni? to queue) from comment #37)
> keyboard_test.js would need some adjustment too.

Don't worry about this test, it is corrected in bug 1184802.

Comment 39

[Reza Akhavan [:jedireza]]

Tim, I've made the changes you requested. PR updated:
https://github.com/mozilla-b2g/gaia/compare/master...jedireza:emoji-keyboard?expand=1

I emailed with Pavel about the missing emojis. It looks like we indeed are missing some and need some help getting the artwork finished. Pavel mentioned checking with Patryk, who IIRC is just getting back from vacation.

Looks like the only blocker right now is the missing emoji.

Comment 40

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Reza Akhavan [:jedireza] from comment #39)
> Tim, I've made the changes you requested. PR updated:
> https://github.com/mozilla-b2g/gaia/compare/master...jedireza:emoji-
> keyboard?expand=1

The patch is ready I think. Send me a pull request and I will r+ it.

> 
> I emailed with Pavel about the missing emojis. It looks like we indeed are
> missing some and need some help getting the artwork finished. Pavel
> mentioned checking with Patryk, who IIRC is just getting back from vacation.
> 
> Looks like the only blocker right now is the missing emoji.

As I said in the e-mail, I think that's a font precedence issue. You should attach the screenshots and ask :jfkthame on this, either on a new bug that would made the necessary change on Gecko/B2G or this bug.

Thanks!

Comment 41

[GitHub Autolander Bot]

Attachment: [gaia] jedireza:emoji-keyboard > mozilla-b2g:master

Comment 42

[Reza Akhavan [:jedireza]]

> The patch is ready I think. Send me a pull request and I will r+ it.

https://github.com/mozilla-b2g/gaia/pull/31263

> As I said in the e-mail, I think that's a font precedence issue. You should attach the screenshots and ask :jfkthame on this, either on a new bug that would made the necessary change on Gecko/B2G or this bug.

I chatted with :pivanov and he's fixed a lot of the missing emoji. Currently it looks like some of the emoji codes exist in default font and the Emoji font is being used as a fallback to the default. I trust he'll work through that.

We're getting there. :)

Comment 43

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8643924 [details] [review]
[gaia] jedireza:emoji-keyboard > mozilla-b2g:master

So we will land this only after the font issue is resolved right?

Comment 44

[Reza Akhavan [:jedireza]]

> So we will land this only after the font issue is resolved right?

Yes, I was going to wait for the font issues to be resolved before adding the checkin-needed flag.

Comment 45

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

Hey Guys, 

I fixed the Emoji font ... but I found an issue when we use it for example in SMS App.
If we want to use Emoji Icons instead the default Unicode icon versions of default font (https://en.wikipedia.org/wiki/Emoji#ref_U2700_as_of_Unicode_version) we need to set font-family: "FirefoxEmoji", sans-serif; to element who display the emojis.

We can set here https://github.com/mozilla-b2g/gaia/blob/master/apps/sms/views/shared/style/composer.css#L23
font-family: "FirefoxEmoji", sans-serif; and the message input element will show the correct emojis and text. But we need to do this in few more places in SMS App and if we want to copy/paste the icon in other app or just use Emoji Keyboard in some other app we need to be sure that the icons will be displayed from our Emoji font not from the default one.

Can we do something better/clever? for example ... what if we set FirefoxEmoji to be the default font for gaia and Fira to be second one in the list? Something like font-family: "FirefoxEmoji", "Fira Sans", sans-serif;

Hope that my explanation is not a mess.

Tim? Reza? Any thoughts?

Comment 46

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Pavel Ivanov [:ivanovpavel][:pivanov] from comment #45)
> We can set here
> https://github.com/mozilla-b2g/gaia/blob/master/apps/sms/views/shared/style/
> composer.css#L23
> font-family: "FirefoxEmoji", sans-serif; and the message input element will
> show the correct emojis and text. But we need to do this in few more places
> in SMS App and if we want to copy/paste the icon in other app or just use
> Emoji Keyboard in some other app we need to be sure that the icons will be
> displayed from our Emoji font not from the default one.
> 
> Can we do something better/clever? for example ... what if we set
> FirefoxEmoji to be the default font for gaia and Fira to be second one in
> the list? Something like font-family: "FirefoxEmoji", "Fira Sans",
> sans-serif;

I think we should just make the Emoji as the default font, if there is no compatibility concern.

BTW, why was the font named "FirefoxEmoji" and not "Firefox Emoji" or any other special name of it's own right? We should not be exposing the internal working name to the world.

Johnathan, I couldn't help but notice we do not put Apple Color Emoji on Mac as a default font (as opposed to Safari). Should we do that on B2G? Thanks.

Comment 47

[Jonathan Kew [:jfkthame]]

I don't think we should make FirefoxEmoji the default or first font for content in general, just so it'll get used for any emoji codepoints that happen to be present..... this will hurt performance globally, because for the vast majority of content we'll be first testing a font that doesn't support the characters in the text, and only second falling back to a font that's actually usable.

What we probably want to do instead is to return FirefoxEmoji from GetCommonFallbackFonts for the appropriate codepoint ranges, so that when an emoji codepoint occurs in the midst of content styled with a standard (text) font, we'll pick the desired fallback. This is what we do for Apple Color Emoji, for example (in gfxPlatformMac::GetCommonFallbackFonts), and for Segoe UI Emoji (in gfx::WindowsGetCommonFallbackFonts).

Currently, AFAIK the implementation in gfxAndroidPlatform is shared between Android and B2G, but we do have some #ifdef'd code in there that distinguishes between MOZ_WIDGET_ANDROID and MOZ_WIDGET_GONK. So I think it'd make sense to add a B2G-specific fragment in GetCommonFallbackFonts there and make it prefer the FirefoxEmoji font.

It looks like this bug is about implementing the keyboard layout, so I'd suggest filing a new bug (in Core::Graphics:Text) for the Gecko change needed here. It should be a pretty simple patch.

Comment 48

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

Hey Jonathan,

here is the Core::Graphics:Text bug 1193481.
Can you help with the bug summary if is not correct?

Tim: I will change the font name to "Firefox Emoji".
Reza: I will fix few glyphs in to the font but I think after bug 1193481 we are ready to land this one.

Comment 49

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

Hey Reza,

Patryk has few comments which I think are reasonable can we fix them? Here are the comments:

1. Swiping and tapping on an emoji both cause vibration feedback. Swiping should not do this. On swipe = no vibration.

2. When you have the emoji keyboard enabled in the main keyboard screen the “EN” button should show a the smiley emoji. When in the emoji keyboard to get back to english keyboard we should be using the “EN” graphic and not a smiley emoji. So basically the “EN” and the “smiley emoji” need to be reversed.

Thanks in advance :)

Comment 50

[Reza Akhavan [:jedireza]]

I'm on it.

Comment 51

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

Thanks Reza :)

Comment 52

[Reza Akhavan [:jedireza]]

> 1. Swiping and tapping on an emoji both cause vibration feedback. Swiping should not do this. On swipe = no vibration.

I started looking into this but haven't made any progress yet.

> 2. When you have the emoji keyboard enabled in the main keyboard screen the “EN” button should show a the smiley emoji. When in the emoji keyboard to get back to english keyboard we should be using the “EN” graphic and not a smiley emoji. So basically the “EN” and the “smiley emoji” need to be reversed.

I'm not sure we should change this or if we do, not include it as a blocker for this bug. The reason I say that is because it's how the keyboard works today. For example on my Foxfooding device I used 3 keyboards; English, Erabic and Emoji (from the marketplace). When I choose a keyboard I see "En" when I'm on English keyboard, "Ar" when I'm on the Arabic keyboard and a Smiley when I'm on the emoji keyboard. In this case it wouldn't be a simple toggle between two keyboards.

Comment 53

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

Patryk, thoughts?

Comment 54

[Jonathan Kew [:jfkthame]]

(In reply to Reza Akhavan [:jedireza] from comment #52)

> > 2. When you have the emoji keyboard enabled in the main keyboard screen the “EN” button should show a the smiley emoji. When in the emoji keyboard to get back to english keyboard we should be using the “EN” graphic and not a smiley emoji. So basically the “EN” and the “smiley emoji” need to be reversed.
> 
> I'm not sure we should change this...

Right; I don't think we should do that. This symbol indicates the *current* keyboard layout, not the layout that tapping it will switch to. The suggested change would be quite confusing for users with multiple keyboards enabled.

Comment 55

[Patryk Adamczyk [:patryk] UX]

For #2, spoke with interaction designers, and let's keep it as is.

Comment 56

[Reza Akhavan [:jedireza]]

> 1. Swiping and tapping on an emoji both cause vibration feedback. Swiping should not do this. On swipe = no vibration.

Tim, some guidance on this would be much appreciated. I found `apps/keyboard/js/keyboard/feedback_manager.js` but finding it difficult to track back down exactly how feedback is triggered/controlled.

I've also noticed while using the keyboard with the vibration setting turned on, that it "feels" as if the vibration is happening immediately (like a touchstart) so I wonder how much will need to change to prevent this feedback when a swipe is detected.

Comment 57

[Tim Guan-tin Chien [:timdream] (please needinfo)]

WebIDE and a break point pointing at [1] can give you a stack trace on where the vibrate is originated.

[1] https://github.com/mozilla-b2g/gaia/blob/e9d7c4e3ede5ca1559d/apps/keyboard/js/keyboard/feedback_manager.js#L49

My educated guess is that it come from the DefaultTargetHandler here [2]

[2] https://github.com/mozilla-b2g/gaia/blob/e9d7c4e3ede5ca1559d7375ac78a7f97d40dc414/apps/keyboard/js/keyboard/target_handlers.js#L17

This is pretty problematic because we do not know if the user intends to swipe or tap at the time of touchstart, but we always trigger feedbacks at touchstart. This applies not only to swipe-able Emoji layout but also candidate selection panel for Asian IMEs.

If we can't live with that we would have to change the UX:

1. Don't trigger feedback at all for keys on the swipe-able panels.
2. Trigger feedback at touchend instead.

Comment 58

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Tim Guan-tin Chien [:timdream] (slow response; please ni? to queue) from comment #57)
> 2. Trigger feedback at touchend instead.

Note that this will make people feel the keyboard is less responsive.

Comment 59

[Reza Akhavan [:jedireza]]

> This applies not only to swipe-able Emoji layout but also candidate selection panel for Asian IMEs.

Let's create a separate bug for handling keyboard feedback, specifically as it relates to swipes? It doesn't sound like vibration should block this bug.

Comment 60

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Reza Akhavan [:jedireza] from comment #59)
> > This applies not only to swipe-able Emoji layout but also candidate selection panel for Asian IMEs.
> 
> Let's create a separate bug for handling keyboard feedback, specifically as
> it relates to swipes? It doesn't sound like vibration should block this bug.

I agree.

Comment 61

[Peter Dolanjski [:pdol]]

[Tracking Requested - why for this release]: Brought up in foxfooding, duping foxfood bug to this one.

Comment 63

[Tim Guan-tin Chien [:timdream] (please needinfo)]

What's the status of this bug?

Comment 64

[Reza Akhavan [:jedireza]]

We seem to be in good shape. Pavel recently finished updating the font (bug 1207260).

Pavel, what other things are needed? Are there any other blockers?

Comment 65

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

Just waiting to land bug 1206844 and we can land this one.

Comment 66

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Applying the patch I am seeing red background from bug 1203455 on the Emoji layout, which is not intended.

https://github.com/mozilla-b2g/gaia/blob/master/apps/keyboard/style/keyboard.css#L28

Please set a background color on the container on the swiping panel.

Comment 67

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8643924 [details] [review]
[gaia] jedireza:emoji-keyboard > mozilla-b2g:master

Removing r+ per comment 66.

Comment 68

[Reza Akhavan [:jedireza]]

Comment on attachment 8643924 [details] [review]
[gaia] jedireza:emoji-keyboard > mozilla-b2g:master

PR updated: rebased from master and added the swipe panel background color.

Comment 69

[Reza Akhavan [:jedireza]]

Comment on attachment 8643924 [details] [review]
[gaia] jedireza:emoji-keyboard > mozilla-b2g:master

Tim there were some build integration tests that were failing. I updated the PR to fix them. Could you take a look at the last commit and let me know if the changes are reasonable.

https://github.com/jedireza/gaia/commit/d6d85e267bc5266a4ee17180d6c066d68a92e8e5

Comment 70

[Tim Guan-tin Chien [:timdream] (please needinfo)]

For the two

assert.deepEqual(inputKeysInManifest.sort(),
-                       [].concat(layoutIds, 'number').sort());
+                       [].concat(layoutIds, 'number', 'emoji').sort());

You chould just find the |var layoutIds| of each test and add |emoji| into the array.

Otherwise the patch looks fine.

Comment 71

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Please also rebase on top of bug 1216409.

Comment 73

[Reza Akhavan [:jedireza]]

Comment on attachment 8643924 [details] [review]
[gaia] jedireza:emoji-keyboard > mozilla-b2g:master

Rebased off of master from a few moments ago. Also applied the VS16 to BMP conversion. Thanks for the script Tim.

Comment 74

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Pavel, the see comment I leave on the pull request. I have problems with some of the symbols:

-- some of them are added to the Emoji layout but they don't exist in the font, please remove them from the layout or update the font file again, or both (as the number cases have shown).
-- the flag symbols are placed on the wrong code points.

Please refer to bug 1216399 comment 4 on how BMP code points works with Emoji and non-Emoji. I asked Reza to remove the CSS style to reveal our problem here. We should not be using CSS to target Emoji rendering but to specify what to show at Unicode level.

Comment 75

[Tim Guan-tin Chien [:timdream] (please needinfo)]

FYI I just found the authoritative documents.

http://www.unicode.org/Public/emoji/1.0//emoji-data.txt

This is the list of Emoji.

http://unicode.org/Public/UCD/latest/ucd/StandardizedVariants.html

and this is list of standardized variants which append an VS16 should convert the boring symbol into Emoji. We should always make sure black white symbols on the Emoji layout comes with an VS16 (and only show Emoji when VS16 exists).

I don't think what bug 1216409 do is entirely correct in this case ... we probably don't need VS16 for some BMP characters and we probably need VS16 for some non-BMP characters. This document should give us the standard guideline.

Comment 76

[Tim Guan-tin Chien [:timdream] (please needinfo)]

There is a ordering chart that could help us design the layout.

http://unicode.org/emoji/charts/emoji-ordering.html

(these charts are published at http://unicode.org/emoji/charts/index.html )

Comment 77

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Pavel, what's the update on comment 74? I don't see pull request being updated nor an updated Emoji font file.

Comment 78

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

Work on in ... I will update the PR tonight and I will ping Reza about it

Comment 79

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

Thing that we can ask for r+ (bug 1221419 will polish the Emoji font) Reza if you are OK :) ask for r+

Comment 80

[Reza Akhavan [:jedireza]]

Comment on attachment 8643924 [details] [review]
[gaia] jedireza:emoji-keyboard > mozilla-b2g:master

Recently rebased with master to ensure treeherder stayed green:
https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=8a0bc740e643cc8d22382d8d7359bed98e1ff397

Tim, I also edited the commit messages of our PR to include the bug id so we could keep both Pavel's and my commit history as is without squashing.

Comment 81

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8643924 [details] [review]
[gaia] jedireza:emoji-keyboard > mozilla-b2g:master

Thanks for updating the commit comments.

I am seeing bug 1216409 is being reverted :'(. I don't see any VS16 is kept in the file (or, it's not visible to me because the current patch does not use escape sequences).

Please check the table at comment 75 and make sure all BMP Emoji codepoint that needs an VS16 has one with it. We need to ensure key label and keyboard output contains VS16 so Emoji don't accidentally turn black and while anywhere.

Comment 82

[Tim Guan-tin Chien [:timdream] (please needinfo)]

I am pretty sure the current patch stripped all VS16 -- I am seeing black and white Emoji just by looking at [1] in Safari.

[1] https://raw.githubusercontent.com/jedireza/gaia/emoji-keyboard/apps/keyboard/js/layouts/emoji.js

If there are VS16 then Safari will pick Color Emoji in this unstyled raw text but there isn't.

Comment 83

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

They have VS16 but I will double check them again.

Comment 84

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Pavel Ivanov [:ivanovpavel][:pivanov] UX from comment #83)
> They have VS16 but I will double check them again.

It would be easier to check if you use escape sequences.

Comment 85

[Reza Akhavan [:jedireza]]

Comment on attachment 8643924 [details] [review]
[gaia] jedireza:emoji-keyboard > mozilla-b2g:master

Tim, Pavel added a patch for the VS16. We're ready for another review.

Also recently rebased and tests are all clear:
https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=0ad16fc3da360aa8dff2348c198ba7a4fdbe660f

Comment 86

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8643924 [details] [review]
[gaia] jedireza:emoji-keyboard > mozilla-b2g:master

I did not load the new font and the patch into the phone. Instead I look at 

https://mozilla.github.io/fxemoji/dist/FirefoxEmoji/index.html

and do spot check if there are symbols missing (on things I have noted on the last review).

1) I don't see U+2122 (the TM sign) in the page.
https://github.com/mozilla-b2g/gaia/pull/31263#discussion_r42580168
2) I am seeing U+1F35 here, which should have been U+1F35E (the "bread" Emoji) on the page. The page indicates it is being encoded in the wrong place too.
https://github.com/mozilla-b2g/gaia/pull/31263#discussion_r44501269
3) According to http://unicode.org/Public/UCD/latest/ucd/StandardizedVariants.html , some of the non-BMP chars need VS16 too because they default to text style
https://github.com/mozilla-b2g/gaia/pull/31263/files#r44502077
https://github.com/mozilla-b2g/gaia/pull/31263/files#r44501970
https://github.com/mozilla-b2g/gaia/pull/31263/files#r44501992
https://github.com/mozilla-b2g/gaia/pull/31263/files#r44502008
https://github.com/mozilla-b2g/gaia/pull/31263/files#r44502033

Set this to r+ once both changes are corrected.

(1) can be corrected by removing TM sign from the Emoji layout for now or actually update the font.
(2) needs to update the font *and* update the layout.
(3) needs to update the layout even though it might not be visible on the browser.

It might be worthy to check 

http://www.unicode.org/Public/emoji/1.0//emoji-data.txt

and ensure anything default to text style are appended with VS16, but I think adding VS16 to BMP code points & (3) should be enough.

(I did not, for example, say U+1F441 ("EYE") should followed with a VS16, even though it is marked as text-default in the data chart. We might want to follow that if we want to.)

(The exact guideline is illustrated here and explained by the accompany paragraph http://unicode.org/reports/tr51/index.html#Emoji_vs_Text_Display )


I am sorry this took longer than we think it would be, but it is important the layout output the right codepoints to ensure interoperability across different devices.

Comment 87

[Reza Akhavan [:jedireza]]

Comment on attachment 8643924 [details] [review]
[gaia] jedireza:emoji-keyboard > mozilla-b2g:master

Tim, Pavel added a new commit to the PR that I assume addresses these issue. No worries on it taking longer than expected, getting it done right is important.

Pavel, if you think the changes address Tim's feedback (above), let's mark this as review? again.

Comment 88

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

Yeah it will be cool if Tim can take a look at these changes again.
Tim can you check it again please :)

Comment 89

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Tim Guan-tin Chien [:timdream] (OOO Nov 16-26; please needinfo) from comment #86)
> 3) According to
> http://unicode.org/Public/UCD/latest/ucd/StandardizedVariants.html , some of
> the non-BMP chars need VS16 too because they default to text style
> https://github.com/mozilla-b2g/gaia/pull/31263/files#r44502077
> https://github.com/mozilla-b2g/gaia/pull/31263/files#r44501970
> https://github.com/mozilla-b2g/gaia/pull/31263/files#r44501992
> https://github.com/mozilla-b2g/gaia/pull/31263/files#r44502008
> https://github.com/mozilla-b2g/gaia/pull/31263/files#r44502033
> 

Not every icons flagged by these comments need a VS16, but according to the spec [1] there seems to be no harm marking *everything* with VS16 anyway so we should land this.

[1]  http://unicode.org/reports/tr51/index.html#Emoji_vs_Text_Display

Comment 90

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

Thanks Tim :)

Reza: It's your turn :)
Thanks for the help to all of you :)

Comment 91

[Reza Akhavan [:jedireza]]

Woohoo! On master:
https://github.com/mozilla-b2g/gaia/commit/e8c15ae4e5324a210000ee0a869a962aa542009f

Comment 92

[Kevin Grandon :kgrandon]

Please try to squash work like this in the future to a single commit (or fewer) if possible. Commits should be atomic in nature, so a commit like `updated build integration tests` is not ideal, and should be squashed into the commit which changed the functionality of the tests. You can squash future work with `git rebase`. This is part of the requirements to land code into gaia.

This feature is awesome though, thanks for working on this!

Comment 93

[Reza Akhavan [:jedireza]]

(In reply to Kevin Grandon :kgrandon from comment #92)
> Please try to squash work like this in the future to a single commit

Yes, we discussed and compromised on this since there we a mixed set of commits from Pavel and myself. Prefixing the commits with the bug number was the middle ground. Now after looking back at them, I could've done a better job at grouping them to reduce the number of commits.

> This feature is awesome though, thanks for working on this!

;) thank you