> On Jan. 23, 2013, 12:32 p.m., Alessandro Russo wrote: > > When I added the tags I followed as example the code for memo and payee > > fields, so if that line of code was wrong for the memo and payee fields it > > will be wrong also for the tag field. > > Allan Anderson wrote: > The patch I proposed takes care of the enabling of the OK button issues, > including those with TAGs, but I think you'll find that the other points I > raised may still exist. > > I'm just about to push this fix, so we'll soon see. > > Allan Anderson wrote: > Erm, there still is a problem with the OK button, to do with the amount > and memo fields, which I thought I'd fixed as part of the patch. I'll need > to look again. > > So far as Tags are concerned, I've assumed that they would function in > the same way as the payee field. Enter a new name, then move focus to > another field. A dialog pops up to ask if a new payee should be created. > With a new tag, however, when focus is moved to another field, the new name > is cleared and no popup appears. > > Whilst this may be what was intended, isn't it a bit inconsistent? > > > Allan Anderson wrote: > Sorry, false alarm. I had to break off, and when I returned, I found > there is not still a problem with the OK button, so far as I can see. I'd > looked at the code and all the calls to slotUpdateButtonState() in that area > had been removed. I started to think that it must be getting called from > elsewhere, but it wasn't. > > I then realised that I could no longer produce that problem. So, what I > had done, I don't now know, but possibly had reverted to a pre-patched > version. Apologies again. > > The Tag issue still is there, and there is a variation. If I create a > new tag, then select that in a new schedule, then click in another field, the > selection disappears, both from the line-edit and from the drop-down. > Clicking the red 'x' restores it to the drop-down, which gets us back to > square one. > > > Alessandro Russo wrote: > The intended behavior of tag field is the following: After you start > typing the tag in the tag field a drop down list show up with all the tag > names that match the characters typed. When you stop typing and click on > another field (or press the Tab key) if a tag with that name exists than it > will be added after the tag field with a little red 'X' on the left and the > name of the tag; the name of this tag will be removed from the combo box list > (you can't insert the same tag two times), if you click on the red 'x' the > tag will be removed from the transaction and it will be re-added to the combo > box (so that you can re-add it if you change idea). If the name that you > wrote in the tag field is different from the names of existing tags a new > dialog will appear (like the payee field), there you can choose to create a > new tag or to close it. If you create the tag it will be automatically added > to the transaction. > > Allan Anderson wrote: > Yes, I can see it works that way in Ledger view, but not in Schedules. > I have created a new tag called 'New Tag', and in the schedule dialog, > I type a 't', and the drop-down appears showing 'New Tag'. I continue to > type > 'ag' and 'New Tag' still shows. If I now type a 'u', the drop-down > disappears. > So far, so good. Now, and this is where it differs, I have 'tagu' in the > line-edit, > and if I now click in the memo field, or any edit field in the > transaction tab widget, > but not in the schedule name box, 'tagu' disappears. Clicking in the > schedule name box > appears to cause the tag to be 'accepted'. In no way can I get the > dialog to appear > asking if I want to accept the new tag. > > I was expecting it to function as for a new payee or category, which it > does in the ledger, > but not in the new schedule. > > Allan > > Alessandro Russo wrote: > Hum.. so maybe there is a bug .. I haven't used the schedules too much... > In the weekend I'll make some tests. > > Marko Käning wrote: > BTW, is it a bug or a feature that the editor doesn't allow to edit > overdue payments of schedules? > > Alessandro Russo wrote: > Ok, i found the problem. Here there is the fix, can you modify your patch > or should I open a different review request? > diff --git a/kmymoney/dialogs/keditscheduledlg.cpp > b/kmymoney/dialogs/keditscheduledlg.cpp > index 7ddf3c3..f3146f3 100644 > --- a/kmymoney/dialogs/keditscheduledlg.cpp > +++ b/kmymoney/dialogs/keditscheduledlg.cpp > @@ -201,6 +201,7 @@ TransactionEditor* KEditScheduleDlg::startEdit(void) > connect(MyMoneyFile::instance(), SIGNAL(dataChanged()), editor, > SLOT(slotReloadEditWidgets())); > // connect(editor, > SIGNAL(finishEdit(KMyMoneyRegister::SelectedTransactions)), this, > SLOT(slotLeaveEditMode(KMyMoneyRegister::SelectedTransactions))); > connect(editor, SIGNAL(createPayee(QString,QString&)), kmymoney, > SLOT(slotPayeeNew(QString,QString&))); > + connect(editor, SIGNAL(createTag(QString,QString&)), kmymoney, > SLOT(slotTagNew(QString,QString&))); > connect(editor, > SIGNAL(createCategory(MyMoneyAccount&,MyMoneyAccount)), kmymoney, > SLOT(slotCategoryNew(MyMoneyAccount&,MyMoneyAccount))); > connect(editor, > SIGNAL(createSecurity(MyMoneyAccount&,MyMoneyAccount)), kmymoney, > SLOT(slotInvestmentNew(MyMoneyAccount&,MyMoneyAccount))); > connect(MyMoneyFile::instance(), SIGNAL(dataChanged()), editor, > SLOT(slotReloadEditWidgets())); > diff --git a/kmymoney/dialogs/kenterscheduledlg.cpp > b/kmymoney/dialogs/kenterscheduledlg.cpp > index 9f4432f..04752f0 100644 > --- a/kmymoney/dialogs/kenterscheduledlg.cpp > +++ b/kmymoney/dialogs/kenterscheduledlg.cpp > @@ -218,6 +218,7 @@ TransactionEditor* KEnterScheduleDlg::startEdit(void) > connect(MyMoneyFile::instance(), SIGNAL(dataChanged()), editor, > SLOT(slotReloadEditWidgets())); > // connect(editor, > SIGNAL(finishEdit(KMyMoneyRegister::SelectedTransactions)), this, > SLOT(slotLeaveEditMode(KMyMoneyRegister::SelectedTransactions))); > connect(editor, SIGNAL(createPayee(QString,QString&)), kmymoney, > SLOT(slotPayeeNew(QString,QString&))); > + connect(editor, SIGNAL(createTag(QString,QString&)), kmymoney, > SLOT(slotTagNew(QString,QString&))); > connect(editor, > SIGNAL(createCategory(MyMoneyAccount&,MyMoneyAccount)), kmymoney, > SLOT(slotCategoryNew(MyMoneyAccount&,MyMoneyAccount))); > connect(editor, > SIGNAL(createSecurity(MyMoneyAccount&,MyMoneyAccount)), kmymoney, > SLOT(slotInvestmentNew(MyMoneyAccount&,MyMoneyAccount))); > connect(MyMoneyFile::instance(), SIGNAL(dataChanged()), editor, > SLOT(slotReloadEditWidgets())); > > You simply need to add the following line: > connect(editor, SIGNAL(createTag(QString,QString&)), kmymoney, > SLOT(slotTagNew(QString,QString&))); > after the similar line with createPayee in the files > dialogs/keditscheduledlg.cpp and dialogs/kenterscheduledlg.cpp > > (I fixed the same bug in the Enter Scheduled dialog)
My patches are committed already, but it doesn't make sense really to start a new review, having got this far, so I've added your changes here. If I type in a new tag name and then click in one of the edit fields, the dialog now appears asking whether to add it. It doesn't appear though if I focus on the schedule name, or widgets not in the transaction area, or click outside the transaction widgets. If I am creating a new schedule, and type in all the fields, the entries remain visible in the line edit boxes. This isn't the case with a new tag. If I type in a new tag name and accept it, it gets cleared. I have to click the red cross then select it from the combo. With the name now showing, however, if I click the memo field it goes again. It also is possible to get more than one red cross visible, together with the added tag name alongside. If I have added a new tag to a schedule, and then decide to delete that tag, and it is the only tag, I get a message saying I can't delete the last tag as there needs to be one to reassign to. I understand that, but if I have a bunch of schedules, how do I find which is the one referencing that tag, to allow me to remove it? Search transaction shows the tag, but selecting it doesn't enable Find. Apologies if I seem 'picky'. I didn't set out to undermine your good work, and it is quite a big change. - Allan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107714/#review26063 ----------------------------------------------------------- On Jan. 12, 2013, 12:07 p.m., Allan Anderson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/107714/ > ----------------------------------------------------------- > > (Updated Jan. 12, 2013, 12:07 p.m.) > > > Review request for KMymoney. > > > Description > ------- > > The problem as originally reported was that in Schedules view, the OK button > became enabled even though no schedule name had been entered. > > It was found that the button became enabled as soon as a payee was entered. > It was also found that this happened when an amount was entered. > > For "payee", line 753 of transactioneditor.cpp has - > "connect(payee,SIGNAL(textChanged(QString)),this,SLOT(slotUpdateButtonState()))", > and slotUpdateButtonState() has - > "emit transactionDataSufficient(isComplete(reason)", > and 'This signal is sent out whenever enough data is present to enter the > transaction into the ledger.' > > Similarly, for "amount", at line 826, the same line appears. > > As neither of these fields is a mandatory one, I believe they should not > affect the OK button status. So, as shown in the patch, I have temporarily > disabled these lines. I have done numerous tests of schedule creation and > editing, and manual entry and editing of transactions without any problem. > > The same area of code in transactioneditor.cpp has several more of these > possibly unneeded lines, although not affecting schedules. For instance, > even with these two lines out and with no mandatory fields completed, if a > payee is selected and the memo, tag field, next due date or status is edited, > the OK button again is enabled wrongly. > > I don't really see any valid reason for 'slotUpdateButtonState()' to be in > this section. What do the wise men think? > > > This addresses bug 311481. > http://bugs.kde.org/show_bug.cgi?id=311481 > > > Diffs > ----- > > kmymoney/dialogs/transactioneditor.cpp 8f6f06b > > Diff: http://git.reviewboard.kde.org/r/107714/diff/ > > > Testing > ------- > > Numerous tests of schedule creation and editing, and manual entry and editing > of transactions without any problem. > > > Thanks, > > Allan Anderson > >
_______________________________________________ KMyMoney-devel mailing list KMyMoney-devel@kde.org https://mail.kde.org/mailman/listinfo/kmymoney-devel
