/moneyguru

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

Cancel becomes bound to "Return" in dialog after a first cancellation #433

Closed
hsoft opened this Issue Jul 5, 2015 · 1 comment

Comments

Assignees

@hsoft hsoft

Labels
Projects
None yet
Milestone
  v2.9
1 participant
@hsoft
@hsoft
Owner

hsoft commented Jul 5, 2015

It's the first time I notice this, but it's probably an old problem: Under Qt, when clicking cancel on a dialog (for example, transaction details), Cancel becomes the new default button during the next invocation, so when you press "Return", the dialog will cancel instead of saving. Not very fun.

@hsoft hsoft added bug qt labels Jul 5, 2015

@hsoft hsoft added this to the v2.9 milestone Jul 5, 2015

@hsoft

This comment has been minimized.

Sign in to view
Owner

hsoft commented Jul 5, 2015

I suspect that it's a collateral effect of our dialog not being created on-the-fly every time (instances are being created on startup and re-used multiple times). We can probably fix a lot of glitches by adopting a create-on-the-fly mechanism for dialogs. However, we can't do this as a bugfix because it's a significant refactoring. We'll probably create a followup issue.

@hsoft hsoft self-assigned this Jul 5, 2015

@hsoft hsoft referenced this issue Jul 6, 2015

Closed

Don't reuse dialog instances #434

@hsoft hsoft closed this in bb6bd90 Jul 6, 2015

hsoft added a commit that referenced this issue Jul 18, 2015

@hsoft
Make panel instantiation on-the-fly 
Prior to this commit, all panels were instantiated at startup,
references held by `MainWindow`. In addition to being inelegant and
wasteful in terms of resources, this led to some glitches like #433.

This commit makes all panel instantiation happen on-the-fly. The
`BaseView` is responsible to provide the panel with the appropriate UI-
layer view instance through `get_panel_view()`.

Conceptually, this change is very simple, but it results in quite many
SLOC changes, especially in test code. Previously all tests involving
panels (many) referred to those panels through global instances held by
`MainWindow`. Because these references no longer exist, we have to get
those instances elsewhere (see `TestApp.get_current_panel()`). That
changes the tests a lot.

We also introduce weakrefs in this commit. Previously, we would merrily
create circular references because most of our UI-related classes were
instantiated once, at startup (and thus released once, at shutdown). No
memory leak.

With on-the-fly instantiation, this changes. Unfreed panels are memory
leaks. We can't rely on Python's GC because under Cocoa, we have to mix
Python's memory model with ObjC's: as long as the UI layer is not
manually released, everything stays in memory. This is why we had to
break circular references. In this commit, I've made sure that every
panel's `__del__()` was called when a panel was closed. There are still
many circular references left in the app, but not in panels.

This is a squash commit of the "dialog-instance" branch.

fixes #434
1ac3b64
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment