GNOME Bugzilla – Bug 125226
A non expanded panel without hide buttons violates the Fitts law
Last modified: 2020-11-07 12:14:13 UTC
With gnome 2.4 when i disable the arrow buttons of a not expanded panel, i foud two handle are. The handle area are useful in a panel placed in the center (symmetric), but when the panel is placed in a corner, the handle waste the precious corner pixel. For example, i've created a non expanded panel and i've positioned in the lower right corner with the "windows menu applet" as the rightmost applet. Without the right handle, i could select the windows menu applet with an easy motion (the Fitts law about the five pixel locations on the screen that the user can access fastest...). I think that a behaviour in wich the handle closest to the corner disappear (could be sufficent to permit the panel to move 5/8 pixel outside the screen display) could be an improvement of the accessibility/usability of the icon/applet of the gnome panel Andrea
Why is the accessibility keyword set for this bug?
Sorry, at time i wasn't sure what category should have filed it. I'm not even sure it's an usability bug. I'm changing right now.
It's not really hard to do this: it's just a matter of looking at the position of the panel before drawing the handles. The code is in panel-toplevel.c. This file is very long, but I can guide someone who wants to fix this.
I'm still interested. =) If i can have some help i would like to give a stab at this, so Vincent, probably i should start grabbing the new 2.10 panel sources, right? Andrea
That's it. I'll be away for a week, so if you want to ask me something, e-mail is probably the best way to join me.
Created attachment 39576 [details] [review] Checks panel position and do allocation This patch checks the toplevel panel location, and depending on where it's located allocate widget space depending on that. This trick only make allocation depending on position, but panel still try to drap expose event. Actually that's a unnecessary resouce, if you approve this, I'll patch the expose function so it won't try to draw the handle.
Comment on attachment 39576 [details] [review] Checks panel position and do allocation Hi Baris, Thanks for your work! Some generic comments: + there are some space changes in this patch (at the end). They're not useful :-) + you should do the diff with the -p argument as it makes it easier to know what function you changed (add 'diff -up' to ~/.cvsrc) + indeed, we don't want to draw the handle Some comments about the code: > if (toplevel->priv->orientation & PANEL_HORIZONTAL_MASK) { >- challoc.x = HANDLE_SIZE; >- challoc.y = 0; >- challoc.width = allocation->width - 2 * HANDLE_SIZE; >- challoc.height = allocation->height; You can keep challoc.y = 0; challoc.height = allocation->height; here since it's always this (in the horizontal case). >+ if (panel_x < SNAP_TOLERANCE && !x_centered) { >+ challoc.x = 0; >+ challoc.y = 0; >+ challoc.width = allocation->width - 2 * HANDLE_SIZE; Shouldn't it be allocation->width -HANDLE_SIZE? >+ challoc.height = allocation->height; >+ >+ } else if (panel_x > screen_x - panel_width - SNAP_TOLERANCE && !x_centered) { >+ challoc.x = HANDLE_SIZE; >+ challoc.y = 0; >+ challoc.width = allocation->width - 0 * HANDLE_SIZE; Shouldn't it be allocation->width -HANDLE_SIZE? >+ challoc.height = allocation->height; >+ } else { >+ challoc.x = HANDLE_SIZE; >+ challoc.y = 0; >+ challoc.width = allocation->width - 2 * HANDLE_SIZE; >+ challoc.height = allocation->height; >+ } > } else { Same comments here.
Created attachment 39585 [details] Fixed previous patch You were absolutely right on your comments, sorry about that. I have no idea why I kept - 2 * HANDLE_SIZE. I thought i did that on purpose, but it was needlessly decreasing allocation size. I put control to expose function so that it won't try to draw handle widget. Indeed it's not showable to user, can you please check if controls right? Spaces fixed as well. Actually maybe the best way to make this to add new gboolean properties to toplevel panel struct, like on edge_on_bottom, edge_on_top, edge_on_left etc. Maybe later, but can't see any other use of those properties apart from this one.
Comment on attachment 39585 [details] Fixed previous patch I attached whole file, damnit.
Created attachment 39586 [details] [review] Here's the patch Comments on #8.
Created attachment 39590 [details] [review] Reworked patch for HEAD I reworked a bit the patch for HEAD. We also need to change panel_toplevel_update_size() to not always request 2 * HANDLE_SIZE... I'll stop working on this for now since bug #107622 will probably require lots of change to panel-toplevel.c. I hope it will also simplify the code... Baris: you can of course continue to work on the patch :-) Thanks
Well, I'm not quite sure what else should be done to current patch, it does what it should do already. Do you think it needs more work? What else should I change/control for such a thing?
panel_toplevel_update_size() needs to be updated. Right now, we won't draw the handle, but the panel is too large because we're requesting space for both handles too.
Created attachment 39610 [details] [review] Using PanelFrameEdges enum to check edges I should have done that before, instead of lots of garbage, but after some code inspection I realized that there's already a call (panel_toplevel_update_edges) that updates panel edge status and set PanelFrameEdges enum according to that. I reworded on patch and used that instead of own calculation of panel position. Also updated panel_toplevel_update_size() call to set width and height correctly. I tested and seems working.
Baris: I'll try to review your latest patch soon! Thanks
*** Bug 352000 has been marked as a duplicate of this bug. ***
This is still there in 2.19.5; i think Boris' patch should work and the idea is good, but needs some cleanup and to be reworked, as it does not apply anymore to 2.19.5. If there is still interest (I hope so) and someone is willing to apply it in a reasonable time (not 2 years :P) I volunteer to do the rework and cleanup job (or maybe Boris itself is willing to!).
I'm still interested in this, yes.
Created attachment 92604 [details] [review] proposed patch This patch applies to HEAD. Also, I made some corrections to previous patch, to make the trick work guessing the position of the panel using edges. Now it works fine on my box.
Vincent?
Marking Baris' patch as obsolete, as anyway mine is based on his.
bugzilla.gnome.org is being replaced by gitlab.gnome.org. We are closing all old feature requests in Bugzilla which have not seen updates for many years. If you still use gnome-panel and if you are still requesting this feature in a currently supported version of GNOME (currently that would be 3.38), then please feel free to report it at https://gitlab.gnome.org/GNOME/gnome-panel/-/issues/ Thank you for reporting this issue and we are sorry it could not be implemented.