GNOME Bugzilla – Bug 60331
Object properties should handle multiple objects
Last modified: 2010-01-14 16:32:56 UTC
At present, the object properties dialog can only alter a single shape, and it's not always clear what shape will be adjusted. For the former, it would be much more efficient for some uses (e.g. changing the background color of a whole swath of objects) if the object properties, or at least the primitive ones (colors, line style, font) could be changed en-masse. Alternately, a bounding set of all possible property changes could be shown in the object properties dialog, if that was easier to implement. If it really must affect only a single shape, it would be useful to have a visual indication of which object is being changed -- scrolling to it and giving it an animated or colored outline, to distinguish it from the other selected objects, for example. For the latter, there's some confusion in which object will be altered when the object properties dialog is invoked (esp via the menu) while multiple objects are selected. It seems to be the rule that the most recently selected shape is the one affected, but this requires more temporal recollection than the user should be required to do.
Mass reassign of bugs to dia-maint@bugzilla.gnome.org.
I second this very, very strongly. This is one of my biggest bugbears when using Dia and it's more and more annoying the more objects one has. It would be great if this could be added for 0.91
I agree with you totally. Even I find myself trying to set properties for a multiple selection. Note that if you group the objects, you can set properties for the entire group. This, however, is not an intuitive solution.
Upping priority. I would like this done for release 0.91.
*** Bug 105083 has been marked as a duplicate of this bug. ***
Won't make it for 0.91. As a usability issue, a must for 0.92.
*** Bug 117417 has been marked as a duplicate of this bug. ***
*** Bug 113139 has been marked as a duplicate of this bug. ***
*** Bug 134310 has been marked as a duplicate of this bug. ***
*** Bug 138071 has been marked as a duplicate of this bug. ***
*** Bug 140928 has been marked as a duplicate of this bug. ***
Bugs should not target milestones we have already passed, mass retargetting to version 0.95.
Created attachment 36560 [details] [review] Dirty way of fixing this bug This patch allows you to apply the properties from the object you get properties from to all objects selected. This is not a really nice way of resolving this issue though, but I let maintainers decide what to do with this. A cleaner way would be to get the properties in common among all the objects selected, if I'm asked to do so, I can manage to work on this issue.
IMO there needs to be a more advanced version. Some work in progress is discused on the list: http://mail.gnome.org/archives/dia-list/2005-January/msg00033.html
Looks like just what I said : You must only display properties that all the objects selected have in common. I'll provide something as soon as I have time.
*** Bug 167497 has been marked as a duplicate of this bug. ***
Comment on attachment 36560 [details] [review] Dirty way of fixing this bug This patch a) does not deal with different kinds of objects being selected, b) smashes unchanged properties that differ between objects, and c) does not work with undo. The discussion on the dia list has better work in this direction.
Without that patch, we can't expect to get this working for 0.95
ok, I should find time to fix this patch with your instructions. That shouldn't be so hard to fix. I will do my best.
*** Bug 301449 has been marked as a duplicate of this bug. ***
*** Bug 305108 has been marked as a duplicate of this bug. ***
*** Bug 310104 has been marked as a duplicate of this bug. ***
This is how I think the properties dialog should work for multiple selected objects: 1) When objects of different kinds are selected, the dialog should actually show a union of all the applicable properties, not their intersection. The user can then freely select objects and set properties. Each property will be set for all objects for which it actually exists. Of course, this is subject to the requirement that the number of properties displayed is not overwhelming to the user. 2) When a property has different values for different objects, the dialog should show it as grayed out, but not disabled. If the user sets a value to this property, it gets coloured again, and is set for all objects as applicable. If the user does not modify it, each object retains the existing value for that object. Currently if I group a dashed line and a solid line, and try to change their colour, both lines become either dashed or solid, depending on which was selected last. A very good implementation of such an interface can be seen in Ex Falso, the tag editor for audio files.
I agree with Sameer on how it should work. However, implementation is not trivial. *Working* patches would be welcome. Not happening by my hand this version, as text handling has superseded it.
*** Bug 392827 has been marked as a duplicate of this bug. ***
(In reply to comment #24) > I agree with Sameer on how it should work. However, implementation is not > trivial. *Working* patches would be welcome. Not happening by my hand this > version, as text handling has superseded it. Been spending some time exploring the dia source code ... I think I at least found the place where work needs to be done, and some idea of what changes to make. I havn't joined the mailing list yet, but I hope someone will give a general nod here so that I can think of implementing an actual solution. I suppose the start would be to replace the call to prop_desc_lists_intersection() in lib/group.c with something better, that creates a union, and at the same time marks properties that have different values. The relevant flag seems to be PROP_FLAG_MULTIVALUE. Once we have a property list with these marked properties, we can modify the property dialog to gray out these marked entries, and apply them only if changed. Eventually, the same kind of union can be created for a general ungrouped selection of objects. Am I right??
Created attachment 81642 [details] [review] signal handling in the properties dialog to handle only those properties that have changed This patch makes it possible to show a properties dialog that is the union of properties for all objects in a group. The PXP_NOTSET flag is used in the property experience field to determine which properties are actually modified by the user. This patch attaches the higher-level property_signal_handler() without checking for prop->event_handler. The signal_handler will call prop->event_handler only if defined. The patch will not work in the current state. For that we need to define a suitable signal for all property widgets that are currently using "FIXME". Also, some signal connections are not working. I couldn't figure out why due to a general lack of experience with Gtk+, but I have fixed one example for the REAL property type.
It's a little confusing to look at what's happening, but it does look like you've gotten them to only change if the user has actually fiddled with the value. Some indication of whether a value has been changed or not will be necessary, as will as "reset" button, given that you can't reset to a multivalue. What I see definitely not working is that only one object is actually changed when applying.
(In reply to comment #28) > It's a little confusing to look at what's happening, but it does look like > you've gotten them to only change if the user has actually fiddled with the > value. Right ... "fiddled" is the correct word, and not "changed". It seems quite non-trivial and unnecessary to actually check whether the value has changed after the user has fiddled with the property widget in the dialog. > Some indication of whether a value has been changed or not will be > necessary, as will as "reset" button, given that you can't reset to a > multivalue. Yes. This is more of a "user-friendliness" call, which I am not experienced enough to make. The indication could be some form of colour change. For the reset, we could have a tickbox per property, like the current python plugin that tries to provide a similar functionality. And of course a reset button in addition. Basically, I don't know what to do with the UI ... hoping someone else can provide better ideas. Aside from that, I have a feeling that using a union of properties might be quite overwhelming for a user and probably not very useful ... an intersection might turn out to be better. > What I see definitely not working is that only one object is > actually changed when applying. Please explain!
Since each property can start out multi-valued, the user cannot easily change a value back through the control, so a value-change check doesn't make sense. I think a checkbox at each value would be the right thing -- definitely with a tooltip. The user could then uncheck it to reset the property to its original (possibly multivalued) value. The concept of the checkbox would be "set this property". The union has the disadvantage that the user cannot tell which objects are affected. However, I don't think it's something we need to strictly decide on now, it'd be nice to try either way, given that the code difference isn't too great. I tried using your code to change the values of several selected objects, but only one object actually changed value. I can give you a more precise description if you want.
(In reply to comment #30) > Since each property can start out multi-valued, the user cannot easily change a > value back through the control, so a value-change check doesn't make sense. I agree. > I think a checkbox at each value would be the right thing -- definitely with a > tooltip. The user could then uncheck it to reset the property to its original > (possibly multivalued) value. The concept of the checkbox would be "set this > property". Only one thing to take care of --- the checkbox doesn't make sense when changing the properties of a single object. In that case, it becomes an unnecessary extra click for the user. It should be easy enough to hide the checkbox when the diaglog is created on a single object. > I tried using your code to change the values of several selected objects, but > only one object actually changed value. I can give you a more precise > description if you want. The patch I provided is not supposed to work for most types of objects, since it depends on signals not yet implemented in the property widgets. Also, you've said "several selected objects" ... I didn't target that. The patch is for group properties only. PS: We should go for the checkbox approach. But that means I don't need to worry about implementing signals in the property widgets. But anyway, I would still like to push the patch submitted elsewhere, for the dynamic_menu, since it makes that widget a bit cleaner.
I just had a talk with our local usability expert about this issue (and more). He suggested that we follow the standard of having a greyed-out selector when there is multiple values, and GTK supports this. Only problem is that our home-grown selectors don't support it, so that'll take some work. I suggest we use the checkbox for those widgets that don't have an "inconsistent" state (I think that's the word) until such time as it gets implemented. He also agreed that using the intersection is the right start, though possibly extending with showing the "standard" settings (line color, line width, dashing etc) if at least one element uses them -- there's a slight problem there of how can the user tell the difference. Another possible extension would be to have tabs of properties, where the first tab shows properties shared by all objects, the next tab shows it for some subset, etc. That, too, would be further down the road.
(In reply to comment #32) > use the checkbox for those widgets that don't have an "inconsistent" state (I > think that's the word) until such time as it gets implemented. Makes sense. I intend to tackle both tasks --- getting the properties dialog to work correctly for groups, and cleaning up the home-grown widgets. Cleaning includes making them behave like standard gtk+ widgets, and removing the deprecated API calls while at it.
I tried out your patch, and it does indeed work for those properties it supports. Nice job. One thing that will need to be incorporated (sooner probably better than later) is undo -- currently undo sets all the objects to the same values.
The patch is now included in the development code. Undo would be a very nice thing to have, and I'm not closing the bug until we have it.
Two important things: We're now getting a *lot* of errors and warnings on startup, starting with (lt-dia:27060): GLib-GObject-WARNING **: gsignal.c:1249: signal "changed" already exists in the `GtkOptionMenu' class ancestry (lt-dia:27060): GLib-CRITICAL **: g_strcasecmp: assertion `s1 != NULL' failed (lt-dia:27060): Gtk-CRITICAL **: gtk_container_foreach: assertion `GTK_IS_CONTAINER (container)' failed ** (lt-dia:27060): CRITICAL **: gtk_wrap_box_set_aspect_ratio: assertion `GTK_IS_WRAP_BOX (wbox)' failed Also, sheet selection now changes the menu immediately, but the sheet icons only change after the menu is selected again. Both these things need fixing ASAP. I'm including the code to allow more people to look at it and to make progress easier. This must be fixed by 0.97.
Created attachment 86555 [details] [review] corrections to the patch dynamic menu signal handling The earlier patch for DiaDynamicMenu and its users had two memory leaks. It also declared default_handlers that were never defined. Fixed the leaks and removed the handlers in this patch.
(In reply to comment #35) > The patch is now included in the development code. Undo would be a very nice > thing to have, and I'm not closing the bug until we have it. > I have not looked at how undo works in Dia, and how to make it work correctly for group-wide properties. Any help will be greatly appreciated. The bug is far from being closed --- there's lots to do. I had been unable to work on this for some time, and one thing is clear --- it's not worth it to have a stop-gap solution of checkboxes in the properties dialog. I would rather put whatever time I have into doing it the right way from the first time itself. Hence I intend to go forward and fix all the signal handling code related to the properties dialog. Then we can also figure out how to support "inconsistent" states in the home-grown widgets.
> (lt-dia:27060): GLib-GObject-WARNING **: gsignal.c:1249: signal "changed" > already exists in the `GtkOptionMenu' class ancestry This seems to be for the DiaDynamicMenu widget. My mistake for choosing that name. Will either use the parent's "changed" signal or invent another name for that signal. > Also, sheet selection now changes the menu immediately, but the sheet icons > only change after the menu is selected again. > > Both these things need fixing ASAP. I'm including the code to allow more > people to look at it and to make progress easier. This must be fixed by 0.97. Great.
I've applied your patch from above. The sheet selection menu seems to simply lag one selection behind, and there's still massive amounts of warnings. I've started a page on live.gnome.org about the undo system.
Created attachment 86618 [details] [review] signal renamed in DiaDynamicMenu, removes all warnings observed This new patch includes the earlier patch about corrections. The signal "changed" was already present in GtkOptionMenu, that DiaDynamicMenu is derived from, which was causing all the headaches. This signal has been renamed to "value-changed". The "changed" signal cannot be reused since the behaviour of DynamicMenu is complicated and causes race conditions. Also fixed DiaSizeSelector so that all the users of DiaDynamicMenu now work correctly and without warnings --- sheet menu, font style selector, color selector and arrow selector. Now to cleanup the rest of the widgets. Note that the property dialog will not work, irrespective of groups or individual objects, until all property widgets can emit signals.
Thank you! I've applied this latest patch to SVN, it does appear to work for the props that have the signal.
Created attachment 91660 [details] [review] simplified some widgets in the property dialog and all of them now have an event handler to track PXP_NOTSET Got tired of waiting for feedback on the mailing list, for some specific changes I was making to one widget. Hence dumping it all here. I have no experience in splitting patches, else would have been glad to do that. If only we were using something like darcs instead of SVN!
The 2007-07-12 patch applied fine for me. But is it supposed to allow changing of properties for multiple objects already? That doesn't seem to work for me yet.
No, the current work is only for a group, not multiple selected objects.
That's definitely a lot easier to look at now. Thanks! Applied in SVN. Now the unit spinner can reasonably be put on more things (corner radius, text padding, arrow head sizes, dash/dot length)... But I can do that. Getting undo to work with multi-object setting is more important.
Created attachment 92692 [details] [review] implements undo for groups; but redo does not work; recursion not verified Here's a basic implementation of undo for setting properties of multiple objects in a group. For some reason redo does not work; any pointers will be useful. Also have not checked yet with a group that contains a group. This particular beast also needs to be tested for the properties dialog!
Update: Further checking shows that the undo/redo works correctly for a group. But redo has problems when there is a group inside a group. Inside the group, the redo sets one value for each property on all objects. Thus if there was a red circle in the nested group and the border was changed to blue, there is a chance that the redo will set the border of all objects to red. Currently investigating this.
Created attachment 92845 [details] [review] undo/redo in setting properties now works for grouped objects, even with nested groups It is done. Undo/Redo now works in all cases, even when a group contains other groups. For this I had to modify the vtable for Objects, ie, struct _ObjectOps. The function object_apply_props() in lib/propobject.c must behave differently when called on a group object. Hence introduced this as a new function in the vtable as "ApplyObjectPropertiesFunc". In order to avoid confusion, also renamed ApplyPropertiesFunc to ApplyPropertiesDialogFunc. All files under objects/ have been updated to reflect these changes in the vtable, hence the large size of this patch. The interesting stuff is happening only in a few files under app/ and lib/
*That's* why it was bugging me that you're working on the groups rather than the current selection: The group system is currently somewhat broken and in the middle of being replaced (using #ifdefs for now, though). See my notes on http://live.gnome.org/Dia/CurrentDevelopment. Adding a new function to the vtable simply to apply properties in a group when we're planning to move that functionality outside of grouping is not good, especially since it breaks binary compatibility (that's why there are empty slots in the object_ops table). I think it would be better to move to selection-based multi-properties now, before it gets even messier. Sorry about not picking up on this earlier.
After a bit of soul searching, I am back. I believe this is still worth doing. > *That's* why it was bugging me that you're working on the groups rather than > the current selection: The group system is currently somewhat broken and in > the middle of being replaced (using #ifdefs for now, though). This is why it is worth doing. Because the new group system is a long way away, and a really important feature currently sucks in Dia. I have a fix for it. > Adding a new function to the > vtable simply to apply properties in a group That's not a trivial feature. And we are not introducing something new. My changes actually fix a bad abstraction of a class method. Every class should have a method to apply properties to itself. Instead the current code wrongly uses a single global function to do this for all classes. Dia was not able to handle group properties because of this problem with the original interface described in struct _ObjectOps. > when we're planning to move that > functionality outside of grouping is not good, especially since it breaks > binary compatibility (that's why there are empty slots in the object_ops > table). I suppose it would be ok if I moved the new function to a reserved slot then. A couple of things that come to mind. Since this new function actually fixes a broken interface, it should not be considered optional, even for new code. I would vote for breaking binary compatibility just to force everyone to take note of this new "fix". Exactly what will be the result of breaking compatibility? Given that Dia is still below 1.0, is it too important? > I think it would be better to move to selection-based multi-properties now, > before it gets even messier. Sorry about not picking up on this earlier. It shouldn't get any messier. I think waiting is a bad idea in this case.
As noted in http://mail.gnome.org/archives/dia-list/2007-August/msg00026.html """ Just looked at the lst patch. It is changing every objects 'vtable' and thus breaking binary compatibility. If necessary at all it should be added at the end of the vtable (see objects.h struct _ObjectOps) having only 4 unused left. Also one would need to implement a default handling for case where the objects function pointer is NULL. """ I'd like to get the functionality in before we may have the all new group object *some* day. But the placement in a new slot (rather than changing exisiting ones) is crucial not only for binary compatibility, but also for better source code maintenance. With a true OO language the compiler would match the correct signature to the correct 'slot'. But with C and also the function pointer casts we would produce very hard to detect bugs if we change function positions in the 'vtable'.
I agree now that it should go into the current system, and not be held up by the new groups. I do agree with Hans, though, that using a reserved slot is the way to go, both to avoid messing with too much code and to keep binary compatibility. Otherwise, your patch looks ok.
Created attachment 96497 [details] [review] retrofitting the vtable with a new function to handle undo, especially when changing group properties Updated all the objects to use the first unused slot in the vtable. Also changed a few names for the sake of readability assuming that will remain transparent in terms of binary compatibility! Especially a new typedef for "ReservedSlotFunc" that helps in type-casting when initialising the vtable. Note that I have not delved into newgroup.c much ... only inserted a warning there, with a null value for the relevant function.
Created attachment 96498 [details] [review] retrofitting the vtable with a new function to handle undo, especially when changing group properties Slightly cleaner version of the same patch ... forgot to dump a diff after compiling and testing it.
There's a couple problems with this patch yet, but it's getting better. You should not use an unused slot in the sense of unused[0], but make a new field and reduce the number of unused slots to four. Also, you don't need to add initialization for the unused slots, as long as there's a comma at the end. Also, rather than having an assert on obj->ops->apply_properties_list, we should just call the default function in case of NULL; otherwise, old binaries will cause assertions by default rather than just working nicely. If the default function is called on NULL, we don't need to insert it in all the other objects. I don't understand the note in object.h: IMPORTANT: In this case, all code should fail if this op is not supported, since there is no sane default behaviour that can handle undo/redo when applying properties. Isn't object_apply_props exactly the behaviour desired for all other objects than Group, given that that used to be the behaviour anyway? Why can't we assume that if the function field is NULL, we can call this function? I'm not yet convinced that adding a new object op is really necessary for doing this. Can you give an example of why any other object than the group object would want to not use the default? There is already an IS_GROUP macro in use, why not keep the old function and have it do a special case for groups? I think it'd be easier to figure out what's going on that way. Passing the object to the object change functions may be a bit outdated, the TextChange doesn't have the object, but it doesn't need it either. It's probably something that could be fixed.
> You should not use an unused slot in the sense of unused[0], but make a new > field and reduce the number of unused slots to four. Also, you don't need to > add initialization for the unused slots, as long as there's a comma at the end. Given the talk about binary compatibility, I decided to choose the most stable approach of doing this, since I have zero experience in such work. I can change this to reduce the number of slots, although personally I still think the best way to do this is to just forget binary compatibility and introduce a new function. > Also, rather than having an assert on obj->ops->apply_properties_list, we > should just call the default function in case of NULL; otherwise, old binaries > will cause assertions by default rather than just working nicely. If the > default function is called on NULL, we don't need to insert it in all the other > objects. The problem is that supporting undo when applying properties is fundamental. It either works or doesn't work, there is no "work nicely". About the need to insert it in all objects, one could say that for all the functions in the vtable! The only reason I am checking the pointer here is because of the fear of the unknown in terms of backward compatibility ... rather than crashing, the code will at least tell us if some old unmaintained object is missing this function. Again, if we didn't care about binary compatibility this check would not be necessary because the missing function would cause compile time errors instead. > IMPORTANT: In this case, all code should fail if this op is not > supported, since there is no sane default behaviour that can handle > undo/redo when applying properties. This is simply meant as a warning for people who write new code ... they should not overlook unused[0]. But once I rearrange the unused array itself, this warning can go away. But I'll ask this once, just to clarify matter --- should I reduce unused[] and insert a new function, or just totally redo the vtable and ignore unused[]? I am not asking this because I like to be a pain in the ass, but only because I totally believe that the second option is the Right Thing to do. Personally, I would like to just remove unused[] from the vtable. > I'm not yet convinced that adding a new object op is really necessary for doing > this. Can you give an example of why any other object than the group object > would want to not use the default? There is already an IS_GROUP macro in use, > why not keep the old function and have it do a special case for groups? I > think it'd be easier to figure out what's going on that way. I must admit, the reason I initially did this was simply because I don't like special cases ... what's the point of emulating object oriented behaviour in C if we don't go the whole way? But only this weekend, I discovered this bug: Undo does not restore size of automatically resized shapes http://bugzilla.gnome.org/show_bug.cgi?id=318182 The perfect way to solve this is that any object that automatically changes other properties as a side effect should create its own composite Change object. > Passing the object to the object change functions may be a bit outdated, the > TextChange doesn't have the object, but it doesn't need it either. It's > probably something that could be fixed. I think this refers to the comments I inserted in unrelated places in the same patch. I admit asking questions in code comments is not very good style ... I'll instead flag this with a FIXME saying this is deprecated behaviour, and should be cleaned up eventually.
Sameer: I'm not questioning that you need to "add a new function" to the vtable, but apparently we made not yet clear how this is done while preserving binary and source compatibility. I tried in comment #52, you may also want to look into http://svn.gnome.org/viewcvs/dia/trunk/lib/object.h?r1=3029&r2=3028&pathrev=3029 which is at least done correctly on the technical level. 1) declare a new function pointer 2) add it at the after the last used slot - this will give 0 initialization for all vtables not implementing it, without the need to change anything in the object implementations (source compatibility). 3) reduce the size of the 'unused' array added for padding, thus the overall size of the vtable stays the same for all object vtables (binary compatibility) 4) use the new function by obj->ops->new_fun(). Before accessing it the function pointer should be checked for !=NULL. If it is an optional member function a fallback should be implmented. If it is a required function a g_warning would be appropriate. (True object oriented design does not allow the function pointer being NULL, so there should be always a default implementation. But there is no mechanism in C to enforce this, like it is in C++ where you can not instantiate a derived object without implementing all pure virtual functions.)
Created attachment 97273 [details] [review] apply_properties_list() makes correct use of an unused slot; assertion changed to a warning Here's the latest version. Fixed everything according to the comments so far. This should be good to go now!
Indeed, just committed. The only thing missing seems to be the handling for multiple selection not just the group object. Thanks! 2007-10-20 Hans Breuer <hans@breuer.org> * app/properties.c lib/group.c lib/newgroup.c lib/objchange.h lib/object.h lib/propobject.c objects/AADL/aadlbox.c objects/AADL/aadlbus.c objects/AADL/aadldata.c objects/AADL/aadldevice.c objects/AADL/aadlmemory.c objects/AADL/aadlpackage.c objects/AADL/aadlprocess.c objects/AADL/aadlprocessor.c objects/AADL/aadlsubprogram.c objects/AADL/aadlsystem.c objects/AADL/aadlthread.c objects/AADL/aadlthreadgroup.c objects/chronogram/chronoline.c objects/chronogram/chronoref.c objects/custom/custom_object.c objects/EML/instantiation.c objects/EML/interaction-ortho.c objects/EML/interaction.c objects/EML/process.c objects/ER/attribute.c objects/ER/entity.c objects/ER/participation.c objects/ER/relationship.c objects/flowchart/box.c objects/flowchart/diamond.c objects/flowchart/ellipse.c objects/flowchart/parallelogram.c objects/FS/flow-ortho.c objects/FS/flow-poly.c objects/FS/flow.c objects/FS/function.c objects/GRAFCET/action.c objects/GRAFCET/condition.c objects/GRAFCET/step.c objects/GRAFCET/transition.c objects/GRAFCET/vector.c objects/GRAFCET/vergent.c objects/Istar/actor.c objects/Istar/goal.c objects/Istar/link.c objects/Istar/other.c objects/Jackson/domain.c objects/Jackson/phenomenon.c objects/Jackson/requirement.c objects/KAOS/goal.c objects/KAOS/metaandorrel.c objects/KAOS/metabinrel.c objects/KAOS/other.c objects/Misc/analog_clock.c objects/network/basestation.c objects/network/bus.c objects/network/radiocell.c objects/network/wanlink.c objects/SADT/annotation.c objects/SADT/arrow.c objects/SADT/box.c objects/SISSI/area.c objects/SISSI/faraday.c objects/SISSI/room.c objects/SISSI/sissi_object.c objects/SISSI/site.c objects/standard/arc.c objects/standard/bezier.c objects/standard/beziergon.c objects/standard/box.c objects/standard/ellipse.c objects/standard/image.c objects/standard/line.c objects/standard/polygon.c objects/standard/polyline.c objects/standard/textobj.c objects/standard/zigzagline.c objects/UML/activity.c objects/UML/actor.c objects/UML/association.c objects/UML/branch.c objects/UML/class.c objects/UML/classicon.c objects/UML/component.c objects/UML/component_feature.c objects/UML/constraint.c objects/UML/dependency.c objects/UML/fork.c objects/UML/generalization.c objects/UML/implements.c objects/UML/large_package.c objects/UML/lifeline.c objects/UML/message.c objects/UML/node.c objects/UML/note.c objects/UML/object.c objects/UML/realizes.c objects/UML/small_package.c objects/UML/state.c objects/UML/state_term.c objects/UML/transition.c objects/UML/usecase.c : finally there is proper group properties handling thanks to patches from Sameer D. Sahasrabuddhe. Almost fixes long-standing bug #60331.
I came across this bug report as I was simply trying to change the background colour of multiple objects. For the benefit of anyone else with this problem, I though I'd note what I did that worked (presumably thanks to Sameer's hard work). I am on version 0.96.1 on Windows XP. Although you can't select multiple objects and directly apply a background colour, you can select multiple objects, group them, set a background colour, then ungroup them. Yay.
Tim, what you'll notice if you keep doing that in 0.96.1 is that you'll change all properties of all the objects, not just the ones you've modified. So if you try to make a blue line and a red line both be thicker that way, both will end up with the same color.
Lars, Cheers for the heads up. I had guessed this would be the case from reading the above discussion :-) Fortunately in my case all the other properties were the same anyway.
*** Bug 526284 has been marked as a duplicate of this bug. ***
*** Bug 526906 has been marked as a duplicate of this bug. ***
*** Bug 538617 has been marked as a duplicate of this bug. ***
Created attachment 123886 [details] [review] introduce a separate function to return property descriptions for a list of objects Bulk of the function group_describe_props is actually a more general operation of getting the property descriptions for a list of objects. Moved that into a separate function. Also introduced a flag to choose whether a union or intersection is desired of all the available properties.
Created attachment 124086 [details] [review] the properties dialog now tracks multiple objects With this patch along with the one before it, it is possible to select multiple objects and modify their properties from a single dialog box. In particular _PropDialog in lib/properties.h was modified: the DiaObject* fields orig_obj and obj_copy are replaced by equivalent GList*. This is accompanied by various wrapper functions for handling lists of objects in appropriate places.
Hi Sameer, I've commited your patches now. Unfortunately the second regression introduced isn't as easy to fix as I thought. At the moment the special dialogs from "UML - CLass" and e.g. Sissi and "Database - Table" are not accesible at all. Probably my commit was a bit too early ;) Should I revert or do you have a fix? 2008-12-07 Hans Breuer <hans@breuer.org> * app/commands.c app/diagram_tree.c app/modify_tool.c app/properties.c app/properties.h lib/group.c lib/object.h lib/propdesc.c lib/propdialogs.c lib/properties.h lib/propinternals.h lib/propobject.c : : patch from Sameer Sahasrabuddhe almost fixing Dia's oldest bug #60331 - Object properties should handle multiple objects (but it breaks with UM - Class) * lib/libdia.def : updated * objects/UML/class.c : don't crash when umlclass_apply_props_from_dialog() get callesd without umlclass_get_properties(), instead fall back to object_apply_props_from_dialog() than
Oops. My mistake for not thoroughly checking for regressions. Didn't check those special dialogs in my eagerness to put up a patch! I don't know what the problem is exactly, since I never even anticipated it. I think you should revert, unless _you_ have a fix :) . I'll poke around a bit more.
Commited a fix - needs some more review/testing, though. 2008-12-08 Hans Breuer <hans@breuer.org> * objects/UML/class.c : don't crash when umlclass_apply_props_from_dialog() get callesd without umlclass_get_properties(), instead fall back to object_apply_props_from_dialog() than * objects/Database/table.c : same here * app/properties.c : for a single object let Object::get_properties() create the "object part" of the property dialog
Created attachment 124250 [details] [review] updated the vtable for Database/table.c to use the correct function In the fix from Hans, the vtable in Database/table.c was not updated. Fixed that to use the correct ApplyPropertiesDialogFunc ... _table_dialog_apply_changes(). Also added a small comment in app/properties.c to improve readability. This is finally it, I think. Both special classes seem to work now.
Thanks, again. There probably is an issue remaining with objects/SISSI, but it crashed not where I expected;)
Sameer, if you don't know remaining issues I think we can finally close this bug ;) : 2009-01-09 Hans Breuer <hans@breuer.org> * lib/propobject.c(object_list_get_propdescriptions) : for single object in the list use the "intersection" because otherwise props with PROP_FLAG_DONT_MERGE would not be accessible anymore, e.g. for "Standard - Outline" there was no way to change the text anymore (hopefully final regression introduced by fixing bug #60331)
With reference to comment 73, the fix mentioned in comment 71 also needs to be applied for SISSI objects and one more dialog that I can't remember right now. While trying to use SISSI objects I encountered the crasher bug that I had to fix in a recent patch for the create_object tool. Also one more issue, but I think it should be filed as a separate bug: when one or more UML class objects are selected, the dialog shown should be the the one for UML class objects and not the generic dialog. Same applies for other special dialogs. The fix is to modify all the special dialog to handle lists of objects, just like the generic dialog, and then examine the currently selected objects in object_list_properties_show(), in app/properties-dialog.c
changing dedicated object dialog implementations to be multi-object aware is definitely a new feature - I'm not even sure we should do. So the remaining things here are fixing objects/SISSI and the "one more dialog that I can't remember".
Should be settled now: 2009-01-24 Hans Breuer <hans@breuer.org> * objects/SISSI/area.c objects/SISSI/faraday.c objects/SISSI/room.c objects/SISSI/sissi.[ch] objects/SISSI/sissi_dialog.c objects/SISSI/sissi_object.c objects/SISSI/site.c : fixed object method DiaObject::apply_props_from_dialog() for the last known issue with changing multiple objects properties, bug #60331 (It did not crash because the SISSI object don't contribute anything to the property list.) * objects/SISSI/sissi_object.c(280) : local variable 'filename' used without having been initialized * objects/SISSI/sissi_dialog.c(380) : incompatible types - from 'struct _GtkWidget *' to 'struct _GtkScrolledWindow *'
*** Bug 606943 has been marked as a duplicate of this bug. ***