GNOME Bugzilla – Bug 112730
some kind of zoom control for the toolbar
Last modified: 2004-12-22 21:47:04 UTC
It would rock to have some sort of zoom control for the toolbar for me, a toggle button toggling between 'zoomed' and 'normal' would work just perfect. I don't need n different sizes to chose from..
Hmm, well there is a zoom widget in gnumeric we could use, and I have the feeling that simple option menu with some predetermined sizes would work just fine too maybe: 50% 75% 100% 150% 200% 300% 400%
I have already implemented a zoom control. > Hmm, well there is a zoom widget in gnumeric we could use, and I have > the feeling that simple option menu with some predetermined sizes > would work just fine too maybe: Coincidentally, it _is_ an option menu with some predefined values that I implemented; but I architectured everything with the goal that having a different kind of control would be easy. So when there is a gtk or gnome stock zoom control, we could easily switch to it. (See also the discussion in bug 77952). I'm going to attach a patch for this, so you can tell me what you think of it, both UI- and implementation-wise. Please bear in mind that I do _not_ think this is ready for inclusion in ephy. In addition to the patch (apply with -p1) you need to copy some icon to data/art/epiphany-zoom-control.png, and do a make install.
Created attachment 16425 [details] [review] a zoom control for the toolbar (work-in-progress)
Some explanations about my patch: - the api (ephy-zoom-action.h) right now is prepared for a much more sophisticated zoom control, i.e. it has zoom/in/out/default actions. If we're going with the simple option menu for the time being, these can be removed. - ephy-zoom-control.h has some superflous functions too, I'll remove them. - ephy-zoom.[ch] has: * support functions to get the next-lower/higher predefined zoom level: these are needed for the zoom in/out both in ephy and ephy-naut-view. * a way to get an array of zoom levels and names (needed for ephy-nautilus-view). The zoom levels were previously declared in ephy-naut-view, but are now needed both there and in ephy-zoom-action for the zoom control, so it seemed ok to move them to a common header file. - about the predefined zoom levels themselves: I just defined a set I like :-) but I we should think about them a little more.
Created attachment 16468 [details] [review] updated patch (work-in-progress)
Changes since the last version of my patch: - simplified the zoom-control and zoom-action api, removed unused signals/callbacks - made ephy-zoom-control an EggToolItem - implemented toolbar overflow menu for zoom control
If you are using my idea for the zoom control (ie just a simple option menu), then in the overflow menu it might be nice to make the entry a submenu with all the available zoom levels as the submenu items. (notice: haven't tested the patch.)
Created attachment 16500 [details] [review] updated patch (work-in-progress)
Changes since last version of the patch: - further api cleanup in ephy-zoom, ephy-zoom-action and ephy-zoom-control. - ephy-zoom-control now follows toolbar style (text only/icon only/text+icon) - only place zoom control on toolbar, not zoom in/out/default too - get rid of stock icon for zoom control
Getting really nice. A few things: - I think it's worth to expose a set_level api in the control. The property is good, but we should have also a func to set it ihmo. - Why did you choose these zoom levels ? ;) zoom_levels[] = +{ + { "71%", 0.7071067811 }, + { "84%", 0.8408964152 }, + { "100%", 1.0 }, + { "119%", 1.1892071149 }, + { "141%", 1.4142135623 }, + { "168%", 1.6817928304 }, + { "200%", 2.0 } +}; - Maybe we could merge set and change zoom ? with ZOOM_TO_VALUE flag ? I just dont like the change name much ... it's not very clear what is the diff from set. - We could put the zoom action in lib/widgets maybe, it's nicely abstract I forgot something I was going to say ... bah will remember at some point. Keep up the good work.
Created attachment 16556 [details] [review] updated patch (work-in-progress)
Changes since the last version of the patch: - ephy-zoom-control and ephy-zoom-action now have getters/setters for the "zoom" object property - move ephy-zoom-action to lib/widgets/ - merged ephy_window_change_zoom back into ephy_window_set_zoom All that's left is getting user feedback, I think. Can someone actually try this patch and give me their opinion about the UI ? Thanks :-)
Ui nazi, would be helpfull if you could try this and review ui. So tomorrow I'll be able to review the code.
Created attachment 16610 [details] [review] updated patch (to apply cleanly, no changes)
Overall i like it. My one comment is that I think we should drop the label for a few reaons. 1. Most importantly its potentially confusing to users. For all other toolbar items (that have a label) clicking the label activates the button (this is why i favor icons+text, you have a bigger clicking target). However in this case clicking the label does nothing and probably shouldn't do anything since that would probably be even more confusing to users who've seen this widget in other parts of the ui. 2. The label uses space needlessly when using prioirity text or text only. as for the icon, probably our best bet for the time being is to glade+gimp an icon. Though chpe does have some interesting ideas on this issue.
Created attachment 16634 [details] [review] updated patch: removed label
Created attachment 16642 [details] [review] updated patch, incorporates review comments
checked in. Very nice work. Thanks !