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

ReaderFooter: Cleanup some more messy dimensions handling #6898

Merged
merged 8 commits into from
Nov 22, 2020

Conversation

NiLuJe
Copy link
Member

@NiLuJe NiLuJe commented Nov 20, 2020

Some stuff was still hacked on manually instead of trusting the widget system to do things right, which it does, if you update the right stuff at the right time the right way ;). This Is The Way.

Fix #6893 (and address #6878 (comment), because it was indeed redundant ^^).

Includes a bonus fix for a number of settings not being applied immediately in PDFs.


This change is Reviewable

(Also, it's redundant, resetLayout already takes care of it).
It's smart enough to figure out what happens on its own, as long as we
actually update the right widget...

Fix koreader#6893
This will be computed properly later on, either by applyFooterMode, via
resetLayout or the text generator.
Copy link
Contributor

@poire-z poire-z left a comment

Choose a reason for hiding this comment

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

Looks alright.

@Frenzie Frenzie added this to the 2020.12 milestone Nov 20, 2020
Copy link
Member

@Frenzie Frenzie left a comment

Choose a reason for hiding this comment

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

Looks fine, let me know if you want me to test though. ;-)

@poire-z
Copy link
Contributor

poire-z commented Nov 20, 2020

Btw, @NiLuJe Footer Master, regarding #6885 (comment) :

In ReaderCropping:onPageCrop(), we backup a lot of settings to reset the view dimension to what's available above the bottom buttons.
Except that ReaderView (I guess) will still add (or remove, depending on how you look at it) the space taken by the footer:
image
If I hide the footer before launching that crop dialog, there is not this bottom grey blank area.

Is there a canonical way to check the footer is visible and hide it if it is (and later unhide it so we get back the footer exactly as it was) I could do in the part where we backup the original dimensions?

@NiLuJe
Copy link
Member Author

NiLuJe commented Nov 20, 2020

@poire-z: You can check the non-ReaderFooter bits of #6830 for how the check (should) work.

Actually toggling it for real might be a bit trickier, though, because I don't think we have a direct way to do that: the tap handler cycles through modes. Might be able to store/set to 0/restore the mode manually and then call a full redraw (the various ways to do that were discussed a few days ago in a PR about a thingy not doing it or... something :D), though?

@NiLuJe
Copy link
Member Author

NiLuJe commented Nov 20, 2020

@Frenzie: I think I checked the usual corner-cases, but, by all means ;).

@poire-z
Copy link
Contributor

poire-z commented Nov 20, 2020

You can check the non-ReaderFooter bits of #6830 for how the check (should) work.
Actually toggling it for real might be a bit trickier, though

Thanks. Actually, I don't really need to toggle it... I just need to lie/tell ReaderView it's not visible :)

    -- backup original footer visibility
    self.orig_view_footer_visibility = self.view.footer_visible
    self.view.footer_visible = false

works for me!

@NiLuJe
Copy link
Member Author

NiLuJe commented Nov 20, 2020

And just fixed a "slightly" older bug (mine ;p) so that toggling some stuff (mainly, the progress bar) in the settings actually repaints stuff immediately and properly in ReaderPaging.

(The old logic was relying on a CRe only chain of events for a bit of refresh optimization).

EDIT: Okay, looking at rg -F 'refreshFooter(true, true)', quite a bit more settings that just toggling the progress bar ;p.

@Frenzie
Copy link
Member

Frenzie commented Nov 21, 2020

@NiLuJe

I think I checked the usual corner-cases

And there's a lot of 'em too. Nothing obviously wrong.

@NiLuJe NiLuJe merged commit 0ae86fc into koreader:master Nov 22, 2020
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.

Can't hide status bar until the page is changed.
3 participants