GNOME Bugzilla – Bug 104249
"New Constant" dialog comments
Last modified: 2004-12-22 21:47:04 UTC
- The widget packing is bit odd... the wider you make the window, the bigger a gap you get down the right hand side, and the OK and Cancel buttons get bigger, which should generally never happen. This window probably just shouldn't be resizable. - OK and Cancel buttons should be right-aligned in the dialog, not centred - All the controls should have mnemonics - "Constant no:" is a bad label... shouldn't use abbreviations in labels anyway, but this is confusing because it's not clear what the 'constant number' is, or what the acceptable range of inputs is... it could easily be confused for the 'value'. A better solution here would be a drop down list showing all the currently defined constants, from which you can choose which slot to fill/overwrite. (If there is no limit to the number of constants you can define, the last entry on this list would be a new, empty slot that you could select.)
Just realised these comments apply to the "Add function" window too. Both windows should probably also have a left-aligned Help button, but especially the Add Function window where the user is given no clue as to what format a function might take.
Calum, thanks for agreeing to add a screenshot of the desired new look to this bug in a day or two. With that we should easily be able to implement what you want.
Here's a quick mockup of what the Add Constants window might look like then... thought an editable list view might be nicer in the end, provided you can specify only the second and third columns are editable...
Created attachment 14000 [details] Proposed redesign of 'Add Constant' dialog
Thanks Calum! A couple further questions before I start implementing this. As we discussed, the window should be slightly taller so that there is no need for a vert. scroll bar, but what about the UI for the following two scenerios: 1/ the user has entered an invalid value (say a constant of 123.123.123)? 2/ the constant/function already exists. With the current enter new con./fun. popups, either of these two situations results in a popup with a description of what the situation is, and cancel and okay buttons. Something like: Constant 4 already exists. Okay to overwrite? [Ok] [Cancel] Now I presume that with the new layout design, as the user can see that a particular con/fun slot already has a value, that it is always okay to overwrite without asking the user, so that problem goes away, but what about if they enter an invalid value? The only invalid value worth checking at this point, is for constant definitions, so perhaps the best solution here is, as the user is typing a constant value in, check to see if what they have typed constitutes a valid number. If not, then beep and prevent invalid keystrokes from being entered. Whadya think?
I agree, there's no need to alert the user that a slot is already filled with this design. As for invalid values... yes, you need to warn the user somehow, I don't believe the HIG specifies which method is preferable though. Personally speaking, I get irritated by only being allowed to type certain characters and being beeped at when I press anything else, so I'd rather have a popup when I pressed enter or otherwise moved focus out of the field. On acknowledging the popup, focus would return to the field and in this case, it would already be set to 'edit' mode so the user could fix the mistake immediately. However, it certainly wouldn't be wrong to do it the other way you suggested, and in some ways it's better, so I guess the solution is to choose whichever one you're more comfortable implementing :)
Created attachment 14102 [details] Partial patch.
Adding Dennis to the CC:. I've attached a partial patch for the changes required to provide the new functionality. I need some help getting it all to work "as advertised". The remaining (hopefully small) problems are: 1/ The buttons on the con/fun popup window aren't aligned as per the screenshot from Calum. 2/ I can't work out the magic to make the second and third columns editable, or I'm not understanding how you get them into an editable state (I just assumed you clicked on them). I suspect the former. My attempt to set an editable state is: if (editable) { GTK_CELL_RENDERER(renderer)->mode = GTK_CELL_RENDERER_MODE_EDITABLE; } in the add_cf_column() routine in gtk.c 3/ The cell_edited routine needs to be checked to see if it will get called if the cells were editable, and that it does the right thing. One thing that will need to be added it to check that a constant value that is entered is a valid number and reject it if it's not. 4/ The Help button needs to be hooked up. Any suggestions on how to do any of these would be very much appreciated.
Rich, I have a working version of the 'Edit Constants' window at home. I want to clean up the code then I will attach a patch to this bug report either tonight or tomorrow.
Excellent. Dennis. Thanks. I'll concentrate on the other gcalctool bugs today.
Created attachment 14181 [details] [review] Suggested patch for the 'Edit Constants' dialog.
Hi Rich, Sorry I took a while to respond. I hope the answers below help point you in the right direction. >1/ The buttons on the con/fun popup window aren't aligned as per > the screenshot from Calum. See gtk_dialog_new_with_buttons(). >2/ I can't work out the magic to make the second and third columns > editable, or I'm not understanding how you get them into an > editable state (I just assumed you clicked on them). I suspect > the former. My attempt to set an editable state is: > >if (editable) { > GTK_CELL_RENDERER(renderer)->mode = GTK_CELL_RENDERER_MODE_EDITABLE; >} Include the "editable" attribute when calling gtk_tree_view_insert_column_with_attributes(). >3/ The cell_edited routine needs to be checked to see if it will > get called if the cells were editable, and that it does the right > thing. One thing that will need to be added it to check that a > constant value that is entered is a valid number and reject it > if it's not. The validation code should go into a function attached to the "edited" signal of the renderer. >4/ The Help button needs to be hooked up. > See gnome_help_display(). Is the future plan to merge gcalctool into gnome-utils? BTW -- when I lived in Sunnyvale, my employer at the time enrolled me as a Sun Key member. This meant I took several classes at the Sun training center in Milpitas, and Sun gave me a nice pull-over sports jacket. So my question to you, have I submitted enough patches to earn a new Sun mouse pad? ;-) Later, Dennis
Hi Dennis. Thanks for the patch! I'm a little confused though. The make_constants_value() routine seems to replicate the code in the make_fixed() routine in display.c. Why do we need two, almost identical routines? What does the trim_zeros() routine do, that make_fixed() wasn't doing? Also, from the naming of the routines, I get the impression that your patch seems to be specific to dealing with just constants. Is that correct? This is just from looking at the patch in my browser. When I get to work I'll try it out.
Hi Dennis. I've now had a chance to try the patch, and have found the following things: 1/ I can edit the description for a constant and press the okay button, but it doesn't retain the new value. In other words, when I bring up the constants menu, the old description is still there. 2/ No popup is displayed for Function editing. I will try to work out how you make the columns editable and correctly align the buttons at the bottom, and then update the original patch for that. I'll also try to work out why it's not saving the new description. Thanks. I think I've got enough information here to finish this.
>1/ I can edit the description for a constant and > press the okay button, but it doesn't retain > the new value. In other words, when I bring up > the constants menu, the old description is still > there. Strange ... it works for me. I can edit both the value and description and the changes are saved and reflected when I press the OK button. >2/ No popup is displayed for Function editing. I didn't implement the Edit Function dialog. >I will try to work out how you make the columns editable >and correctly align the buttons at the bottom, and >then update the original patch for that. I'll also >try to work out why it's not saving the new description. You should really be using gtk dialogs for the windows. I see you prefer to create a regular window and then hide and show it. This is not how other gnome apps are coded. Just create and destroy a gtk dialog. It would make your code easier to understand and debug. Thanks.
Actually, I'm taking your dialog approach for my new patch. I just wasn't aware of it (being new to Gtk+ ;-) Combined patch almost done (plus function handling as well). I should be checking in a fix this afternoon. PS: Working on your Sun mouse pad... ;-)
Fixed in v4.2.45 of gcalctool. Calum, could you check this out please and give it a try? There might be some slight adjustments needed; I've given it minimal testing. If there is anything not to your liking, please just add a comment to this bug and reopen it. Dennis, thanks again for your great help on this.
I've had a play with it and it looks good.