I've committed the dialog. Please have a look, and let's get over with it
2008/11/14, Kevin Menard <[EMAIL PROTECTED]>: > > Comments are in-line. > > > On Wed, Nov 12, 2008 at 4:38 AM, Andrus Adamchik <[EMAIL PROTECTED]> > wrote: > > Cool. I very much like the direction. Here is a few notes on the > > implementation: > > > > 1. I think "revert" and "clear" are redundant. They don't revert previous > > "save", but simply go back in the browser. It is just as easy to use the > > browser to achieve that. So I suggest we remove those buttons. > > > Are you seeing something different than me? If I modify an existing > relationship, revert takes me back to that mapped relationship. Clear > removes all selection. > > > > 2. "Save Path" should probably be called "Select", as we are not really > > saving anything until "Done" is clicked. Also since we only have 1 button > > now, maybe to make things more compact and consistent with other similar > > interfaces, implement it as a toolbar on top of the browser (see Select > > Query Ordering tab for an example). > > > +1. > > > > 3. DbRelationships. There's a bit of a problem figuring context of the > new > > relationship. It correctly uses a target of the currently selected > > DbRelationship path as its source, however its location is disjoint from > the > > browser so it is not immediately clear. It also clutters the view a bit. > So > > maybe we can add an extra dialog started with a "new relationship" button > > icon on the browser toolbar (see #2 - we will have a toolbar), that > allows > > you to select target entity, cardinality (to-one, to-many) and continue > to > > the joins mapping? > > > +1. The UI is a bit overloaded at the moment. Another option may be > to implement the dialog as a frame with tab panes. > > I'm still not a super huge fan of the constant expansion, but I think > the improvements made are making it less of an issue. > > I'd also like to see a bit more culling. Right now 1-1 are culled. > We could also cull 1 - m, m - 1, since the 1 is the same in each case. > I think that would cut down on confusion a bit more. > > -- > > Kevin >
