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

Clear UI leftovers before doing an OTA-install #11412

Merged
merged 3 commits into from Jan 29, 2024

Conversation

zwim
Copy link
Contributor

@zwim zwim commented Jan 26, 2024

This is the logical continuation of #9114.

In #9114 the screen is cleared before downloading an update.

Then (unchanged by this PR) a ConfirmBox to ask for a confirmation of the new update is shown.
... no changes ...
After the successful download the user is asked to finally install the update.

Changes in this PR:
1.) Ask the user explicitly to update now or later. (No change in functionality,but only in texting, as later could have been selected by tapping outside the dialog before).
2.) Clear the screen before update, so that the user does only see whats happening (and not fragments of the book)..

Draft because: Does anybody know an easy solution to prevent the ConfirmBox to be movable (because moving it, shows some fragments of theold book's text)?


This change is Reviewable

@zwim zwim requested a review from NiLuJe January 26, 2024 21:52
@Frenzie
Copy link
Member

Frenzie commented Jan 26, 2024

I feel like https://xkcd.com/1172/ but I've always enjoyed being. able to continue reading while that's happening. 😊

Edit: although that might be more of a comment on #9114.

Comment on lines 409 to 410
text = _("Update is ready. Install it?"),
ok_text = _("Now"),
Copy link
Member

Choose a reason for hiding this comment

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

I'd rather just keep this as is, or perhaps Update now.

@@ -405,9 +405,10 @@ end

function Device:install()
local ConfirmBox = require("ui/widget/confirmbox")
local ignore_events = {"swipe", "hold", "hold_release", "hold_pan", "touch", "pan", "pan_release"}
Copy link
Contributor

Choose a reason for hiding this comment

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

If we really need to have some ConfirmBox and InfoMessage unmovable (and less need to have some events ignored but other not ignored), it could be simpler to pass unmovable=true to these widgets, which would pass it to their MovableContainer, and have:

function MovableContainer:init()
    if Device:isTouchDevice() and not self.unmovable then

(Even if it is strange to have an unmovable MovableContainer :) but that can be explained with a comment -- this can be passed whe needed to have MovableContainer present but a no-op, so we don't have to change the widget layout or something.)

@zwim zwim marked this pull request as ready for review January 29, 2024 18:55
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.

Fine with me for the unmovable stuff.
(No opinion about the OTA related stuff.)

@Frenzie Frenzie added this to the 2024.02 milestone Jan 29, 2024
@Frenzie Frenzie merged commit f836f6a into koreader:master Jan 29, 2024
3 checks passed
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

3 participants