| Re: [udig-devel] Quick notes from review of database connection patch |
Hey Jody, Thanks for your review. My responses to your comments are inline below. I do stress though that your comments are mostly bugs whereas we first need to nail down if the overall design makes sense. I'm willing to work on the bugs once we get your blessing that this design for event handling and sharing makes sense. On Thu, 2007-06-14 at 16:03 -0700, Jody Garnett wrote: > Hi ACuster :-) > > Here are my notes; > > - Host - should list old entries? no, that's the job of "recent" One of the reasons to fix the dialog was to prevent uDig auto-magically knowing/filling out values---which ended up broken because the inheritance system was not well thought out. This bug stopped Andrea from using uDig which is a shame right when he was working on the transactional WFS. > - lookup button springs into existence? can we disable it when not > applicable so it is not a "surpise" It should not. It is present but disabled until sufficient values are filled out that it might be able to make a lookup connection at which point it is activated. See screenshot #1 below. > - Hitting tab should select all of the text when moving between feilds > (when going from host, to port, to password) yes, this is a TODO item in the code. For now this is not enabled. > - tab order not straight down the page, password -> lookup -> schema indeed, this is complicated. One of the TODO items is to look into this and decide if it is worth adding all the code simply to make the tab order more intuitive. Currently the code is the same as the old system. > - Lookup populates the "Database" list ... Not sure what you are trying to say here. I sure hope it does since that's the point of lookup. > - Advanced should be a > V arrow What is the widget name you have in mind? Changing the widget is easy. Here I am merely following the old dialog. > - connect button hates me! Killed all my settings? And then lookup no > longer works. Yes, this is bad UI. At that point, the "finish" button should be active. Perhaps, I should 'disable' all the dialog input at this point. The problem, documented in the code iirc, is that we need to reset the dialog prior to the next use and there's no way to hook into the "finish" button since that is created by eclipse and not by us. Suggestions are welcome for alternative ways to reset the dialog. > - Next works nice :-) Displays the list of layers with nice decorators ? not sure what db you are using. > > On the second run (to connect to a different database): > - Recent appears as new field on at the top? This was not here the first > time, since we have only one entry we are stuck, if we had an "" entry > we could clear out the fields to start from scratch ... and we must > select something. no. You don't need to select anything in the Recent menu, you can just fill out the values. At least that's the intent; you may be right that this is not working correctly at present. > Given that Recent has a selection I am not sure if we > are modifying or doing something new. Ah, that's interesting. I'll have to think about how to communicate what is happening. > - Changing the settings and hitting next, with an invalid connection, > produces and error, but then shows list of layers from the first time, > interesting! ?The connection has been made using the previous values? We were supposed to reset these. I'd need more detailed instructions to assess what you are doing and why the dialog is not working. > - if the connect button fails we should disable the next button, wonder > if we could make this obvious, ask users to press the connect button > before Next is an option? you must be connecting to something other than PostGIS because that dialog doesn't have or use the next button. If so, I'm going to need details and we will have to work together because I don't have access to any of the proprietary databases so I can't trouble shoot against them. Thanks again for the review. Let's nail down a design that is maintainable over the long run and then we can tackle the bugs. --adrian
Attachment:
Screenshot-1.png
Description: PNG image
