On Sat, 2004-02-07 at 10:48, Joergen Scheibengruber wrote: > That sounds like a good idea :-) The maybe is probably not necessary, I > think having it default to "if available" is fine. Ok - actually, having thought about this a bit more, let's just go with "No" and "Yes". The UI resizing caused by "if available" would be pretty ugly. > Regarding the icon: imho it's ok, but if someone comes up with a nice > "no cover available", this would of course be better :-) I think we'll really need to do that - I'd like for clicking on the "no cover available" icon to download an album cover. > Ok, I have created my own branch now :-) It was really easy (with the > instructions on the rb-site). Great! > I have changed quite a bit again: > - I have created a own widget RBAlbumIcon in widgets/rb-album-icon.[c|h] > that contains most of the functionality > - I use gnome-vfs-async calls instead of normal ones for loading the > image (like hadess recommended) Ok, very good work in general. A few comments: if (album_icon->priv->uri) g_free(album_icon->priv->uri); g_free already tests if it's NULL, so you can just say: g_free (album_icon->priv->uri); Next, you need to hold the GDK lock in all gnome-vfs async callbacks. Basically just run GDK_THREADS_ENTER () when the function starts, and GDK_THREADS_LEAVE () before it returns (be sure to handle all return paths!). Regarding style, you have spaces between the function name and call in some places, but not others. Also, the indentation around the function headers is...odd: http://web.verbum.org/files/rb-indentation.png I'm guessing your tab width is set to 4 or so? It should be 8. I also get this assertion on startup: (rhythmbox:5969): Gtk-CRITICAL **: file gtkwidget.c: line 1911 (gtk_widget_hide_all): assertion `GTK_IS_WIDGET (widget)' failed > - There is now a "tooltip" for the cover_icon, that shows a 200x200 > version of the cover ( > http://www.wh-hms.uni-ulm.de/~mfcn/shared/rhythmbox-album_icon-tooltip.png) I'm uneasy about doing this in a mouseover - it's a cool hack, but it's a little intrusive, especially because it's near the playback control buttons. Maybe changing the timeout to a second would be enough. Or we could just make clicking on it open it in an image viewer. Also, you need to hold the GDK lock in those idle handlers too.
This is a digitally signed message part
