Bug 129281 - Need a way to simultaneously edit the properties of multiple view objects.
Summary: Need a way to simultaneously edit the properties of multiple view objects.
Status: RESOLVED FIXED
Alias: None
Product: kst
Classification: Applications
Component: general (show other bugs)
Version: 1.x
Platform: unspecified Linux
: NOR wishlist
Target Milestone: ---
Assignee: kst
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-06-17 04:43 UTC by Netterfield
Modified: 2007-06-07 23:37 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments
Proposed patch (62.88 KB, patch)
2006-06-30 01:33 UTC, Andrew Walker
Details
Proposed patch (62.59 KB, patch)
2006-07-07 20:13 UTC, Andrew Walker
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Netterfield 2006-06-17 04:43:45 UTC
Version:           1.3.0_devel (using KDE 3.5.3, Kubuntu Package 4:3.5.3-0ubuntu0.1 dapper)
Compiler:          Target: i486-linux-gnu
OS:                Linux (i686) release 2.6.15-25-686

View objects need a way to apply settings to multiple objects.

In kst there are two models:
  1) EditMultiple in data objects
  2) Apply to... in the plot dialog.

(1) is more general (it will always be possible to do what you want) but is more steps to use, and is much harder to code and maintain.
(2) is easier to code and to use, but sometimes doesn't do what you want (though for legends, it would probably always do what you want)

In order to get the capabilities in place soon, I suggest we implement (2) for labels and legends now, and then move to (1) in the future when we have time to do it.
Comment 1 Nicolas Brisset 2006-06-19 09:41:39 UTC
Yes, please :-) Sounds good to me !
Comment 2 Andrew Walker 2006-06-30 01:33:52 UTC
Created attachment 16829 [details]
Proposed patch

Implements option 1 as the best long-term option. I believe everything is
functional though there is sure to be some debate about exactly how the feature
should work.
Comment 3 George Staikos 2006-07-04 16:50:56 UTC
 I'm not convinced by this in terms of where we want to go with view objects, 
or with some issues with the patch itself.  I think this patch makes view 
object code difficult to maintain.  We have this problem with data objects 
already, where Edit Multiple is often broken (and many releases simply crash 
in this mode) due to the unexpected impact of changes to dialogs.  Also the 
patch removes comments that should not be removed (again!), the " " label 
trick seems error-prone and hacky, and we shouldn't be filling up KstApp with 
this stuff.  Let's hold off on this patch for a while until we're sure of 
what we want, and until it's cleaned up.
Comment 4 Nicolas Brisset 2006-07-04 17:54:58 UTC
Even though the concept of being able to edit multiple view objects simultaneously sounds appealing, if it is too difficult to implement I think it can be left aside for a while, at least for manually-created objects. After all, if the user wants e.g. big fonts he can set them to his taste when creating the objects! Maybe all that is required is an easy way to set default properties (imagine you want to add many arrows with the same width, line type and color, not needing to change all of them for each new arrow is a good thing). I think the values were sticky at some point, but it is apparently no longer the case... We should probably add a checkbox or extra button to set current properties as default for other objects of the same type...

Now, the problem with legends is different as those are automatically-created objects. The user does not really get a chance to specify their default properties, which is the reason why it was *GOOD* to be able to change these properties from the plot dialog for all of them in one shot. Just restore that and allow sticky properties for manually-created object, and I'll be happy :-)
Comment 5 Andrew Walker 2006-07-04 22:04:36 UTC
The original bug report from Barth would seem to indicate that this is the way we want to go. The patch makes it more difficult to maintain the view object code, but any new feature makes the code more difficult to maintain. 

If the " " label causes errors then a bug report can be submitted or a more complete description of the problem attached to this bug report. I don't recall any crashes, or bug reports, for the Edit Multiple for data objects - but if there were then developers need to be more careful and ensure they understand the feature before modifying the code.

Comment 6 Andrew Walker 2006-07-04 22:14:46 UTC
Nicolas, the sticky properties were commented out with the following:

    // Removed by George.  This is very strange.  Some dialogs have 10+
    // properties, and when I change 8 of them, the next "new" object of the
    // same type has all of these modified on me.  I have to go through and
    // change them all back to what they were before.  I think this is too
    // confusing and annoying.  We could add a sticky flag or something like
    // that if this feature is really demanded.
    
    // and then save this viewObject's properties as the default
    if (_top) {
      _top->saveDefaults(_viewObject); 
    }

If you think this should be re-introduced in some form, please enter a new bug report.
Comment 7 Nicolas Brisset 2006-07-05 09:14:29 UTC
I think this should be reintroduced, but with either a checkbox or button so that the user can choose whether to reuse these settings as defaults for the next objects. The default should be as now, but there should be the option of remembering the settings for subsequent objects. While I'm thinking about this, a button to revert to "factory defaults" might be nice too...
I'll open a new report for the sticky view objects defaults, but I think we still have to agree on whether we want to be able to edit multiple view objects and the priority this feature has.
In any case, I consider it necessary to offer the user a possibility of editing multiple legends in one shot for the reasons exposed above, be it via the plot dialog or elsewhere.
Comment 8 Nicolas Brisset 2006-07-05 09:32:45 UTC
Someone has just made a *very interesting* suggestion to me: why not implement a "copy format" action (like in MS Word, with single click->single edit and double-click->multiple edit modes), which would allow to pick an object to get settings from and apply them to subsequent objects of the same type.
I don't know whether that is better from a coding perspective, but for users it is probably as good as an "edit multiple..." dialog because it allows for in-place editing of objects: you don't have to look them up in a possibly long list with possibly meaningless names, you just click on them.
I personnally think that's the best approach usability-wise (though I must confess I have not tried Andrew's patch and I don't know what the UI is like). Are there other opinions out there ?
Comment 9 George Staikos 2006-07-05 12:35:03 UTC
On first thought it sounds good to me.
Comment 10 George Staikos 2006-07-05 13:40:42 UTC
On Tuesday 04 July 2006 16:04, Andrew Walker wrote:

> ------- The original bug report from Barth would seem to indicate that this
> is the way we want to go. The patch makes it more difficult to maintain the
> view object code, but any new feature makes the code more difficult to
> maintain.


  That's definitely not true, and in particular not true with respect to the 
scope of the impact.

> If the " " label causes errors then a bug report can be submitted or a more
> complete description of the problem attached to this bug report. 


  I don't understand why it is necessary to introduce this.

> I don't 
> recall any crashes, or bug reports, for the Edit Multiple for data objects
> - but if there were then developers need to be more careful and ensure they
> understand the feature before modifying the code.


  Right, because I just fixed those crashes and bugs as I found them.  I did 
not file bug reports.  I don't want to make Kst more over-complicated than it 
already is.
Comment 11 Andrew Walker 2006-07-06 23:43:54 UTC
There are at least three statements in my previous comment, so I'm not sure which one you're saying is not true. To my mind they're all self-evident but you obviously dispute at least one of them.

The " " label is used to identify the unchanged state. We could use "" but then there is more likely to be ambiguity as to what was intended. Another possiblity is something like "<Unchanged>" which would work just as well, but gives more characters to delete.

I don't think that adding a bug report makes "Kst more over-complicated". It just adds fixed entries to the bug-tracking system, which can be referred to back to when necessary.
Comment 12 Netterfield 2006-07-07 06:00:59 UTC
OK: so I just tried Andrew's patch and read all the email.

Other than minor quibbles (small bugs which can easily be fixed) I think that the interface is basically what we want.

Quibbles:

-unchanged objects all need some visual indication that they have not been changed.  Check boxes currently have the horizontal lines, which is ugly, but works.  Numerical, text, colour and lists have no indication.  Perhaps the same hatching as the background to all unchanged items would be good.  Maybe grey diagonal lines?  Does Windows have a standard for this?  This applies to the data objects as well.

-using " " as 'not changed' for text entries seems like a bad idea, as one might type ' ' to clear it and indicate it as changed, and then....  Maybe typing anything in the entry makes it changed (just like check boxes currently) - otherwise it is unchanged...

-The location of the "Edit Multiple" button is very space inefficient.  Maybe it should be in the top row or bottom row of the dialog.

-changing which plots are selected in the edit-multiple box should re-enable 'apply'.

-We need better tag names!

-Eventually we need to enable this for the (regularized) plot dialog.

I make no comment here on the patch's code - but George has, so I would encourage George to make some suggestions as to how this could be implemented in a clean way.
Comment 13 Netterfield 2006-07-07 06:25:51 UTC
Another feature quibble:

Un-modified numerical entries should have some default value so that up and down do something useful...  Either the value from the most recently selected object in the edit multiple list, or the current default.
Comment 14 Andrew Walker 2006-07-07 20:11:43 UTC
I think all of Barth's points are good ones, but of course they all add further complexity to the code which seems to be exactly what George is arguing against. I've added the two suggestions by Barth that do not add to complexity and will attach it is a revised patch.

After that I'll await some feedback from George as to possible ways of avoiding complexity while attaining the desired functionality.
Comment 15 Andrew Walker 2006-07-07 20:13:37 UTC
Created attachment 16918 [details]
Proposed patch
Comment 16 Andrew Walker 2006-07-07 20:20:06 UTC
One possibility to indicate unchanged values would be to set the foreground colors of unmodified widgets to a color other than black. This would work for all objects other than color selection, where some other technique would be required.

Whichever approach we take the same method would need to be used for the data objects to ensure consistency across the UI.
Comment 17 Netterfield 2006-07-09 00:08:21 UTC
For reference, what points are addressed here?
Comment 18 Andrew Walker 2006-07-10 19:22:00 UTC
-The location of the "Edit Multiple" button is very space inefficient.  Maybe it should be in the top row or bottom row of the dialog. 

-changing which plots are selected in the edit-multiple box should re-enable 'apply'.
Comment 19 Andrew Walker 2006-07-11 22:58:15 UTC
George, do you have any comments on this or should I go ahead and make the necessary changes.
Comment 20 George Staikos 2006-07-11 23:59:44 UTC
On Friday 07 July 2006 14:20, Andrew Walker wrote:
> ------- One possibility to indicate unchanged values would be to set the
> foreground colors of unmodified widgets to a color other than black. This
> would work for all objects other than color selection, where some other
> technique would be required.
>
> Whichever approach we take the same method would need to be used for the
> data objects to ensure consistency across the UI.


  This would be a major accessibility violation.
Comment 21 George Staikos 2006-07-12 00:00:56 UTC
On Tuesday 11 July 2006 16:58, Andrew Walker wrote:

> ------- George, do you have any comments on this or should I go ahead and
> make the necessary changes.


  I need time to review.  I was at KDE Four Core last week and was offline 
until just now.  I'm catching up this week.
Comment 22 Andrew Walker 2006-07-12 00:43:00 UTC
So to avoid the accessibility violation we could switch the foreground and background colors, which would maintain the contrast.
Comment 23 George Staikos 2006-07-12 01:19:16 UTC
On Tuesday 11 July 2006 18:13, Barth Netterfield wrote:
> On Tuesday 11 July 2006 14:59, George Staikos wrote:
> >   This would be a major accessibility violation.
>
> Is this because it is accesability requires high-contrast, which requires 2
> color only?  So you are saying that it is not permitted to use color to
> indicate state in the UI?
>
> Sad, because the alternative of stripes or something is pretty ugly.
> Also surprising, as kde already indicates things as disabled using colour.


   In general using color to distinguish state is really not a good approach.  
The "disabled" color is actually a state displayed by the theme.  A better 
approach is to add an indicator.
Comment 24 George Staikos 2006-07-12 03:57:51 UTC
   I think that the best approach here is to inherit from all of the widgets 
we will need and make "edit multiple mode" versions of them.  This would 
provide a consistent interface to the widgets and hide a lot of the ugliness.  
Then we can change the UI elements from the widget side also.  The big 
problem is that introducing edit multiple makes a mess of the dialogs, 
creates a large potential for introduction of bugs or regressions (we've seen 
this several times already), and generally makes working on the dialogs 
unpleasant.  If we can find a way to solve these problems, then we'll be in 
good shape to add this feature.
Comment 25 Andrew Walker 2006-07-13 01:24:06 UTC
It would seem that we're deciding how to code a feature before we've decided on the functionality. There is no need to subclass these widgets if we don't require any enhanced functionality - and its not yet clear that we do.
Comment 26 George Staikos 2006-07-13 19:12:07 UTC
On Wednesday 12 July 2006 19:24, Andrew Walker wrote:
> ------- It would seem that we're deciding how to code a feature before
> we've decided on the functionality. There is no need to subclass these
> widgets if we don't require any enhanced functionality - and its not yet
> clear that we do. _______________________________________________


   ... but you attached a patch?
Comment 27 Andrew Walker 2006-07-13 19:25:55 UTC
That's right. The patch used the same functionality we have had for the data objects for a long time now. A reasonable approach. 

However, what you're doing is now specifying how we should code this feature after the existing functionality was re-opened for discussion. I think we need to finish the latter discussion before launching into the former.
Comment 28 George Staikos 2006-07-13 20:05:16 UTC
  If that's the case then all of this should be put on hold.  I contacted 
someone from the KDE HCI group to ask about how to make such an interface to 
accomplish the task.  I'll report back when I have an answer.
Comment 29 Netterfield 2006-07-21 02:38:59 UTC
Has any progress been made on understanding the UI?  I don't want to stall 
forever.

There are two examples I am aware of:
1) The configure all fonts sub-dialog from kconfig.  Here, each thing you want 
to change gets a checkbox to indicate you want to change it.  The widgets are 
disabled until their check box is checked.
Good points:
        -already in kde
        -can easily toggle between selected and not selected.
        -obvious what it means
Bad points:
        -not clear how to do check boxes
        -takes up more space
        -as implimented, it takes an extra click.

2) The character dialog in open office:  This appears to be what the dialogs 
in kst are modeled after.  Widgets with properties common to all selected 
objects are filled in with the common property.  Un-selected widgets are 
blank, except for check boxes, which are hatched.
Good points:
        -What we have and no one has complained much
        -Its in openoffice, so people may have seen it before
        -it requires pretty minimal number of key strokes
Bad points:
        -Not clear how to make an entry un-applied once it has been applied.
        -hard to distinguish between an empty text entry, and a non-applied text 
entry.

I propose we use 1, with the following modification:
        -QCheckboxes have a built in tri-state mode: we should use it for checkboxes.

Thoughts?
cbn
Comment 30 Andrew Walker 2006-07-21 02:43:30 UTC
My preference would be 2) for the following reasons:

The UI for 1) will be okay for simple dialogs but rapidly become overwhelming for more complex dialogs. Eventually we'll need to handle edit multiple for the plot dialog. How many checkboxes would the user be faced with per tab?

In most cases I suspect the user will change only one or two properties and will easily be able to keep track of what they have and haven't edited. In the cases where they get confused they just hit Cancel and start again.
Comment 31 Netterfield 2006-07-26 03:43:09 UTC
OK.  From a UI point of view, lacking any other suggestions, we should go with option 2 (which is basically what has been started).

George has suggested that all of the widgets used in edit multiple dialog get subclassed to have edit multiple properties.  I think this is a good idea.

QCheckBoxes already have a tri-state mode.  They should be used (instead of our own tri-state indication) but I'm not sure that the interface is so good.

It would be good to have a mechanism built into the widgets for deciding of all selected objects already share a setting (in which case they will display that property, rather than being tri-stated.)

Andrew, can you propose an API for tristate widgets?
Comment 32 Andrew Walker 2006-07-26 18:31:12 UTC
I would think we would need the functionality defined before the API. The following are the widgets we would need to define the functionality for and subclass under this approach:

QSpinBox
QComboBox
QLineEdit
KColorButton
KURLRequester
KDoubleSpinBox
KFontCombo

I'm still concerned that we appear to have made the decision to sub-class before knowing what functionality we want.
Comment 33 George Staikos 2006-07-27 07:30:34 UTC
  I agree with option #2 with an API that makes it more sane to work with than 
what we currently have.
Comment 34 Netterfield 2006-10-10 14:13:21 UTC
SVN commit 594184 by netterfield:

-Reword 'Apply to..." labels in the viewobject widget.
-Add 'Apply to..." to the legend dialog, to allow the legend properties
to be set to all legends in a window, or in the whole document.

CCBUG: 129281


 M  +53 -17    kstviewlegend.cpp  
 M  +6 -0      kstviewlegend.h  
 M  +14 -11    view2dplotwidget.ui  
 M  +223 -157  viewlegendwidget.ui  
 M  +3 -1      viewlegendwidget.ui.h  
Comment 35 Andrew Walker 2007-06-07 23:37:51 UTC
The Edit Multiple functionality is now present. The only remaining object that needs to be converted is Kst2DPlot, which is addressed in bug report #111114.