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.

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

Reload comics view - some fixes #219

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

vedgy
Copy link
Contributor

@vedgy vedgy commented Feb 18, 2021

See the commit messages for details.

@vedgy
Copy link
Contributor Author

vedgy commented Mar 1, 2021

I have implemented many more somewhat dependent fixes on top of this branch: https://github.com/vedgy/yacreader/commits/navigation-reloading-drag-fixes. I'll create a separate pull request after this one is merged.

@vedgy vedgy mentioned this pull request Mar 13, 2021
I don't see a scenario where this member function could be useful.
Removing prevents calling it by mistake in place of
YACReaderHistoryController::currentSourceContainer().
After the current order was assigned to comics while a reading list was
selected or in search mode, the last selected folder's info was loaded.
In the search mode case this made the UI look half-way between Normal
and Searching navigation statuses. Now the last selected folder's or
list's info or the last search result is reloaded.

One issue remains: after the current order is assigned in search mode,
the first comic in the search results is selected rather than the comic
to which the first number was assigned. I don't know how to fix this.

An alternative fix is to disable asignOrderAction while a reading list
is selected and in search mode. But this is more difficult to implement
and inconsistent with the other enabled-in-these-cases comic actions.
Disabling the action would also remove the existing possibly useful
feature of assigning numbers across directories.
This commit fixes two bugs:
1. When the current reading list is deselected by clicking outside of
all rows in YACReaderReadingListsView, the reading list is still loaded
and active in the comics view. Triggering the *Delete selected comics*
action deletes the selected comics from disk rather than from the
current label/list then.
2. The meaning of the *Delete selected comics* action in search mode
depends on whether a reading list was (and still is) selected when the
search mode was entered.

Now the comics are deleted from the current label/list if and only if
a reading list is loaded in the comics view. It doesn't make sense to
delete comics from some reading list in search mode, so deletion from
disk occurs, as well as when a folder is loaded in the comics view.
Wherever YACReaderNavigationController::reselectCurrentSource() was
called, simply reloading the comics view is sufficient: when a dialog is
accepted or when the last comic is deleted from the current reading
list. Similarly there is no need to reselect the current folder when the
last comic is deleted from the current folder.

Simply reloading the comics view saves some work. In addition, in all
changed places forward navigation history entries were cut off and lost
because of the call to YACReaderHistoryController::updateHistory() in
selectedFolder() or selectedList().

Now that reselectCurrentSource() is not used anymore, it can be safely
removed rather than fixed. Its implementation suffered from two bugs:
1. Checking whether a reading list is selected is not reliable, because
a reading list can be deselected but not unloaded by clicking outside of
all rows in YACReaderReadingListsView.
2. It didn't account for the possibility of active search mode.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant