Hi Chris,

        1) The StatusBarHandler class is a great idea.  It should be a
                standalone class, either BackupWindow.h or its own header.

                Also I notice that StatusBarHandler gets rid of the call
                to m_pStatusBar->show_now()... any thoughts on that?
Yeah, I moved it to BackupWindow.h. First thought was to util.h, but found that it could only be used in BackupWindow. I saw that show_now() was not used in some situations, and without it everything worked fine. Now it's back. :-)
        2) All Barry functionality was wrapped in the DeviceInterface
                class.  As such, m_pProbe should not be in BackupWindow,
                and BackupWindow shouldn't have to do 'new Probe', nor
                worry about its memory handling.
I added some member functions to DeviceIface as a substitute. Looks much better.
        3) The drop down list should say "No device selected" or
                "Please select device" if there are multiple devices
                available.  It shouldn't just be empty.  Also, if there
                is only one device available, it should connect to it
                right away.  If you want to make this configurable, that's
                ok, (default to the old behaviour) but for those who use
                the GUI a lot, that extra series of clicks just to select
                their one and only device will get tiring.
I cannot find a good way to display the message in the ComboBox, hence I put them in the Label. I wonder if there's any way of displaying messages on a ComboBox when no item is selected? If so, I think the caption of "(n devices)" would be great.
        4) I don't like the new packed layout. This is a show stopper for me.
                Everything is packed up tight against the edge of the
                window, and looks bad.  I've attached a screenshot of what
                the default looks like on my system.
It looked fine under my theme, so I haven't noticed this problem. Now some padding and spacing are added, it should be better, at least under the themes I have tested.
On the "future ideas" front (which might be future patches if you like),
it might be a good idea for Scan() to clear any existing table entries, so
that it can be called repeatedly to refresh the device list.  Perhaps on
a timer or a button.
Implemented. :-)
Also, the better way of handling CheckWorkingDevice() is to enable and
disable buttons and menu items based on what is possible given the current
state.  If no device is selected, the Backup button, for example, should
be greyed out.  The check you have now is fine, this would just be nice
to have.

Applied, but I kept the CheckWorkingDevice() calls to prevent unpredictable events.

Best Regards,
Ryan

Attachment: patches.tar.gz
Description: GNU Zip compressed data

------------------------------------------------------------------------------
Enter the BlackBerry Developer Challenge  
This is your chance to win up to $100,000 in prizes! For a limited time, 
vendors submitting new applications to BlackBerry App World(TM) will have 
the opportunity to enter the BlackBerry Developer Challenge. See full prize 
details at: http://p.sf.net/sfu/blackberry
_______________________________________________
Barry-devel mailing list
Barry-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/barry-devel

Reply via email to