| Summary: | Need a way to simultaneously edit the properties of multiple view objects. | ||
|---|---|---|---|
| Product: | [Applications] kst | Reporter: | Netterfield <netterfield> |
| Component: | general | Assignee: | kst |
| Status: | RESOLVED FIXED | ||
| Severity: | wishlist | ||
| Priority: | NOR | ||
| Version: | 1.x | ||
| Target Milestone: | --- | ||
| Platform: | unspecified | ||
| OS: | Linux | ||
| Latest Commit: | Version Fixed In: | ||
| Sentry Crash Report: | |||
| Attachments: |
Proposed patch
Proposed patch |
||
|
Description
Netterfield
2006-06-17 04:43:45 UTC
Yes, please :-) Sounds good to me ! 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.
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. 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 :-) 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. 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.
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. 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 ? On first thought it sounds good to me. 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. 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. 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. 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. 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. Created attachment 16918 [details]
Proposed patch
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. For reference, what points are addressed here? -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'. George, do you have any comments on this or should I go ahead and make the necessary changes. 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. 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.
So to avoid the accessibility violation we could switch the foreground and background colors, which would maintain the contrast. 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. 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. 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. 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?
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. 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. 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 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. 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? 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. I agree with option #2 with an API that makes it more sane to work with than what we currently have. 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 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. |