Re: Setting the properties of multiple selected objects in stead of just one (the first or only selected->data)

[Philip Van Hoof <spamfrommailing freax org>]

On Mon, 2005-01-10 at 19:52 +0100, Philip Van Hoof wrote:
> [subject]: This patch will do that.
> 

This new one fixes some issues Hans explained on the mailinglists.

Aside from the issue's this one also adds undo/redo history support for
new functionality.

I haven't yet introduced a checkbox before each property that could
allow the user to choose whether or not setting it should be shared
across all selected instances of objects.

I'd rather create a new menu "Object" and a submenuitem "Properties" and
leave the "Objects->Properties" menu the way after applying this patch. 

It sounds like a much more convenient and more "usable in terms of
usability"-solution to me, than to introduce a checkbox before each
property in the intersection of properties of the selected objects.

It's clear to the user that the menu selection "Objects->Properties" is
about setting the properties of all object in the current selection. If
the user dislikes that, she'd probably search for a menu "Object->
Properties" rather than "Objects->Properties". Creating custom widgets
which massive amounts of checkboxes and a jungle of options isn't going
to make it more easy for her to understand what exactly is going to
happen. I did the test with my girlfriend using a prototype of such a
dialogbox quickly created with Glade-2. I can assure you that the
usability-aspect of my prototyped dialogbox was below low. She didn't
understand a thing about that checkbox next to each object-property. I
needed multiple minutes to explain it to her. Insane!

About the fact that there's already a python plugin that does this: I
wouldn't do this in a plugin. It's clear that this most requested
feature isn't unworthy to get into the main-code and therefor shouldn't
be done as a plugin. Let alone a python-plugin which even requires the
availability of extra libraries like a Python environment and PyGtk.
Thats like requiring a webbrowser for the feature "selecting objects on
a diagram". It might make sense to us, developers. I promise you,
however, it wont make any sense to a moral common user. And it
shouldn't. I won't expect from my girlfriend to EVER understand such
things about "computers". I'll just code them for her, so that she can
USE them. Thats the purpose of my life as a programmer, right? To make
computers USEable for USERs.

To Hans Breuer: My first example wasn't a patch already. It was more a
sneak preview for the concept which I had in mind. It basically was my
question to the group linked to the "but"-paragraph in the HACKING file:

Feel free to hack away at dia, but you're advised to contact
the dia maintainers and/or the mailing list if you do any
larger work --- this is in everyone's interest so that work is
not duplicated.

.. Indeed. So I did.


Since I wanted to code something this evening already, I finished it in
the hopes that it might just get accepted. I wouldn't have bothered with
the python plugin since I don't think writing a plugin for everything is
really a solution for every bug/feature request. As I stated above, this
feels to me like core functionality. Not like an extension.

The C++ style comments weren't suppose give the idea that I would have
committed it on approval. It where comments that I had put there while
writing the E-mail. Nevertheless I've made sure there aren't any 
C++ style comments left in this new patch. I also removed the extern
function declaration. I actually copied it from another file which I
haven't yet altered. So it wouldn't be only me decreasing code quality:

[freax lort lib]$ grep pdtpp_is_visible_no_standard propdialogs.c
extern gboolean pdtpp_is_visible_no_standard(const PropDescription
*pdesc);
      props = prop_list_from_descs(pdesc,pdtpp_is_visible_no_standard);
[freax lort lib]$

So .. I recommend removing it from propdialogs.c to increase code
quality of that file. As you noticed in my patch, newcomer developers
exploring the current sources might get confused by such things in the
current code while trying to fix or add something trivial like I've been
trying to do. Here's a little patch that will remove the line:

Index: propdialogs.c
===================================================================
RCS file: /cvs/gnome/dia/lib/propdialogs.c,v
retrieving revision 1.10
diff -u -r1.10 propdialogs.c
--- propdialogs.c       21 May 2004 15:48:34 -0000      1.10
+++ propdialogs.c       11 Jan 2005 00:40:53 -0000
@@ -289,7 +289,6 @@

   }
 }
-extern gboolean pdtpp_is_visible_no_standard(const PropDescription *pdesc);
 static void
 prop_dialog_fill(PropDialog *dialog, DiaObject *obj, gboolean is_default) {
   const PropDescription *pdesc;

I tested this. It compiles without the extern declaration.



-- 
Philip Van Hoof, Software Developer @ Cronos
home: me at freax dot org
gnome: pvanhoof at gnome dot org
work: philip dot vanhoof at cronos dot be
junk: philip dot vanhoof at gmail dot com
http://www.freax.be, http://www.freax.eu.org

? Diagram1.dia.autosave
? lib/diamarshal.c
? lib/diamarshal.h
? plug-ins/gprint/Makefile
? plug-ins/gprint/Makefile.in
Index: ChangeLog
===================================================================
RCS file: /cvs/gnome/dia/ChangeLog,v
retrieving revision 1.1657
diff -u -r1.1657 ChangeLog
--- ChangeLog	31 Dec 2004 20:13:35 -0000	1.1657
+++ ChangeLog	11 Jan 2005 00:19:57 -0000
@@ -1,3 +1,9 @@
+2004-01-10  Philip Van Hoof  <pvanhoof gnome org>
+
+	* app/properties.c: A properties box when multiple objects
+	are selected. The intersection of the available properties
+	is showed.
+
 2004-12-31  Hans Breuer  <hans breuer org>
 
 	* lib/dia_dirs.c(dia_get_canonical_path) : complete
Index: app/commands.c
===================================================================
RCS file: /cvs/gnome/dia/app/commands.c,v
retrieving revision 1.128
diff -u -r1.128 commands.c
--- app/commands.c	6 Nov 2004 16:51:23 -0000	1.128
+++ app/commands.c	11 Jan 2005 00:19:59 -0000
@@ -1082,8 +1082,13 @@
   if (!dia) return;
 
   if (dia->data->selected != NULL) {
+
+    properties_of_selected_show (dia, dia->data->selected);
+/*
     selected = dia->data->selected->data;
     properties_show(dia, selected);
+*/
+
   } else {
     diagram_properties_show(dia);
   } 
Index: app/properties.c
===================================================================
RCS file: /cvs/gnome/dia/app/properties.c,v
retrieving revision 1.35
diff -u -r1.35 properties.c
--- app/properties.c	4 Jul 2004 23:03:07 -0000	1.35
+++ app/properties.c	11 Jan 2005 00:19:59 -0000
@@ -22,7 +22,8 @@
 
 #include <gtk/gtk.h>
 #include <gdk/gdkkeysyms.h>
-
+#include "propinternals.h"
+#include "lib/properties.h"
 #include "intl.h"
 #include "properties.h"
 #include "object_ops.h"
@@ -163,6 +164,119 @@
 
   return 0;
 }
+
+void
+properties_of_selected_show (Diagram *dia, GList *selected_in)
+{
+  GList *selected = g_list_first(selected_in);
+
+  if (selected)
+  {
+	gint dialog_retval = 0;
+	PropDialog *pdialog = prop_dialog_new (selected->data, FALSE);
+	GList *have = NULL;
+	PropDescription *pdesc = NULL;
+	GtkWidget *dialog = gtk_dialog_new_with_buttons(
+             	("Properties of multiple objects"),
+             GTK_WINDOW (ddisplay_active()->shell),
+             GTK_DIALOG_DESTROY_WITH_PARENT,
+             GTK_STOCK_CLOSE, GTK_RESPONSE_CLOSE,
+             GTK_STOCK_APPLY, GTK_RESPONSE_APPLY,
+             GTK_STOCK_OK, GTK_RESPONSE_OK,
+             NULL);
+
+	gtk_dialog_set_default_response (GTK_DIALOG(dialog), GTK_RESPONSE_OK);
+
+
+	have = g_list_append (have, (gpointer)object_get_prop_descriptions (selected->data));
+
+	while (selected)
+	{
+		DiaObject *obj = selected->data;
+		if (object_complies_with_stdprop (obj))
+		{
+			guint i;
+			GPtrArray *props = NULL;
+
+			
+			have = g_list_append (have, (gpointer)object_get_prop_descriptions (obj));
+			pdesc = prop_desc_lists_intersection (have);
+
+			props = prop_list_from_descs(pdesc, pdtpp_is_visible);
+
+			obj->ops->get_props(obj,props);
+
+			for (i = 0; i < props->len; i++) {
+				Property *prop = (Property*)g_ptr_array_index(props,i);
+    				prop_dialog_add_property(pdialog, prop);
+			}
+
+			
+		}
+
+		selected = g_list_next (selected);
+	}
+
+	g_list_free (have);
+
+
+	gtk_box_pack_start_defaults (GTK_BOX (GTK_DIALOG (dialog)->vbox), prop_dialog_get_widget(pdialog));
+	gtk_widget_show_all (prop_dialog_get_widget (pdialog));
+
+
+	dialog_retval = gtk_dialog_run (GTK_DIALOG(dialog));
+
+	if (dialog_retval == GTK_RESPONSE_APPLY || dialog_retval == GTK_RESPONSE_OK)
+	{
+
+		selected = selected_in;
+		while (selected)
+		{
+			if (selected->data) 
+			{
+				ObjectChange *obj_change = NULL;
+
+				current_obj = selected->data;
+				current_dia = dia;
+				object_part = pdialog->widget;
+
+
+				object_add_updates(current_obj, current_dia);
+				obj_change = current_obj->ops->apply_properties(current_obj, object_part);
+ 				object_add_updates(current_obj, current_dia);
+
+				diagram_update_connections_object(current_dia, current_obj, TRUE);
+
+ 				if (obj_change != NULL) {
+					undo_object_change(current_dia, current_obj, obj_change);
+				}
+
+				diagram_modified(current_dia);
+
+				diagram_object_modified(current_dia, current_obj);
+
+				diagram_update_extents(current_dia);
+
+				if (obj_change != NULL) {
+					undo_set_transactionpoint(current_dia->undo);
+				}  else {
+					message_warning(_("This object doesn't support Undo/Redo.\n"
+						"Undo information erased."));
+					undo_clear(current_dia->undo);
+				}
+				diagram_flush(current_dia);
+
+
+			}
+			selected = g_list_next (selected);
+		}
+	}
+
+	gtk_widget_destroy (dialog);
+
+  }
+}
+
 
 void
 properties_show(Diagram *dia, DiaObject *obj)
Index: app/properties.h
===================================================================
RCS file: /cvs/gnome/dia/app/properties.h,v
retrieving revision 1.6
diff -u -r1.6 properties.h
--- app/properties.h	21 May 2004 15:48:33 -0000	1.6
+++ app/properties.h	11 Jan 2005 00:19:59 -0000
@@ -21,6 +21,7 @@
 #include "object.h"
 #include "diagram.h"
 
+void properties_of_selected_show (Diagram *dia, GList *selected_in);
 void properties_show(Diagram *dia, DiaObject *obj);
 void properties_update_if_shown(Diagram *dia, DiaObject *obj);
 void properties_hide_if_shown(Diagram *dia, DiaObject *obj);