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

Fix custom type saving when only focus changes for user-entered type #1147

Merged
merged 1 commit into from Sep 19, 2017

Conversation

Projects
None yet
3 participants
@techee
Copy link
Contributor

techee commented Sep 19, 2017

At the moment when user types a custom type into the type combo box and
doesn't press enter, the entered type isn't used. This patch tries to fix
the problem by installing an event filter for the combo box and checking
when it loses focus - when it does, it performs type updates too.

On the way the patch also changes the signal used inside moveCurrentField()
from activated() to currentIndexChanged() to make it consistent with the
rest of the file.

Fixes #1144.

Fix custom type saving when only focus changes for user-entered type
At the moment when user types a custom type into the type combo box and
doesn't press enter, the entered type isn't used. This patch tries to fix
the problem by installing an event filter for the combo box and checking
when it loses focus - when it does, it performs type updates too.

On the way the patch also changes the signal used inside moveCurrentField()
from activated() to currentIndexChanged() to make it consistent with the
rest of the file.
@techee

This comment has been minimized.

Copy link
Contributor Author

techee commented Sep 19, 2017

To workaround the missing "focus out" signal I found this:

https://stackoverflow.com/questions/321656/get-a-notification-event-signal-when-a-qt-widget-gets-focus

and using the event filter seemed to be the easiest way.

@justinclift justinclift requested a review from MKleusberg Sep 19, 2017

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Sep 19, 2017

Awesome @techee, looks like a win. @MKleusberg will probably review this sometime today, and merge it if good. 😄

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Sep 19, 2017

@MKleusberg I haven't created the 3.10.1 release yet. Is this something which should go in it?

@MKleusberg

This comment has been minimized.

Copy link
Member

MKleusberg commented Sep 19, 2017

Awesome, @techee! 👍 I can't see any problem with this. Thanks a lot 😄 If you ever feel like opening another PR you're very welcome to do so.

@justinclift Sure, why not. I'll backport it in a second.

@MKleusberg MKleusberg merged commit 969d3e4 into sqlitebrowser:master Sep 19, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@MKleusberg

This comment has been minimized.

Copy link
Member

MKleusberg commented Sep 19, 2017

@justinclift Done 😄

@MKleusberg MKleusberg added this to the 3.10.1 - Gaaah! milestone Sep 19, 2017

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Sep 19, 2017

@MKleusberg Awesome, thanks. 😄

I'll into the release process shortly. A bit behind schedule, but it'll still probably be today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment