Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change TJvDBComboBox search to be case sensitive #128

Merged
merged 5 commits into from May 28, 2019

Conversation

Projects
None yet
3 participants
@MHumm
Copy link
Contributor

commented May 20, 2019

Made search Key case sensitive by default but additionally provided a property to control this.
Fix for Mantis issue 6361.
http://issuetracker.delphi-jedi.org/view.php?id=6361

Show resolved Hide resolved jvcl/run/JvDBCombobox.pas Outdated
Show resolved Hide resolved jvcl/run/JvDBCombobox.pas Outdated
@ronaldhoek

This comment has been minimized.

Copy link
Contributor

commented May 21, 2019

I would suggest the default behavior too remain case insensitive as it would break the current code.
This is mostly de default behavior of text searches... (See code review suggestions)

@MHumm

This comment has been minimized.

Copy link
Contributor Author

commented May 21, 2019

Created a new commit with your requested changes. I hope it can be merged now.

@ronaldhoek

This comment has been minimized.

Copy link
Contributor

commented May 22, 2019

The code works as to having 'case sensitive key values' (zie 'Values' property).
But when EnableValues is false the combo box still works case insensitive regarding the stored data.

Two solutions:

  • change property name to 'CaseSensitiveValues', therefore making it clear it only functions when using the Values property data when EnableValues is true (= default)
  • also, make it work when EnableValues is false (but then you need to dig into the combo box itself)
@MHumm

This comment has been minimized.

Copy link
Contributor Author

commented May 22, 2019

As you can see I opted for your first suggestion. I hope it is good enough for now.

Show resolved Hide resolved jvcl/run/JvDBCombobox.pas Outdated
Show resolved Hide resolved jvcl/run/JvDBCombobox.pas Outdated
Show resolved Hide resolved jvcl/run/JvDBCombobox.pas Outdated
@MHumm

This comment has been minimized.

Copy link
Contributor Author

commented May 23, 2019

Renamed the getters and setters as requested. Please merge in now if good enough now.

@obones

This comment has been minimized.

Copy link
Member

commented May 24, 2019

There are two remarks remaining

@ronaldhoek

This comment has been minimized.

Copy link
Contributor

commented May 24, 2019

There are two remarks remaining

When these are both resolved (I made the change locally) the code functions as intended.
(Tested with XE2 and 10.3.1)

Markus Humm
Fixed spelling of Result and changed property name in derrived class …
…so it matches the one in the ancestor class.
@MHumm

This comment has been minimized.

Copy link
Contributor Author

commented May 24, 2019

Ok, fixed both issues, so I hope this can be pulled in now.

@ronaldhoek

This comment has been minimized.

Copy link
Contributor

commented May 27, 2019

Tested using applied changes - from my part OK to merge @obones

@obones obones merged commit 8e5f050 into project-jedi:master May 28, 2019

@MHumm MHumm deleted the MHumm:Mantis6361_JvDBComboBox_Search branch May 28, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.