Bug 125873 - Improvements to the "ordered" datawizard
Summary: Improvements to the "ordered" datawizard
Status: RESOLVED FIXED
Alias: None
Product: kst
Classification: Applications
Component: general (show other bugs)
Version: 1.x
Platform: unspecified Solaris
: NOR wishlist
Target Milestone: ---
Assignee: kst
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-04-19 11:37 UTC by Nicolas Brisset
Modified: 2006-05-05 02:35 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Nicolas Brisset 2006-04-19 11:37:32 UTC
Version:           1.3.0_devel (using KDE 3.4.0, compiled sources)
Compiler:          gcc version 3.4.3
OS:                SunOS (sun4u) release 5.8

Another two usability requests for the new datawizard:
1) implement D&D in _vectorsToPlot to allow reorganizing curves more easily than clicking the buttons
2) create the new plots column-wise instead of row-wise. This is linked with the fact that very often, the need to order plots comes from the need to look at groups of variables in a "synchronous" way (and scanning the column vertically allows to see directly the interrelations, while it is much more complicated to visualize it when the variables are in a same row). It is quite difficult to order the variables for a column-wise result when the list shown to the user is used to create the plots row-wise.

I hope point 2) issomewhat clear, otherwise just ask for more (clearer) explanations :-)
Comment 1 George Staikos 2006-04-21 15:30:42 UTC
SVN commit 532182 by staikos:

- disable sorting on the plot vectors
- items are appended to the list
- add a checkbox to reorder column-wise

I'm not going to do drag and drop reorder right now.  It's more of a distraction
from important things than anything else.
BUG: 125871
BUG: 125873


 M  +18 -2     datawizard.ui  
 M  +37 -5     datawizard.ui.h  


--- trunk/extragear/graphics/kst/src/libkstapp/datawizard.ui #532181:532182
@@ -425,7 +425,7 @@
                             <enum>AutoOneFit</enum>
                         </property>
                         <property name="selectionMode">
-                            <enum>Extended</enum>
+                            <enum>QListView::Extended</enum>
                         </property>
                         <property name="showSortIndicator">
                             <bool>true</bool>
@@ -515,7 +515,7 @@
                             <enum>AutoOneFit</enum>
                         </property>
                         <property name="selectionMode">
-                            <enum>Extended</enum>
+                            <enum>QListView::Extended</enum>
                         </property>
                         <property name="showSortIndicator">
                             <bool>true</bool>
@@ -933,6 +933,14 @@
                             </widget>
                         </hbox>
                     </widget>
+                    <widget class="QCheckBox">
+                        <property name="name">
+                            <cstring>_orderInColumns</cstring>
+                        </property>
+                        <property name="text">
+                            <string>Order in columns</string>
+                        </property>
+                    </widget>
                 </vbox>
             </widget>
             <widget class="QButtonGroup" row="0" column="1" rowspan="2" colspan="1">
@@ -1515,6 +1523,12 @@
         <receiver>DataWizard</receiver>
         <slot>vectorsDroppedBack(QDropEvent*)</slot>
     </connection>
+    <connection>
+        <sender>_vectorsToPlot</sender>
+        <signal>dropped(QDropEvent*)</signal>
+        <receiver>DataWizard</receiver>
+        <slot>updateVectorPageButtons()</slot>
+    </connection>
 </connections>
 <tabstops>
     <tabstop>_url</tabstop>
@@ -1599,6 +1613,7 @@
     <include location="local" impldecl="in implementation">datarangewidget.h</include>
     <include location="global" impldecl="in implementation">kurlcompletion.h</include>
     <include location="global" impldecl="in implementation">kdebug.h</include>
+    <include location="global" impldecl="in implementation">vectorlistview.h</include>
     <include location="local" impldecl="in implementation">datawizard.ui.h</include>
 </includes>
 <variables>
@@ -1636,6 +1651,7 @@
     <slot access="private">configureSource()</slot>
     <slot access="private">up()</slot>
     <slot access="private">down()</slot>
+    <slot access="private">updateVectorPageButtons()</slot>
     <slot access="private">add()</slot>
     <slot access="private">remove()</slot>
     <slot access="private">clear()</slot>
--- trunk/extragear/graphics/kst/src/libkstapp/datawizard.ui.h #532181:532182
@@ -34,6 +34,7 @@
     _vectorsToPlot->setAcceptDrops(true);
     _vectors->addColumn(i18n("Position"));
     _vectors->setSorting(1);
+    _vectorsToPlot->setSorting(-1);
 
     // No help button
     setHelpEnabled(_pageDataSource, false);
@@ -727,6 +728,23 @@
 	    }
 	}
     }
+
+    // Reorder the vectors if the user wants it
+    if (_orderInColumns) {
+	const KstVectorList lOld = l;
+	const int count = lOld.count();
+	const int cols = signed(sqrt(plots.count()));
+	int row = 0, col = 0;
+	for (int i = 0; i < count; ++i) {
+	    l[row * cols + col] = lOld[i];
+	    ++row;
+	    if (row >= cols) {
+		++col;
+		row = 0;
+	    }
+	}
+    }
+
     // create the data curves
     app->slotUpdateProgress(n_steps, prg, i18n("Creating curves..."));
     KstViewObjectList::Iterator pit = plots.begin();
@@ -1038,6 +1056,8 @@
     cfg.writeEntry("CycleThrough", _cycleThrough->isChecked());
     cfg.writeEntry("CycleExisting", _cycleExisting->isChecked());
     cfg.writeEntry("PlotNumber", _plotNumber->value());
+
+    cfg.writeEntry("OrderInColumns", _orderInColumns->isChecked());
 }
 
 
@@ -1104,6 +1124,7 @@
     _cycleThrough->setChecked(cfg.readBoolEntry("CycleThrough", false));
     _cycleExisting->setChecked(cfg.readBoolEntry("CycleExisting", false));
     _plotNumber->setValue(cfg.readNumEntry("PlotNumber", 2));
+    _orderInColumns->setChecked(cfg.readBoolEntry("OrderInColumns", false));
 }
 
 
@@ -1190,6 +1211,12 @@
 }
 
 
+void DataWizard::updateVectorPageButtons()
+{
+    setNextEnabled(_pageVectors, yVectorsOk());
+}
+
+
 void DataWizard::add()
 {
     QPtrList<QListViewItem> lst;
@@ -1202,11 +1229,15 @@
 	++it;
     }
 
+    QListViewItem *last = _vectorsToPlot->lastItem();
     QPtrListIterator<QListViewItem> iter(lst);
     while (iter.current()) {
-	_vectors->takeItem(iter.current());
-	_vectorsToPlot->insertItem(iter.current());
-	iter.current()->setSelected(false);
+	QListViewItem *item = iter.current();
+	_vectors->takeItem(item);
+	_vectorsToPlot->insertItem(item);
+	item->moveItem(last);
+	item->setSelected(false);
+	last = item;
 	++iter;
     }
 
@@ -1214,7 +1245,7 @@
     if (_vectors->currentItem()) {
 	_vectors->currentItem()->setSelected(true);
     }
-    setNextEnabled(_pageVectors, yVectorsOk());
+    updateVectorPageButtons();
 }
 
 
@@ -1242,7 +1273,7 @@
     if (_vectorsToPlot->currentItem()) {
 	_vectorsToPlot->currentItem()->setSelected(true);
     }
-    setNextEnabled(_pageVectors, yVectorsOk());
+    updateVectorPageButtons();
 
     vectorsDroppedBack(0L); // abuse
 }
@@ -1261,6 +1292,7 @@
 	++it;
     }
     _vectors->sort();
+    updateVectorPageButtons();
 }
 
 // vim: ts=8 sw=4 noet
Comment 2 Nicolas Brisset 2006-04-21 16:53:41 UTC
There must be a small quirk somewhere: items are correctly appended to the end, ordering by columns looks like it's going to work, but at some point something's off by one. I have not quite figure out the problem yet, but it looks like the list iterator goes one too far at some point (resulting in an offset in the order), and the last curve is created twice...
Comment 3 Nicolas Brisset 2006-04-21 16:54:25 UTC
I forgot to mention it, but even when the order is not set to column-wise, there is a problem.
Comment 4 George Staikos 2006-04-22 07:04:32 UTC
Haven't seen this so far.  Any specific way to reproduce it?
Comment 5 Nicolas Brisset 2006-04-28 11:51:23 UTC
To reproduce it, use the test file I will attach shortly, and do the following:

1) kst -w test.dat
2) select Var1->6 in that order
3) select INDEX as X vector
4) choose 3 columns, order in columns
5) Hit "Finish": you get the order [1,3,5;4,6,6] instead of [1,3,5;2,4,6]

Now, try to get [1,2,3;4,5,6] with the same procedure by just deselecting "order in columns": you get [1,3,5;4,6,6] again.

And while we are at it, I think the extended selection mode for the _vectors listbox is not as good as the multiple selection mode there used to be as it requires extra keyboard input (Shift/Ctrl) while you really rarely need the extra flexibility of extended vs multiple. I'd vote to revert to multiple.
Comment 6 Andrew Walker 2006-05-02 20:18:31 UTC
There are at least a couple of problems with the following piece of code from datawizard.ui.h:

    // Reorder the vectors if the user wants it
    if (_orderInColumns) {
	const KstVectorList lOld = l;
	const int count = lOld.count();
	const int cols = signed(sqrt(plots.count()));
	int row = 0, col = 0;
	for (int i = 0; i < count; ++i) {
	    l[row * cols + col] = lOld[i];
	    ++row;
	    if (row >= cols) {
		++col;
		row = 0;
	    }
	}
    }

1) _orderInColumns is a widget and so will always be non-NULL. This should probably read _orderInColumns->isChecked() so that the vectors are only re-ordered if the box is checked
2) the index of l calculated by l[row * cols + col] does not map one-to-one.
If we assume (for example) that cols = 2 then we get the sequence 0 2 1 3 2 4,
which causes the problem decribed by Nicholas.

Comment 7 George Staikos 2006-05-03 22:03:18 UTC
SVN commit 537055 by staikos:

fix the order-by-column code.  I still don't really like it but I guess it
works as well as it can without reworking the plot layouting code yet again.
BUG: 125873


 M  +10 -4     datawizard.ui.h  


--- trunk/extragear/graphics/kst/src/libkstapp/datawizard.ui.h #537054:537055
@@ -730,17 +730,23 @@
     }
 
     // Reorder the vectors if the user wants it
-    if (_orderInColumns) {
+    if (_orderInColumns->isChecked()) {
 	const KstVectorList lOld = l;
 	const int count = lOld.count();
 	const int cols = signed(sqrt(plots.count()));
+	const int rows = cols + (count - cols * cols) / cols;
+	int overflow = count % cols;
 	int row = 0, col = 0;
 	for (int i = 0; i < count; ++i) {
 	    l[row * cols + col] = lOld[i];
 	    ++row;
-	    if (row >= cols) {
-		++col;
-		row = 0;
+	    if (row >= rows) {
+		if (overflow > 0) {
+		    --overflow;
+		} else {
+		    ++col;
+		    row = 0;
+		}
 	    }
 	}
     }
Comment 8 Andrew Walker 2006-05-05 02:35:49 UTC
The plo and "Differentiate between Curves" dialogs have very similar functionality. in terms of moving list items from one list to another.

It would be nice to keep the icons/text on the buttons consistent across the dialogs.

it would also be nice to have a more consistent approach to the layout of the buttons.