Skip to content

Ignore event_type to prevent unexpected model changed popup #3278

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

Merged

Conversation

teunhoevenaars
Copy link
Contributor

@teunhoevenaars teunhoevenaars commented Apr 28, 2024

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bug fix
  • Feature
  • Chore (refactoring, formatting, local variables, other cleanup)
  • Documentation content changes

What is the current behavior?

Issue Number: #3275

What is the new behavior?

time-out after save action, before file monitor is restarted

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@teunhoevenaars teunhoevenaars marked this pull request as draft April 28, 2024 10:31
@github-actions github-actions bot added the python Pull requests that update Python code label Apr 28, 2024
@teunhoevenaars teunhoevenaars changed the title Tim-out to prevent unexpected model changed popup Time-out to prevent unexpected model changed popup Apr 28, 2024
Comment on lines 337 to 339
time.sleep(2)
self.filename = filename
self._update_monitor()
Copy link
Member

Choose a reason for hiding this comment

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

This should be handled via GLib.timeout_add. This way the whole UI freezes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

in the end, the timeout really did not fully resolve this... I still saw the popups. Therefore, I just ran Gaphor with debugger and realized that the popups on Mac are triggered specifically by ATTRIBUTE_CHANGED events, which I don't think are events we're interested in. As such, I've now added a check against this event-type, which resolves this issue. Happy to hear your thoughts on this!

@teunhoevenaars teunhoevenaars changed the title Time-out to prevent unexpected model changed popup Ignore event_type to prevent unexpected model changed popup May 4, 2024
@teunhoevenaars teunhoevenaars marked this pull request as ready for review May 4, 2024 15:32
@danyeaw danyeaw added fix A fix for a bug and removed python Pull requests that update Python code labels May 5, 2024
Copy link
Member

@danyeaw danyeaw left a comment

Choose a reason for hiding this comment

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

Hi @teunhoevenaars, thanks! These changes look great!

@danyeaw danyeaw merged commit c6169df into gaphor:main May 5, 2024
@amolenaar
Copy link
Member

Works well. It doesn't trigger if I touch the file, but after a real change the popup shows up. This is a nice improvement.

@teunhoevenaars teunhoevenaars deleted the unexpected_model_changed_popup_test branch May 8, 2024 21:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix A fix for a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants