Re: Say goodbye to those ugly ass add-to-panel submenu blues

[Mark McLoughlin <mark skynet ie>]

Hi Iain,
	I'm really glad you've done this ... I've been putting it off because I
thought it was going to suck without categorisation :-)

	Some comments from playing with it:

  + Adding an applet to the panel makes it be added at the start of
    the panel instead of where you clicked on the panel. Was that
    change intentional ?

  + Drag and Drop needs to be implemented.

  + When adding a launcher, destroying the dialog and bringing up
    a very similar dialog is a bit to distracting. It would be nice
    if you could re-use the same dialog to minimize the effect.

    Another problem is that because it *looks* like the same dialog
    it feels like there should be a "back" button.

  + The launcher list is definitely too long, and we do have somewhat
    sensible categorisation there ... so I'd use the categorisation.

  + I've heaps of icon-less launchers on my system and the dialog
    looks terrible because of it. Maybe group the icon-less lauchers
    at the bottom of the category .. or even don't show them at all?

  + When bringing up the launcher selection dialog, it spews warnings
    for me about using "&". You musn't be escaping the text somewhere.

  + You might want to try using the panel name in the dialog:

      "Add an item to Bottom Panel"

    The whole panel name thing kind of sucks, but at least on the
    default panel setup it would give you an indication of where
    your applet is going to be added.

  + Is there anyway we can wrap the descriptions at least so they
    don't require you to scroll horizontally?

  + Ctrl-F treeview searching doesn't work with the dialog. Works fine
    for the launchers dialog, though.

  + The mnemonic for the list isn't working.

  + The "Add to panel" button in the dialog could do with the + icon
    and and mnemonic. It should also be the default button more than
    likely.


	Some comments on the patch itself:

  + You should remove the categories altogether from the code now that
    we're not using them - i.e. from AppletMenuInfo and from the 
    bonobo-activation query.

  + I don't like the way it stores the drag_data string in the list
    store and parsing it later. I'd prefer something us to store a boxed
    structure with an enum and union for type specific data. We could
    also generate the drag data from that too.

  + Use StudlyCaps for structure names.

  + Try a bit of error handling in make_pixbuf().

  + Use a for loop for iterating lists for consistency with the rest
    of the panel.

  + Make the duff Help buttons do nothing instead of closing the dialog
    for now.

  + You removed the code to make the "Add to Panel" menuitem insensitive
    when the applet_id_list and object_id_list keys are readonly. We
    still need that.

    Related to this, we shouldn't have any applets in the dialog when
    applet_id_list is readonly and vice versa.

  + The default dialog size is going to to big on low resolutions. Make
    it a function of the screen size.

  + You need to do gtk_window_set_screen () on the dialogs before you
    show them so it works on multihead. You might also want to think
    about the position of the dialog on Xinerama.

  + I'm not so sure adding icons in an idle handler is really necessary
    here. I'd leave it out until anyone complains.

  + Do you know what ? Move this into a separate file. menu.c is too
    big and this isn't a menu.


	Anyway, this is definitely on track and it should be in for 2.6. 
	Great stuff !

Thanks,
Mark.