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

Scroll previous page to the bottom feature fix #18

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

PalmtopTiger
Copy link
Contributor

When the top of the current page is reached, a previous page should be shown scrolled to the bottom. But it doesn’t happen, because an existing call of view->scrollToBottom() scrolls to the bottom only the current page. Image loading takes place in a separate thread and always scrolls a new page to the top on completion.
I’m not sure, if I fixed the bug right, but it works.

@stolowski
Copy link
Owner

stolowski commented Nov 22, 2016

Hi, thanks for another contribution and I apologize for the delay with looking at it... I've just been reviewing the project preparing the new release and tested your changes, but unfortunately I didn't see the difference. This was a bit unexpected as looking at the code, I'd expect it to actuallty scroll to bottom in every case, also when going to the next page when scrolling to the bottom (which of course is not desired)...
Anyway, please let me know if I missed anything and re-test it yourself after merging the latest master. Thanks!

@PalmtopTiger
Copy link
Contributor Author

Hi.
Page scrolls to bottom only when we go to the previous page. To achieve this I added scrollPageToBottom flag that is set to true, only in this case, and cleared afterwards:
https://github.com/PalmtopTiger/QComicBook/blob/eea4d89c4175d0834b9eb5139835e92571169319/src/ComicMainWindow.cpp#L881
https://github.com/PalmtopTiger/QComicBook/blob/eea4d89c4175d0834b9eb5139835e92571169319/src/ComicMainWindow.cpp#L674

I build QComicBook from master and this bug still persists.
I'll try to describe the origin of the bug:
When we go to the previous page, the application adds a request to the asynchronous loading of a new page:

addRequest(m_currentPage, props.twoPagesMode() && n+1 < numOfPages());

Immediately after that, the application scrolls the current page to the bottom, while a new page is loading in another thread:
view->scrollToBottom();

Sometimes you can even see how bottom of the current page flashes.
When the previous page is loaded application scrolls the page to the top irrespective of the fact, it is the previous or next page:
verticalScrollBar()->triggerAction(QAbstractSlider::SliderToMinimum);

Disabling the scrolling does not solve the problem, because we don't know the geometry of the new image, and we can scroll only to the height of the current page. Scrolling should be performed when the page was loaded. For example, in pageLoaded, as I implemented:
https://github.com/PalmtopTiger/QComicBook/blob/eea4d89c4175d0834b9eb5139835e92571169319/src/ComicMainWindow.cpp#L673

I fixed the bug in the simplest way. If you wish, you could close the pull request and fix the bug properly by yourself.

Also, I prepared the test file for this bug:
http://www.mediafire.com/file/79oap34x48n38la/%5BQComicBook%5D%5Bpull18%5D_Scroll_to_bottom.zip
In the transition from page 3 to page 2, we should see a green area with a number 2, as it happens with my fix. Without my fix, we see the red zone.
By the way, on my computer the lower part of the page 1 is cut off.

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.

2 participants