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

Remember selected transaction with multiple account views opened #252

Closed
hsoft opened this Issue Jun 22, 2013 · 9 comments

Comments

Projects
None yet
1 participant
@hsoft
Owner

hsoft commented Jun 22, 2013

From email:

If you open two or more accounts one with small amount of transactions
(all fit into one page) and second account with big amount (several
pages of transactions), scroll position is not preserved when you
switch between accounts.

This problem is a limitation due to the current design (there's only one actual entry view, which is shared by all opened accounts). It's possible to send the scroll position down to the model and save it for the next refresh, but it seems like a lot of code for such a small feature. I wonder if it's not better to simply restore the selected transaction(s) at the middle of the table instead (it's currently restored at "the nearest point", so if the previous account was scrolled up, the selected transaction will be at the bottom of the scrollable table).

UPDATE: Forget about the paragraph above, I didn't remember that account views don't remember their own selected transactions yet and that this value is still document-wide. So, to fix this, each account view has to remember its own selected transaction.

@hsoft hsoft closed this Jun 22, 2013

@hsoft

This comment has been minimized.

Owner

hsoft commented Jun 22, 2013

I have a look on your source code, I don't like this heavy use of properties, but I managed to find where are changing account panes (core/gui/main_window.py, _change_current_pane, line 66) there should be something like 'remeber first visible transaction in hiding account' and 'scroll to first visible transaction in showing account'. You vill have something like 'first visible transaction' for every entry pane.

or more simple approach: just scroll to first selected transaction, if it is not visible.

@hsoft

This comment has been minimized.

Owner

hsoft commented Jun 22, 2013

moneyGuru already does that. The problem, as I wrote is that it does the minimum scrolling necessary to make the first selected transaction visible, which means that most of the time, the selected transaction ends up either at the very top or the very bottom, which is probably not optimal in most cases. That's why I was suggesting to scroll until the first selected transaction was in the middle of the table.

@hsoft

This comment has been minimized.

Owner

hsoft commented Jun 22, 2013

Oops, sorry, I checked again and it doesn't. I thought it did. I have to change the ticket.

@hsoft

This comment has been minimized.

Owner

hsoft commented Jun 22, 2013

(by the way, this is part of an ongoing, long run refactoring process described at http://www.hardcoded.net/docs/moneyguru/refactorings.html#more-responsibilities-to-the-view-classes )

@hsoft

This comment has been minimized.

Owner

hsoft commented Jun 22, 2013

The transaction selection update/restore process right now is a mess right now, and before I can un-mess it and implement this selection behavior, I'd ideally have to fully complete the proxy/model instantiation shift I'm currently in the middle of so that I can get rid of the shared account view kludge that's there right now. Once this is done, implementing this ticket will be much easier. Later.

@hsoft

This comment has been minimized.

Owner

hsoft commented Jun 22, 2013

The core gui instantiation refactoring process is complete, so I can give this ticket another try.

@hsoft

This comment has been minimized.

Owner

hsoft commented Jun 22, 2013

(from [3f54ce82b68e]) [#252] Replaced most references to MainWindow.shown_account by AccountView's own account reference.

Also, removed the shown_account_changed notification. All of this is in preparation of show_account's complete removal so we can have each AccountView instance independent of each other.
https://bitbucket.org/hsoft/moneyguru/changeset/3f54ce82b68e/

@hsoft

This comment has been minimized.

Owner

hsoft commented Jun 22, 2013

(from [c536e5ba7b40]) [#252 state:fixed] Completely removed MainWindow.shown_account.

This completes the transition from a shared AccountView to one instance of AccountView per opened Account pane. This shared AccountView thing was the cause of the entry selection glitch in #252. With one account instance per account, the entry selection doesn't have to be restored because it isn't touched when we switch accounts.
https://bitbucket.org/hsoft/moneyguru/changeset/c536e5ba7b40/

@hsoft

This comment has been minimized.

Owner

hsoft commented Jun 22, 2013

good work, thank you

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