GNOME Bugzilla – Bug 113859
Nautilus should use gnome icon theme for the spinner animation
Last modified: 2004-12-22 21:47:04 UTC
I'm going to attach a patch that loads the spinner frames from a single png (with a grid of images) and use gnome_icon_theme to retrieve the image path. I'll post here also the gnome-icon-theme patch because I dont see a module for it in bugzilla.
Created attachment 16885 [details] [review] Nautilus throbber patch
Created attachment 16886 [details] [review] gnome-icon-theme patch
Created attachment 16887 [details] Png that should go in gnome-icon-theme/36x36/apps
Created attachment 16888 [details] [review] Makefile.am should go in gnome-icon-theme/36x36/apps
Created attachment 16902 [details] [review] Updated nautilus patch. (Fix a bug on size_request and cleanup some unnecessary changes)
I think we should handle the first image differently, like the "throbber/rest" icon was handled before. You might not always want to include the no-activity image in the set of throb-icons. + icon = gnome_icon_theme_lookup_icon (throbber->details->icon_theme, + "gnome-spinner", -1, NULL, &size); + g_return_if_fail (icon != NULL); Can't use g_return_if_fail here. We must be able to handle no icon ok, and return_if_fail can be compiled out if you're disabling debug. Just make it g_warn and return. + } + else { + element = g_list_nth (throbber->details->image_list, throbber->details->current_frame); and + if (throbber->details->current_frame > throbber->details->max_frame - 1) + { Are not in the nautilus coding style. See docs/style-guide.html.
Created attachment 16926 [details] [review] Nautilus patch
Created attachment 16927 [details] [review] Gnome icon theme patch
Alex, thanks for the quick review. - I got back quiescent_pixbuf and added a gnome-spinner-rest icon to fill it. That seem to be cleaner than hardcoding a position in the grid to be rest. - I used g_warning instead of g_return_if_fail - The code with styles problems is no more in the patch (because of quiescient_pixbuf changes)
Created attachment 16936 [details] [review] Yeah another nautilus patch update
Changes: - Do not copy the image when there is no need to scale it, it's enough to ref it, in fact subpixbuf keep a ref on the grid pixbuf. - Do not go through all the list of animation images when calculating dimensions, in the grid they are forced to be all the same size
scale_to_real_size() has a coding style issue: if (throbber->details->small_mode) { result = gdk_pixbuf_scale_simple (pixbuf, size * 2 / 3, size * 2 / 3, GDK_INTERP_BILINEAR); } else { In nautilus_throbber_load_images(): for (y = 0; y < grid_height; y += size) { for (x = 0; x < grid_width ; x += size) { pixbuf = extract_frame (throbber, icon_pixbuf, x, y, size); image_list = g_list_prepend (image_list, pixbuf); } You have to handle extract_frame returning NULL. That may happen if e.g. the grid size is not a multiple of the icon size. I didn't actually test the code, I just assume it works well. If you don't know of any problems with it it should be fine to commit once the comments above are fixed.
Checked in. I tested it quite well, but if there are problems let me know and I'll fix them.