Re: [Planner Dev] Planner enhancements and patches up to 31st March 2004 from Lincoln sandbox.

[Richard Hult <richard imendio com>]

Hi again, 

Thanks for the quick response!

> > We need to think about the part that adds a submenu with all the task
> > dialog pages. I'm not sure it really adds so much that it justifies the
> > extra UI clutter.
> 
> Once you start to use it then it does become useful. The dialogs
> are also hotkeyed too. Planner is a end-user tool but until
> people become fully conversant with it the menus should help guide
> users as to whats available. Having the dialog submenu entry also
> helps driving people over-the-phone (if it ever comes to that
> when we have world domination ;)
>
> To get to a task edit you can either,
> Right-mouse,
> Hotkey/accelerator,
> Action -> Edit...->Focus...

The thing is that we still have a lot of features to add, and the
menus/toolbars are already starting to fill up. And it's often a bad
idea to remove a lot of things when people have get used to them, so we
need to keep things clean and really think about the UIs we add.

As long as getting the task dialog up is very easy, you can always
change the active tab both with the mouse and the keyboard. We could
also make the dialog remember the last opened page, for example.

> >>http://bugzilla.gnome.org/show_bug.cgi?id=138368
> >>(Altering resource assignments does not trigger save reminder)
> > 
> > 
> > The correct fix should be done in the model rather than the view, i.e.
> > somewhere in libplanner/.
> 
> I think its because task_dialog_resource_units_cell_edited() should
> be using assignment_set_property() and not just g_object_set() and then
> I'd add the mrp_object_changed (MRP_OBJECT (resource)) to assignment_set_property().
> 
> Is that a better way ?.

It should ideally be mrp_object_changed (assignment) but there might be
a bug somewhere so that it doesn't listen to the changed signal on
assignments, not sure.
 
> > In the same file: "if (strncmp(short_name,"NULL",4) == 0)" that area
> > shouldn't be added. If the string is set to NULL, it indicates that a
> > database upgrade wasn't done correctly (or if we allow null,
> > then ..._get_string should be changed to return an empty string
> > instead).
> 
> I sort of agree but I'd like to keep this hack as it is because we do
> not yet have an upgrade routine so this hides any potential mess in the
> database from the user. The code can be gutted out later but it does a job
> right now.

It would be a lot better if we could decide null fields if present mean
an empty string, and fix _get_string() to return an empty string instead
of "NULL".

> > @@ -2063,6 +2069,7 @@ sql_read_tasks (SQLData *data)
> >  				     "note", note,
> >  				     "type", type,
> >  				     "percent_complete", percent_complete,
> > +				     "priority", (priority ? priority : 0),
> > 
> > No need for the check there, if it's 0, the value already is 0.
> 
> I'll have to think about what I'm doing here to make sure I'm handling
> nulls in the priority field OK.

The above code is the same as just "priority", priority, so it should be
fine :)
 
> Typedef enum {
> 	TASK_PAGE_GENERAL = 0,
> 	TASK_PAGE_RESOURCES   = 1,
> 	TASK_PAGE_PREDECESSORS  =   2,
> 	TASK_PAGE_NOTES          =  3,
> } EditDialogPage;

I think it would be best to put it in planner-task-dialog.h and call the
type TaskDialogPage, and there's no need to assign the numbers manually.


Thanks again and nice to hear about your plans!
Richard

-- 
Richard Hult                    richard imendio com
Imendio                         http://www.imendio.com