patchPSPP - Patches: patch #6376, crosstabs dialog

 
 

You are not allowed to post comments on this tracker with your current authentication level.

patch #6376: crosstabs dialog

Submitter:  Jason H Stover <jstover>
Submitted:  Tue 15 Jan 2008 04:31:49 AM UTC
   
 
Category:  None Item Group:  None
Status:  Done Assigned to:  jstover
Open/Closed:  Closed

Jump to the original submission

Fri 08 Feb 2008 09:18:44 AM UTC, comment #12: 

I think this is ready to check in.

Can you change "2007" to "2008" in crosstabs-dialog.h

John Darrington <jmd>
Group administrator
Thu 07 Feb 2008 03:38:05 PM UTC, comment #11: 

Latest version attached.

> 1. Can you set the "shadow-type" property of the GtkFrame to "Etched In".


Done.

> 2. When I select the "None" checkbox in the Cells subdialog, the syntax is generated has /CELLS = CELLS_NONE but when I run this syntax, it's rejected by the parser. Similarly for the Statistics subdialog


I thought I had fixed that some time ago, but I guess not.
It should work now.


(file #14985)

Jason H Stover <jstover>
Group Member
Thu 07 Feb 2008 10:01:03 AM UTC, comment #10: 

1.  Can you set the "shadow-type" property of the GtkFrame to "Etched In".

2.  When I select the "None" checkbox in the Cells subdialog, the syntax is generated has /CELLS = CELLS_NONE  but when I run this syntax, it's rejected by the parser.  Similarly for the Statistics subdialog.

John Darrington <jmd>
Group administrator
Wed 06 Feb 2008 05:30:55 PM UTC, comment #9: 

Latest patch attached. I fixed all three items.


(file #14974)

Jason H Stover <jstover>
Group Member
Wed 06 Feb 2008 11:12:21 AM UTC, comment #8: 

1. I've found that putting radiobuttons in Vbuttonboxes causes problems.  Use an ordinary GtkVbox instead.

2. Put the buttons into a GtkFrame, like its done on the Frequencies Format subdialog (and others).

3. If you change the order of the two calls to psppire_selector_set_subjects, I think it'll solve the other issue.

John Darrington <jmd>
Group administrator
Wed 06 Feb 2008 02:39:07 AM UTC, comment #7: 

OK, latest version of the crosstabs dialog is attached. I addressed each previous critique (and some no one else found), except this one:

'1. When I double click on a row in the left hand treeview, the
variable gets inserted into the lower (columns) box. I suppose it really doesn't matter, but I'd expected it to go into the upper (rows) box. '

This was intentional, for the benefit of users in the Southern Hemisphere. But I can change if you like, as soon as I figure out how.


(file #14966)

Jason H Stover <jstover>
Group Member
Tue 05 Feb 2008 11:59:23 AM UTC, comment #6: 

It looks good.

Some minor points:

1.  When I double click on a row in the left hand treeview, the
 variable gets inserted into the lower (columns) box.  I suppose it really doesn't matter, but I'd expected it to go into the upper (rows) box.

2.  Like you say, the horizontal scrollbars can go.  Also the column headings can be made invisible I think.


3.  With regard to the ComboBox, I'm not sure what's going on here.  It looks to me that glade is being stupid again.  Anyway, as there are only three items, I suggest that we have a group of three radiobuttons instead.  This would certainly be more consistent with the rest of the gui.

John Darrington <jmd>
Group administrator
Tue 05 Feb 2008 05:24:22 AM UTC, comment #5: 

I finally fixed most of the problems John mentioned.

The only one I'm not sure how to fix is this one:

"6. Upon initially opening the Format subdialog, the combobox has no value selected. Is this correct behaviour? "

I wasn't sure how to do this in glade. I clicked all the clicky glade buttons, and typed text in the glade boxes, but it didn't seem to matter. No words appeared on the combobox.

Oh, and I just realized I forgot to drop the horizontal scroll bars. Arg!

Aside from those matters, how does it look?


(file #14965)

Jason H Stover <jstover>
Group Member
Sun 20 Jan 2008 05:55:45 AM UTC, comment #4: 

1.  The Cells and Formats dialogs have no window title.

2.  For consistency with other dialogs, buttons which open subdialogs should have "..." appended to their labels.

3.  In subdialogs, when I click Cancel, I would expect all the changes I've made since opening the dialog to be discarded.  That is to say, the next time I open the subdialog the contents should be as if I had never touched it.  But the Cells and Statistics subdialogs, don't behave this way.
Implementing this is easiest if you have the dialog operate on a temporary GtkListStore, which gets copied into the real one, iff the dialog is closed with Continue.
There's a function in helper.c called clone_list_store to help you do this.

4.  Conversly the Format subdialog doesn't remember the state after I click Continue.

5.  I suggest that the Format subdialog would look better as a single vbox, instead of two juxtaposed vboxes.  Also, the combobox's Expand packing property would be better set to False.

6.  Upon initially opening the Format subdialog, the combobox has no value selected.  Is this correct behaviour?

7.  I'm not sure that the horizontal scrollbars are needed for the Statistics and Cells treeviews.

8.  In the Statistics and Cells subdialogs, I think that having both a label and a visible header in the treeviews is redundant.? One or the other would suffice.

9.  In the code, the copyright year should be changed to 2008

10. I'm getting a few warnings when I compile.

11. In dialog_state_valid, shouldn't & be && ?

John Darrington <jmd>
Group administrator
Sun 20 Jan 2008 04:01:07 AM UTC, comment #3: 

Here is an updated patch. I incorporated all but the one aforementioned suggestion from John, and fixed the handling of the enum's.


(file #14842)

Jason H Stover <jstover>
Group Member
Wed 16 Jan 2008 05:14:34 PM UTC, comment #2: 

I incorporated the suggestions, except the fifth:

"5. I'm not sure that it wouldn't be better to have a single "Options" dialog instead of three seperate ones. Perhaps just stick the contents of each into a cell of a hbox? "

I could add the format, cell and stats options in a single window, but didn't because the other program has them in separate windows. Does anyone have any preference?

I haven't posted an updated patch because I found a bug in the syntax generator, and it looks like I'm going to have to rewrite quite a bit of the code. I had just copied the technique in frequencies-dialog.c of using enum's and preprocessor #'s to handle the subcommands, but that doesn't work so easily in crosstabs, in which NONE can refer to either CELLS or STATISTICS. So I'm wondering what would be a better way. I will post an updated patch after I fix it.

Jason H Stover <jstover>
Group Member
Tue 15 Jan 2008 05:43:43 AM UTC, comment #1: 

I reworked the main dialog a little, using a table instead of [vh]boxes, and I think the result is simpler and more aesthetically pleasing.  New glade file is attached.

The subdialogs still need attention:
1. They should all have the set_transient_for property.
2. The tree-views should be in scrolled windows, otherwise the treeview widget could become too large (if for instance a very large font was the user's default).
3. Bold text in the frame labels is inconsistent with the rest of the GUI. Use plain text.
4. They should all have appropriate window titles.
5. I'm not sure that it wouldn't be better to have a single "Options" dialog instead of three seperate ones.  Perhaps just stick the contents of each into a cell of a hbox?



(file #14803)

John Darrington <jmd>
Group administrator
Tue 15 Jan 2008 04:31:49 AM UTC, original submission:  

The attached patch implements a dialog for CROSSTABS.

Jason H Stover <jstover>
Group Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attached Files
file #14985:  crosstabs-4.patch added by jstover (43KiB - text/x-diff)
file #14974:  crosstabs-3.patch added by jstover (42KiB - text/x-patch)
file #14966:  crosstabs-2.patch added by jstover (42KiB - text/x-diff)
file #14965:  crosstabs-1.patch added by jstover (40KiB - text/x-diff)
file #14842:  crosstabs-dialog-0.patch added by jstover (39KiB - text/x-diff)
file #14803:  crosstabs.glade added by jmd (22KiB - application/x-glade)
file #14802:  crosstabs-dialog.patch added by jstover (42KiB - text/x-diff)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by jmd (Updated the item)
  • -email is unavailable- added by jstover (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 10 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2008-02-08 jstover StatusReady For Test/Review Done
        Open/ClosedOpen Closed
    2008-02-07 jstover Attached File- Added crosstabs-4.patch, #14985
    2008-02-06 jstover Attached File- Added crosstabs-3.patch, #14974
    2008-02-06 jstover Attached File- Added crosstabs-2.patch, #14966
    2008-02-05 jstover Attached File- Added crosstabs-1.patch, #14965
    2008-01-20 jstover Attached File- Added crosstabs-dialog-0.patch, #14842
    2008-01-15 jmd Attached File- Added crosstabs.glade, #14803
        Assigned toNone jstover
    2008-01-15 jstover Attached File- Added crosstabs-dialog.patch, #14802

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code