GNOME Bugzilla – Bug 695773
Register rewrite: combobox use in transaction number field
Last modified: 2018-06-29 23:14:15 UTC
I recently noticed that the transaction number field in the new register code uses a combobox for editing the field. This is a change compared to the old register code. I'm not sure what the use case for this is. My bank accounts use transaction numbers. With history recorded for a couple of years the dropdown list for the combo box becomes unusable because it has way to many entries in it. Perhaps there is a use case I don't see.
When I started looking at the rewrite, I was under the impression that all cells in each column have the same renderers and as the action field required a combo that was chosen. Am I wrong ? I have only just found the last number kvp and this will be in the next patch so I can think of two easy ways to change this, limit the list to say the last 5 entries or better still just have an entry for the last number so the combo will always have one entry. There will also be the existing option to press + on the blank entry or with it populated from the combo. What do you think ?
As a side note, I reported this bug mostly so we remember the small details. I consider this fine-tuning and not priority. I'll probably post others like this as I notice minor issues. They can be tuned as we go along. (In reply to comment #1) > When I started looking at the rewrite, I was under the impression that all > cells in each column have the same renderers and as the action field required a > combo that was chosen. Am I wrong ? > Do you mean that a GtkTreeView can only handle one cell renderer per column ? Perhaps. I haven't studied this in detail. > I have only just found the last number kvp and this will be in the next patch > so I can think of two easy ways to change this, limit the list to say the last > 5 entries or better still just have an entry for the last number so the combo > will always have one entry. There will also be the existing option to press + > on the blank entry or with it populated from the combo. > > What do you think ? Those are possible work arounds, but in essence I don't think the number field should use a combobox renderer. Where possible I like to give consistent visual feedback to end users. A combo box suggests multiple predefined choices, which is not a sensible hint for the number field. I haven't looked in the code, but aren't we juggling cell renders already ? For example if you switch the focus from a Description cell to a Date cell, the description cell seems to switch from a text edit widget to a label widget and the date cell from label widget to datedtime combo box. Is this something that the GtkTreeView code does automatically ? Or is this something we do in callbacks ? I'm just wondering if it's not possible to set up a different cell renderer based on the row type. We do already change the column headings based on whether the active row is a transaction row, a split row or the second line of a double line entry. Can't we use similar code to define the desired cell renderer on the fly ?
(In reply to comment #2) As I understand it, each column can have multiple renderer's which are shared across all rows in that column. There is a clear function that removes all renderer's and then you can add new ones but not sure about that. What I have discovered is you can hide renderer's based on the model which I have tested on a test tree application and seems to work. So what we can have for the Number/Action column is two renderer's, first being a text renderer with entry and the second is a combo box with entry with there visibility set by the cell data function. I will have a look at this when I look at the hot keys for this column.
Created attachment 239275 [details] [review] Register rewrite patch This patch changes the the Number / Action column from a single combo renderer to two, text renderer and combo renderer both with entry. We use the model to decide which one will be visible for each row. Also set up is the number accelerator key. This patch needs to be applied after the one in bug 696152.
Comment on attachment 239275 [details] [review] Register rewrite patch Thank you for your patch. This one seems to work pretty well, except for a small cosmetic issue. I would have applied it if not for the crasher in the previous patch (update11). The minor issue is in the keyboard accelerator for the transaction number field: When you click on any one cell, the first thing that happens is that the whole line is selected, but no cell is switched to an editable renderer. That only happens when you click a second time in a cell. So suppose my first click is on a transaction number cell. This highlights the whole transaction row (the old register behaviour would be to go into edit mode directly for that cell). If you now type '+' to insert the next auto-incremented value, the row goes into multi-line mode instead of inserting the next number. Hitting '-' switches back to single-line mode. If you click a second time in the number cell, it goes into edit mode and now the + and - keys work as expected. I think the core issue is that clicking a cell should immediately go into edit mode (which is the behaviour of the old register code). That is perhaps the work of an independent patch, which would obsolete this issue here.
Interesting how different people view things, I did not really notice the two clicking for the entries, one to highlight the row, the second to allow entry for that cell. I had a look at changing this, it is very easy to enable for general navigation but blows up the 'transaction_changed_confirm' function when you cancel or discard. Looks like this whole area will need to be changed after much thought so will have to wait.
Comment on attachment 239275 [details] [review] Register rewrite patch Doesn't apply on current trunk: Hunk #11 in gnc-tree-model-split-reg.c and hunk #17 in gnc-tree-view-split-reg.c.
Never mind about not applying; I hadn't applied patch 11.
Bob, did your last patch (#18) fix the double click issue ? It seems so. I only needed to click once to start editing fields. If so, this bug can be closed.
Forgot to update this one, yes it is fixed so can be closed.
GnuCash bug tracking has moved to a new Bugzilla host. This bug has been copied to https://bugs.gnucash.org/show_bug.cgi?id=695773. Please update any external references or bookmarks.