[Aptitude-devel] Changesets 00288a0dcae2 through 4710b5c94fcc

[Daniel Burrows]

On Thu, Dec 04, 2008 at 06:19:54AM +0100, Obey Arthur Liu <arthur at milliways.fr> was heard to say:
> Daniel Burrows a écrit :
> >   I'm back from vacation, and I've been trying to catch up on the work
> > that went on while I was gone.
> 
> Hope you had a good vacation  :)

  Yes, I did, thank you.

> >   Right now we have several almost-redundant ways of doing the same
> > thing.  The context menu and Package menu are identical to each other,
> > and the big fat buttons in the description area do a subtly different
> > thing (when several packages are selected, the menus act on all of them
> > but the buttons only act on the one whose description is displayed).
> > Leaving technical issues aside, is this too much redundancy in the UI,
> > or is shoving the actions in the user's face more important?
> 
> I think that what is displayed right now is ok, as far as "shoving in
> the face goes"

  BTW, in case it wasn't obvious, I was being a little flip there -- as
you know, I put the buttons in there, and it's because I think there's
actual utility in shoving actions in the user's face sometimes. :-)

> >   Would it perhaps be better to have a second "package" toolbar?  I
> > think I decided against this because I felt like it would inevitably be
> > too cryptic: the great thing about the buttons is that they say exactly
> > what they do.
> 
> We don't have much screen real estate left anyway.

  Another good reason not to do that.

> > ====
> > +  void EntityTreeView::on_cursor_changed()
> > +  {
> > +    // TODO: we might plan to do some more elaborate filtering
> > +    //       based on which tab is being active. If not, we may
> > +    //       as well revert to a simple signal instead of this
> > +    //       virtual function.
> > +    signal_selection_change();
> > +  }
> > +
> > ====
> >
> >   This seems like a completely unused and undocumented function, maybe
> > left over from some intermediate version of the source tree.  Can it
> > just be pruned?
> 
> I mentionned it somewhere else but that's because only detecting cursor
> changes is not enough to reliably detect where the "interface focus" is.
> It's probably a bad idea to put code about that in a function called
> on_cursor_changed(). I should stick with the signal and put the relevant
> code someplace else.

  Whatever the function is, my point was that it's not referred to
anywhere.  I figured it was left over, but it sounds like it's actually
meant to slot into a design outline that you have in mind?

> >   The Package menu still doesn't apply to the currently active tab.  I
> > think that in order to handle this, we'll have to allow tabs to know
> > whether they're currently selected.  I'll implement a flag on each Tab,
> > along with signals when a tab is selected or deselected (with a
> > guarantee that deselection is sent before selection).
> 
> Useful.

  As I noted in my other email, I ended up implementing some specialized
code in the tab manager to handle the Package menu.  But the signal and
the flag are still in there anyway; I can imagine that they might be
useful for other stuff that doesn't need the coordination.

> >   I also noticed a lot of this:
> >
> > (aptitude:7476): glibmm-CRITICAL **: Glib::ObjectBase*
> Glib::wrap_create_new_wrapper(GObject*): assertion `wrap_func_table !=
> 0' failed
> >
> > (aptitude:7476): glibmm-WARNING **: Failed to wrap object of type
> 'GtkAccelLabel'. Hint: this error is commonly caused by failing to call
> a library init() function.
> >
> > (aptitude:7476): Gtk-CRITICAL **: gtk_bin_remove: assertion
> `GTK_IS_WIDGET (child)' failed
> >
> >   I'm not sure where it's coming from, but it should be tracked down
> fixed.

  Removing the Package menu from the Glade file made this go away.  I
still don't know why.

> The point here is that I want most actions that can be done with the
> mouse to be done with the keyboard. At least, enough to have a decent
> keyboard-only experience that's not too far from the ncurses one. GTK+
> is not very good at that but I think it's doable.
> There's also this GUI design rule that says that all the actions should
> also be available through the menus (ok, openoffice and office pre-2007
> are bad design examples). GTK+ seems to enforce some of this by only
> allowing accelerators (keyboard shortcuts) to be simply defined with
> menu items. On another hand, that's a good idea because the user knows
> to look for shortcuts by in the menus. It's what the ncurses interface
> does (although there are also some "secret" shortcuts, in the sense that
> they don't appear in the menus but only in the documentation).

  That makes sense.  BTW, I think GTK+ only allows keyboard shortcuts
to be *defined* with menu items because otherwise you just bind to the
key event hook and do whatever you want.  Only menu items need special
treatment.

> The pop-up context menu code creates menus on-the-fly about the selected
> packages. It's quite easy to know which packages the user is thinking
> about in this case: it's those right under the mouse.
> It gets more complicated with the menubar "package" menu. We don't
> really know which selection the user is thinking about when he goes to
> the package menu and the menu has to be created *before* he tries to
> click (or finally doesn't) on the menu. That's why this menu gets
> recreated all the time. It's not very pleasant but I haven't figured out
> how this can be done on-the-fly. Gtk::Menu and Gtk::MenuShell don't seem
> to offer ways to do this. Maybe I should have a look at how the others
> do it.

  I got that part. :-)  I don't see any problem with continually
recreating the menu as long as it works (and it seems to be fine as
long as we don't destroy a menu that Glade created ... not that I
understand why that matters).

  One difficulty I noticed while working on this is that while the
behavior of the Package menu for the packages tab is obvious (and
probably also for the dashboard tab and perhaps the info tab), it's
not clear what should happen in the dependency chains tab.  I suspect
that it might just be too confusing to have the menu activated on that
tab; users who want to change package states can right-click or use
keyboard shortcuts.

  Daniel