After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 563841 - Give smileys a facelift!
Give smileys a facelift!
Status: RESOLVED FIXED
Product: GtkHtml
Classification: Other
Component: Editing
3.25.x
Other Linux
: Normal enhancement
: ---
Assigned To: Srinivasa Ragavan
Srinivasa Ragavan
evolution[composer]
Depends on:
Blocks:
 
 
Reported: 2008-12-09 12:34 UTC by Matthew Barnes
Modified: 2008-12-09 18:49 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Screenshot: Emoticon Menu (69.57 KB, image/png)
2008-12-09 12:36 UTC, Matthew Barnes
  Details
Screenshot: Emoticon Button (50.88 KB, image/png)
2008-12-09 12:36 UTC, Matthew Barnes
  Details
Proposed patch (65.71 KB, patch)
2008-12-09 12:41 UTC, Matthew Barnes
committed Details | Review

Description Matthew Barnes 2008-12-09 12:34:13 UTC
Emoticons are very important in written communication because they help you share your feelings and stuff, especially for those who write HTML mails.  Er, yeah.  Anyway, I gave our emoticon support an upgrade.  Check it:

  - Add a toolbar button for inserting emoticons.

  - Use only standard icons name, as defined by the icon naming specification.

  - Use ALL available face icons defined by the icon naming specification.

  - Recognize more icons for our "magic smileys" feature.


That last one was a pain, let me tell ya.  Even had to use a spreadsheet to get the parse table right...
Comment 1 Matthew Barnes 2008-12-09 12:36:15 UTC
Created attachment 124261 [details]
Screenshot: Emoticon Menu

Also notice the new button on the HTML toolbar.
Comment 2 Matthew Barnes 2008-12-09 12:36:49 UTC
Created attachment 124262 [details]
Screenshot: Emoticon Button

Toolbar button, expanded.
Comment 3 Matthew Barnes 2008-12-09 12:41:51 UTC
Created attachment 124263 [details] [review]
Proposed patch

Created a GtkhtmlFaceAction class (based on GtkRecentAction) for dealing with emoticons.  All available emoticons are listed in one easy-to-read table in gtkhtml-face-chooser.c.
Comment 4 Matthew Barnes 2008-12-09 12:43:32 UTC
Note, for testing purposes you can just run the GtkHtml test editor program:

   gtkhtml/components/editor/gtkhtml-editor-test
Comment 5 Milan Crha 2008-12-09 16:37:53 UTC
I didn't read code fully, just briefly gone through. Two things:
a) it's pity we cannot use 
+static GtkhtmlFace available_faces[] = {
also in the
 static gchar *picto_icon_names [] = {
(that's nothing I would want from you to change, just a sigh.)

b) this line is obsolete now, isn't it?
+	g_object_set (action, "icon-name", "face-smile", NULL);

c) compiler claims:
gtkhtml-face-action.c: In function ‘face_action_create_tool_item’:
gtkhtml-face-action.c:128: warning: passing argument 1 of ‘gtk_widget_show’ from incompatible pointer type

d) One thing, you added face-button to HTML toolbar, but the menu itself is available even for a plaintext mode. Maybe moving to the generic panel?

Feel free to fix whatever you'll think is really wrong from the above things and commit to trunk.
Comment 6 Matthew Barnes 2008-12-09 18:00:15 UTC
(In reply to comment #5)

a) Agreed.  It'll be "interesting" to port that face parser thing to WebKit.

b) No, it sets the menu and tool item icon.  Unfortunately gtk_action_new()
   only takes a stock-id argument, so you have to set the icon-name property
   explicitly to use a themed icon.  I'm hoping GTK+ will someday drop the
   distinction; it's just awkward and confusing.

c) Drat.  Will fix before committing.

d) Neither the face button nor the menu make much sense in plaintext mode.
   User can type :-) just as easily as he can reach for a menu or toolbar.
   I don't know, I'll have to think on that some more.  Pretty sure I want
   to leave the button in the HTML bar for now.
Comment 7 Matthew Barnes 2008-12-09 18:30:56 UTC
Committed to trunk (revision 9060).
Comment 8 Milan Crha 2008-12-09 18:49:31 UTC
(In reply to comment #6)
> b) No, it sets the menu and tool item icon.  Unfortunately gtk_action_new()
>    only takes a stock-id argument, so you have to set the icon-name property
>    explicitly to use a themed icon.  I'm hoping GTK+ will someday drop the
>    distinction; it's just awkward and confusing.

Oh, right, I misread the code, I'm sorry.

> d) Neither the face button nor the menu make much sense in plaintext mode.
>    User can type :-) just as easily as he can reach for a menu or toolbar.
>    I don't know, I'll have to think on that some more.  Pretty sure I want
>    to leave the button in the HTML bar for now.

There is one difference between typing and choosing from the menu, when switching back to HTML mode. Nothing serious, feel free to keep it in an HTML toolbar.