Skip to content

Bug fix: update page_turning_inverted icon/text#12386

Closed
wanderor wants to merge 1 commit into
koreader:masterfrom
wanderor:master
Closed

Bug fix: update page_turning_inverted icon/text#12386
wanderor wants to merge 1 commit into
koreader:masterfrom
wanderor:master

Conversation

@wanderor

@wanderor wanderor commented Aug 20, 2024

Copy link
Copy Markdown

Bug fix: update page_turning_inverted icon/text when inverse_reading_order is updated in reader UI, not in settings.

Note that reader UI loads inverse_reading_order from settings and then maintains it, allowing users to frequently change this value (e.g. when switching the device between hands.)


This change is Reviewable

…order

is updated in reader UI, not in settings.

Note that reader UI loads inverse_reading_order from settings and then
maintains it, allowing users to frequently change this value (e.g. when
switching the device between hands.)
if G_reader_settings:isTrue("input_invert_page_turn_keys") or
G_reader_settings:isTrue("input_invert_left_page_turn_keys") or
G_reader_settings:isTrue("input_invert_right_page_turn_keys") or
footer.ui.view.inverse_reading_order then

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This feels a bit suspicious. If G_reader_settings:isTrue("inverse_reading_order") won't do the trick that may well be a problem because the menu also depends on that for example. In which case it should presumably also be updated here to resolve the problem globally:

function ReaderView:onToggleReadingOrder()
self.inverse_reading_order = not self.inverse_reading_order
self:setupTouchZones()
local is_rtl = self.inverse_reading_order ~= BD.mirroredUILayout() -- mirrored reading
Notification:notify(is_rtl and _("RTL page turning.") or _("LTR page turning."))
return true
end

But that's just a quick note before heading off to bed; I may have overlooked something.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Hi @Frenzie , inverse_reading_order is a per-document setting, that is, you can set different values for each book. This makes sense as different books may use different languages and layouts. The global setting of inverse_reading_order is more of a default value.

ReaderView loads and saves the per-document value:

self.inverse_reading_order = config:isTrue("inverse_reading_order") or G_reader_settings:isTrue("inverse_reading_order")

self.ui.doc_settings:saveSetting("inverse_reading_order", self.inverse_reading_order)

A more illustrative reference is:

text_func = function()
local text = _("Invert page turn taps and swipes")
if G_reader_settings:isTrue("inverse_reading_order") then
text = text .. ""
end
return text
end,
checked_func = function()
return ReaderUI.instance.view.inverse_reading_order
end,

This code snippet:

  • Appends ★ to the menu text when the global default value is true.
  • Checks the menu when the per-document value is true, which may or may not equals to the global value.

@Commodore64user FYI.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

inverse_reading_order is a per-document setting

I don't think that's a relevant detail wrt my comment, but that it does it the same way elsewhere may be fair enough.

@Commodore64user

Commodore64user commented Aug 20, 2024

Copy link
Copy Markdown
Member

Seeing as this hasn't even made it to a stable release, I am slightly annoyed you didn't report the problem instead. Nevertheless what needs to be fixed here is the toggle not footer check.

@wanderor

Copy link
Copy Markdown
Author

Seeing as this hasn't even made it to a stable release, I am slightly annoyed you didn't report the problem instead. Nevertheless what needs to be fixed here is the toggle not footer check.

Hi @Commodore64user , thank you for contributing the great feature in the first place. Sorry that I didn't check the history and reported the issue I found earlier.

I have different opinion regarding #12388 and replied there.

if G_reader_settings:isTrue("input_invert_page_turn_keys") or
G_reader_settings:isTrue("input_invert_left_page_turn_keys") or
G_reader_settings:isTrue("input_invert_right_page_turn_keys") or
footer.ui.view.inverse_reading_order then

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can be footer.view.inverse_reading_order.

self.footer = ReaderFooter:new{
view = self,
ui = self.ui,
}

@Commodore64user

Commodore64user commented Aug 21, 2024

Copy link
Copy Markdown
Member

Seeing as this hasn't even made it to a stable release, I am slightly annoyed you didn't report the problem instead. Nevertheless what needs to be fixed here is the toggle not footer check.

Hi @Commodore64user , thank you for contributing the great feature in the first place. Sorry that I didn't check the history and reported the issue I found earlier.

I have different opinion regarding #12388 and replied there.

No worries mate! but since this was my PR, please let me deal with it.

@poire-z

poire-z commented Aug 25, 2024

Copy link
Copy Markdown
Contributor

Superceded by #12388.

@poire-z poire-z closed this Aug 25, 2024
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.

5 participants