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.

Sign up for GitHub

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

Jump to bottom

Minor fix: eventFilter drop on minimized; comments lost sync after GUI rest #299

Merged
merged 6 commits into from
Sep 24, 2018
Merged

Minor fix: eventFilter drop on minimized; comments lost sync after GUI rest #299

merged 6 commits into from
Sep 24, 2018

Conversation

davidlatwe
Copy link
Collaborator

This PR will provide two fix:

  1. eventFilter get removed when main window minimized.

    did not found this bug on previous PR, sorry :(

  2. Comment did not restore back to host after rest, here's the scenario:
    1. Artists input comments to QML comment box
    2. Validation failed or some other cases that make artist to press reset botton
    3. Comments remain in QML comment box and did not make any new input
    4. Published, but the comment did not collect to database

davidlatwe
davidlatwe commented Sep 19, 2018
View reviewed changes
pyblish_qml/control.py
"""Update comments to host and notify subscribers"""
self.host.update(key="comment", value=comment)
self.host.emit("commented", comment=comment)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was trying this kind of simple (lazy) fix.
Dose this work for you ? @mottosso

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it works, it works!

davidlatwe
davidlatwe commented Sep 19, 2018
View reviewed changes
pyblish_qml/ipc/server.py
return self.vessel.hide()
success = self._dispatch("hide")
self.vessel.hide()
return success

Copy link
Collaborator Author

@davidlatwe davidlatwe Sep 19, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

eventFilter will get removed if proxy._dispatch returns False/None, which suggest that subprocess connection has closed.
It's obvious that returning the result of self.vessel.hide() is incorrect.

@mottosso
Copy link
Member

Thanks @davidlatwe, once you're happy, you're welcome to merge. (I've added you to the contributor list!)

@davidlatwe
Copy link
Collaborator Author

Much appreciated !
Merge will be made right after I got back to my workplace and bump the version.
Thanks again @mottosso :)

@davidlatwe davidlatwe force-pushed the minor-fix branch 2 times, most recently from a4f6dd3 to 541309b Compare September 24, 2018 05:46
@davidlatwe
Copy link
Collaborator Author

Sorry for the spamming, didn't know that git rebase will break GitHub code review link, so I did a reset and re-bump the version with conflict resolved.

Merging now :)

@davidlatwe davidlatwe merged commit d89a2f4 into pyblish:master Sep 24, 2018
@davidlatwe davidlatwe deleted the minor-fix branch September 24, 2018 06:28
@mottosso
Copy link
Member

Thanks @davidlatwe :)

@davidlatwe davidlatwe mentioned this pull request Oct 1, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mottosso mottosso mottosso left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@davidlatwe @mottosso