In order to fix some bugs i did significant changes in TDBLookup[List|Combo]Box controls

I did a lot of testing but may broken something

Please report problems you have

Luiz

I will if I find some.
Thanks.

Marcos Douglas

I found a bug in TDBLookupComboBox
To reproduce:
1. Define properties AutoComplete and AutoDropDown to True;
2. Type a new value (the DropDown will open) that not exists in DataSet (list);
3. If you type ENTER the combo will clear, because no value on list was found (OK);
4. Repeat #2. Now if you type [Down] to select a item (use the keyboard and don't type ENTER), but after type a new value that not exists in DataSet list, the combo is not cleaned.

Marcos Douglas

Bugtracker http://bugs.freepascal.org/view.php?id=20886

Marcos Douglas

Don't know if this is in the scope of the changes you made but the lookup in the dbgrid (picklist) is (still?) broken. Take the test program used in http://bugs.freepascal.org/view.php?id=20841 and change the Name in Lookup grid using the dropdown box inside the grid. Selecting second 'xxx' selects MasterID 1. Same problem the way around: when entering 3 in MasterID and then opening dropdownbox in DBGrid positions the selection to the first 'xxx'.

Selection from DBLookupCombo is fine.

A difference with Delphi is the fact that the dataset cursor of the ListSource doesn't change to the selected record when selecting an item in the ListSource. When changing the selected item in a lookup control (including dbgrid picklist) all lookup controls using the same ListSource are updated to the new record, except for the dataset itself. This behavior doesn't make sense.

Ludo

AFAIK this is the same behavior as before my changes.

It's a drawback from the decision to allow use the dropdown style (Delphi TDBLookupControl allows only dropdownlist, so no reference here what's the best approach)

I think is not a good idea to clear the text the user entered. In the other side the control can have inconsistent values compared to dataset that is also not good.

Let the other DBLookup users do an opinion. If they think this is the correct behavior i can implement that (clearing the text that does not exist in the list).

For myself, i prefer to use dropdownlist, since the chance to user be confused is smaller.

BTW:
What's your widgetset?
Pressing enter in either cases under win32 does not clear the content here. Just select the text

Luiz

No. I compared 2 apps using r34068 and r34129 and they have a different comportment.

It's a drawback from the decision to allow use the dropdown style (Delphi TDBLookupControl allows only dropdownlist, so no reference here what's the best approach)

I prefer csDropDownList, but AutoComplete only works using csDropDown! And I need it.

I think is not a good idea to clear the text the user entered. In the other side the control can have inconsistent values compared to dataset that is also not good.

I don't know if is good or not, but before this change the component clean itself if the KeyValue is NULL.

If dropdownlist option works like TRxDBLookupCombo works in Delphi 7... would be very good.

BTW: What's your widgetset?

Only Windows: Lazarus 0.9.31 r34129 FPC 2.5.1 i386-win32-win32/win64

Pressing enter in either cases under win32 does not clear the content here. Just select the text

I think I wrote better in bug tracker:
1. Define properties AutoComplete and AutoDropDown to True;
2. Type a new value (the DropDown will open) that not exists in DataSet (list);
3. If you type ENTER the combo will clear, because no value on list was found (OK);
4. Repeat 0000002. Now if you type [Down] to select a item (use the keyboard and don't type ENTER), but after type a new value that not exists in DataSet list and ENTER. The ComboBox is not cleaned.

Attention in:
+ AutoComplete and AutroDropDown;
+ Item #4: When the dropdown open, just click down (selecting a item) but don't close it; after, type something that do not exists in list and press ENTER. You see the Text typed still there.

Marcos Douglas

I disagree, see http://bugs.freepascal.org/view.php?id=20886

+1

Marcos Douglas

I also noted the different behavior and did not changed. In my opinion the LCL behavior is better since ListSource must be used only as lookup for the data to be displayed and it's cursor should not be changed. It must change only the field to what is linked

In fact all db controls (not only lookup) linked to the same field are changed.

Can you post a project example where this cause a problem?

Luiz

No. DBGrid does not use DBLookupComboBox

Luiz

Still cannot reproduce.

Can you create a sample project and add to the bugtracker?

Luiz

BTW: independent of this, if there's no objection by others, i will implement the feature you requested

Luiz

> Selection from DBLookupCombo is fine. I disagree, see http://bugs.freepascal.org/view.php?id=20886

I referred to the DBLookupCombo behavior in the testcase for http://bugs.freepascal.org/view.php?id=20841. Not as a general statement.

There is actually a very small problem: when selecting a line with MasterID =3 the dropdownbox will select the first xxx instead of the second.

Looking at your issue, I can't reproduce what you are seeing. Entering a non existing value and then pressing enter won't ever clear the TDBLookupComboBox. The DataSource is not updated but the text stays. SVN 34097 win xp 32.

Ludo

Yes, I will... later.

Marcos Douglas

Maybe a property to choose?

In fact all db controls (not only lookup) linked to the same field are changed. Can you post a project example where this cause a problem?

I think this is a problem if we use a code sharing between Delphi and Lazarus.

Marcos Douglas

Thanks, but was like this in the past.
I used in r34068 before update to r34129 today.

Sorry, but when will you do this change? I need to know if I downgrade my revision or espero your patch.

Marcos Douglas

Sorry, correction:
[...]my revision or _wait_ your patch.

I thought in I English but wrote in Portuguese... oh my...

Marcos Douglas

Well, if you use a listsource that is an active datasource for another control, I would expect the 2 to be in sync. If I want a listsource that is only a lookup, then I use a different (and probably smaller containing only 1 or 2 fields, filterd, etc) one. An expectation that is perhaps based on an habit created by Delphi but still...

In fact all db controls (not only lookup) linked to the same field are changed.

Linked to the datasource that is, not the ListSource.

Can you post a project example where this cause a problem?

Ex:Order processing, article detail window opened, customer wants XYZ but "the one with the round corners". Order taker enters XYZ in order screen and scrolls through the different values while looking at detail screen until he finds "round corners" or something similar.
Ex:Order processing, same customer, 2 addresses, 2 ID's. Customer detail window used to check address, contact person or whatever. The same can be done without ListSource cursor updates but not as efficient. It requires more user actions.

Ludo

Well, the componente do not clean[1] itself (sorry about that) but the bug still exists, but otherwise.

[1]  I had a EditingDoneEvent like this:
procedure TFooHelper.ComboEditingDoneCallback(Sender: TObject);
var
  cbx: TDBLookupComboBox;
begin
  if Sender is TDBLookupComboBox then
  begin
    cbx := TDBLookupComboBox(Sender);
    if VarIsNull(cbx.KeyValue) then
      cbx.Text := '';
  end;
end;

Well, I took your exemple here http://bugs.freepascal.org/view.php?id=20841 Please, copy paste the code below:

object DBLookupComboBox3: TDBLookupComboBox
Left = 96
Height = 21
Top = 416
Width = 179
AutoComplete = True
AutoDropDown = True
DataField = 'MasterId'
DataSource = LookupDatasource
KeyField = 'Id'
ListField = 'NAME'
ListFieldIndex = 0
ListSource = MasterDatasource
LookupCache = False
TabOrder = 6
end

Implements the EditingDoneEvent like this:
procedure TFooHelper.DBLookupComboBox3EditingDone(Sender: TObject);
var
  cbx: TDBLookupComboBox;
begin
  cbx := TDBLookupComboBox(Sender);
  ShowMessage(VarToStr(cbx.KeyValue));    ///<<<<<<<<<   see here
end;

So, even the component do not clean itself, if I type a value that not exists in list the KeyValue would be NULL, don't?

Marcos Douglas

IMHO, if there is a property to user choose like ScrollDataSet (default False)... would be great.

Marcos Douglas

Wat if i have one field point to a value in the listsource and another field pointing to another value?

What is the cursor position of the ListSource?

The same can be done without ListSource cursor updates but not as efficient. It requires more user actions.

You are looking for a TDataset that implements Master/Detail relationship. AFAIK TDBF and TSqlite3Dataset implements that.

Luiz

Resume:
The problem exists if the user typed a value that not exists on list (DataSet) but the KeyValue continue with the last value selected before.

Marcos Douglas

Sorry, I don't understand what you mean.

I know the article is called XYZsomething, I enter XYZ in the lookupfield (datasource = command line, listsource = articles) and push up and down to scroll to the one that has "round corners" in the description field that is displayed in another window (or panel) displaying ListSource. It is XYZ5, I press enter and my field is updated. That is how it works in Delphi.

I'm familiar with Master/Detail, thank you. This is about scrolling in a dataset while selecting field data in another dataset. The emphasis is on while selecting (before posting) and not after entering data as in Master/Detail.

Ludo

Please create a specific bugreport for dbgrid so i can provide a patch

Luiz

In a dataset i have to fields: PreferredFruit and FallbackFruit.

Both are indexed fields that lookup de display value in the same dataset Fruits

I set the PreferredFruit to 1 (Orange) and FallbackFruit to 2 (Banana)

Where the cursor of dataset Fruits should be?

Luiz

Yes. The underly problem is in ComboBox with AutodropDown not updating ItemIndex correctly. I will look if is a colateral effect of AutoDropDown or win32 behavior

Luiz

Before the patch it works fine using the EditingDone event, as I wrote before (sorry don't see that before, but this event is dynamic for all TDBLookupCombo, in all forms, a long time ago...) so, IMHO this is a colateral effect, as you said.

I tried to find the bug but I could not found it. =(

Marcos Douglas

I already found the bug (is related to AutoDropDown in win32). I will test other widgetsets later to see what can be done.

The changes in DBLookup just exposed a bug that was already there

Luiz

In the mean time, while the actual bug is not fixed, you can workaround by adding

ItemIndex := Items.IndexOf(Text)

in your EditingDone event before calling KeyValue

Luiz

DBGrid? It is the dropdownbox of the TDBLookupComboBox that highlights the wrong xxx. There is no DBGrid involved.

In your test program, select in TDBLookupComboBox the second xxx, MasterID of the datasource is updated to 3. That is fixed. Update uses key instead of listvalue. Now the other way around: select in the datasource a record with MasterID = 3, open the dropdownbox in TDBLookupComboBox and you'll see the first xxx highlighted. There is still a locate of the listvalue instead of key.

Ludo

The last one you selected. So, when user selects PreferredFruit, he can look at the the details of Fruits to help him in selecting the right fruit (customer hates red fruit and fruit with hard shells, so the user verifies color and shell type before selecting). Then he selects FallbackFruit. Same usage, verify Fruits details before selecting FallbackFruit, user is not interested in the details of Orange anymore.

Your next question: where is the cursor in the dataset Fruits if I change the cursor in the customer dataset? Answer: Delphi selects the first key field in the dataset record (the one with lowest fielddefs index or fields index if no fielddefs). If the user wants another field, an AfterScroll on the customer dataset can fix that.

Ludo

Yes, I'm already doing this. ;-)
But thanks, for all.

Marcos Douglas

That is right.

Marcos Douglas

Sorry. I'm referring to your first mail:

Mon, 12 Dec 2011 16:06:22 +0100 (CET)

"

Don't know if this is in the scope of the changes you made but the lookup in the dbgrid (picklist) is (still?) broken

"

This is the internal GUI toolkit (win32) behavior. We cant change that.

The same thing occurs with a vanilla combobox. There's no call in LCL or widgetset involved

Luiz

I've got your point.

Mine is that the control should keep less intrusive as it was designed earlier (the original author explicitly avoided changing listsource cursor).

With this it can have more granular usage for those that dont want to interfere to the dataset position and also for those that want to change it, and know the collateral effects, just add a single call to Locate in OnSelect or OnEditingDone

I will concentrate in the remaining bugs

Luiz

Sorry. I'm referring to your first mail:

http://bugs.freepascal.org/view.php?id=20895

Forgot to mention that I'm using svn 34079.

Hmm.. Selecting a line with MasterId 2, highlights "eeee" correctly. A miracle? No, TDBLookupComboBox.DataChange does a FLookup.GetKeyIndex and updates the combobox. FLookup.GetKeyIndex does a simple "run over Flookup and stops on first match". So, yes it can be changed.

Ludo

Less intrusive became occulted. Not only are we missing an important (for me at least) Delphi feature but the current implementation makes it virtually impossible to emulate the Delphi behavior since everything related to the lookup is private:
A convoluted workaround for DBLookupComboBox was to create a DBLookupComboBox.OnChange, retrieve DBLookupComboBox.ItemIndex and move its listsource to that position. Unfortunately, recent changes have made this even more difficult since DBLookupComboBox2.ListSource returns nil if Datafield is a lookup field. Setting ListSource when Datafield is ftLookup was superfluous but possible and useful. Beats me why this was removed. Progress? Now I have to get the field information of DataField to get to the ListSource :
DBLookupComboBox.DataSource.DataSet.FieldByName(DBLookupComboBox.DataField). LookupDataSet.RecNo:=DBLookupComboBox.ItemIndex+1 :( I'll create a seperate bugreport for this regression. Even if ListSource isn't settable for a ftLookup Datafield, it shouldn't return nil.

DBGrid is even worse. For the picklist I haven't found anything (event or property) to get to its index or value.

The remedy for being less intrusive is worse than the disease.

As written before, if you don't want 'collateral' effects, don't use a list datasource that is active in another control. In the current implementation there are also 'collateral' effects: fe. the order of elements in the lookup dataset is the same as the original dataset. Changing the sort order in the original dataset, changes the order in the dblookupcontrol. Apply a filter to the dataset and the dblookupcontrol is filtered also. Why wanting to break this link in one direction only?

I will concentrate in the remaining bugs

That's indeed a privilege that comes with developer status...

Ludo

I am totally in agreement with Ludo on this one.

I think the users should at least be offered the opportunity to change the behaviour. I have always counted on the Delphi-compatible behaviour: selecting an element in a lookup positions the cursor in the lookup dataset.

So a property 'SyncLookupCursor' or so should maybe be introduced. The default can be 'False', but I for one would need the 'True'.

Michael.

Correction: I won't create a bug report. It is now compatible with Delphi. I was using a loophole to ease the pain of emulating Delphi compatible behavior which is now closed for the sake of Delphi compatibility :)))

Ludo

That is I said before...

Marcos Douglas

Encountered a crash in dblookup.inc.
http://bugs.freepascal.org/view.php?id=20900
Easily reproducable but can't tell if this is related to recent changes.

Ludo

Hit another regression: open TDBLookupComboBox click on current value: record is marked as modified (Post called when cursor moved, etc). Before the changes, the record was updated only when the value changed.

Ludo

Please, send a patch.

Luiz

OK. I can do that

Please open a bug report with a sample project with the expected behavior pointing that is a delphi compatibility

Since the LookupSource is named ListSource

I think of ScrollList or ScrollListSource or ScrollListCursor

Luiz

Fixed

Luiz

Ludo Brands wrote

> I will concentrate in the remaining bugs > That's indeed a privilege that comes with developer status...

To be clear here:

It's was not me that made the behavior and was not my intention to change it when i started to look at those controls. It's the opposite, i tried at most not change the behavior. I'm just fixing the bugs, some long standing.

This is my spare time. And i spent more from my free time than i initially think (could not progress my main code project because of this). I faced bugs (and tried to fix) in TComboBox in gtk2, win32 and qt. Developers who tried to debug widgetsets know how hard it is and how long can take.

So the minimum right, or privilege as you say, that i can have is choose in what to work.

Luiz

I said before ScrollDataSet, but ScrollListSource is a better name.

Marcos Douglas

Did some further testing. The combobox gets the correct index but the combobox repositions the selection as soon as the dropdownbox is opened and the value is copied to the edit control. So windows starts the "update selection as you type" immediately. Looks like this is also intended to work on sorted strings because when you have a 'xxxx' on the top of the list and a 'xxx' on the bottom, you'll never get to the 'xxx' by typing the letters. A workaround to this is to use csDropDownList. The value is displayed in a text control instead of an edit control and the selection stays when opening the dropdownbox. Downside is that keyboard selection only works on the first character.
Not very much that can be done about that...

Ludo

The same tests (and conclusions) i did.

Not very much that can be done about that...

As i said before ;-)

Luiz

That was already understood.

We are all spending more spare time than intended on this. Testing, reporting bugs, debating the solution, creating patches,...

So the minimum right, or privilege as you say, that i can have is choose in what to work.

Nobody will deny this. What I'm referring to, and this is not personal, is that even when the patch or new functionality is provided by a "reporter", a "developper" has the privilege to not include the patch for whatever reason. Don't get me wrong. I don't want to (re-)start a flame war on this subject. Open source isn't democracy. I accept that and I just re-stated, not contested, the facts.
Perhaps this is a language/communication problem but the "I will concentrate in the remaining bugs" sounded pretty much as a "case closed". Don't forget you made a call for test. When the test result reports bugs that are important for the user and that are simply dismissed for no good reason, the reporter will perhaps also have the feeling that he is wasting his spare time ;)

Ludo

Fixed

Luiz

Fixed

Luiz

Implemented as ScrollListDataset. Defaults to false

Luiz

Thanks!

Marcos Douglas