ReaderThumbnails: update cached page thumbnail on bookmark note change by hius07 · Pull Request #10303 · koreader/koreader

hius07 · 2023-04-07T06:45:12Z

For Page browser.
Also minor cleaning of ReaderBookmark.

This change is

poire-z · 2023-04-07T20:14:21Z

        bookmark.type = self:getBookmarkType(bookmark)
        bookmark.text_orig = bm.text or bm.notes
        bookmark.mandatory = self:getBookmarkPageString(bm.page)
-        self.ui:handleEvent(Event:new("BookmarkEdited", bm))


You removed one of my two BookmarkEdited. So, this one won't send any event, which kills my hunt for completeness of bookmark related events :) Unless it should really not be considered "bookmark edited" ?
May be you should just change the other BookmarkEdited to BookmarkUpdated and let this one be (and be not handled by ReaderThumbnail).
Or use a single BookmarkUpdate with an additional parameter visual_change=true/false.

I think the event about editing the note must be sent when we press "Save" only, because when we press "Cancel" nothing changes.

The one you removed on line 1005 is triggerered only when doing Cancel somewhere ? Looks like its trigerred when add/edit note in readerhighlight?
But obviously ok if it is like you say (can't look seriously at the code, trusting you).

Looks like its trigerred when add/edit note in readerhighlight?

When add/edit note dialog is shown.
Then we can press Cancel and no changes are saved.
So let's trigger the event within the Save callback only.

So, you're saying that the following code path is taken only when the user hits Cancel somewhere - and it is then fine to remove line 1005 ?

koreader/frontend/apps/reader/modules/readerbookmark.lua

Lines 994 to 1005 in 34c2dab

function ReaderBookmark:setBookmarkNote(item, from_highlight, is_new_note, new_text)

local bookmark

if from_highlight then

local bm = self.bookmarks[self:getBookmarkIndexFullScan(item)]

if bm.text == nil or bm.text == "" then

bm.text = self:getBookmarkAutoText(bm)

end

bookmark = util.tableDeepCopy(bm)

bookmark.type = self:getBookmarkType(bookmark)

bookmark.text_orig = bm.text or bm.notes

bookmark.mandatory = self:getBookmarkPageString(bm.page)

self.ui:handleEvent(Event:new("BookmarkEdited", bm))

(It is not obvious that this is the case after a quick look, but trusting you if you think it really is :)

No, this piece of code is a preparing part for showing the input dialog.
And the bookmark is "edited" only after pressing the Save button.

OK, so it is just me that added Event:new("BookmarkEdited") here without thinking too much, and it was not needed, right ? :)

Yes I think the event was added earlier than the bookmark is actually edited.

poire-z

Fine with me if all is clear for you.

@hius07

KOReader 2023.04 "Solar Panel" ![koreader-2023-04-fs8](https://user-images.githubusercontent.com/202757/234975122-d7739c71-74aa-4cb0-a086-72a4bba70f52.png) It's been another busy month squashing many bugs. Our Mac users will be happy to hear that I told macOS we've supported HiDPI since long before anyone came up with such terminology (koreader#10341), and that the program can now natively build on M1 devices (koreader#10291). Solar panel credit: <https://openclipart.org/detail/294030/solar-energy> by gnokii We'd like to thank all contributors for their efforts. Some highlights since the previous release include: * Readerzooming: fix use of default settings (koreader#10205) @hius07 * ButtonDialog/ButtonDialogTitle: consistent 'width' handling (koreader#10230) @poire-z * MovableContainer: add support for anchoring initial position (koreader#10230) @poire-z * Book map, Page browser: add top left menu (koreader#10230) @poire-z * Book style tweak: add button with CSS suggestions (koreader#10230) @poire-z * crengine: fix parsing of multibytes encodings (koreader#10230) @poire-z * readerstyletweak: update profiles on unregistering in dispatcher (koreader#10247) @hius07 * Filesearcher: add search in book metadata (koreader#10198) @hius07 * Fix: Updated legacy directory, which crashed the program (koreader#10260) @Mochitto * ReaderLink: allow a forward location stack (koreader#10228) @yparitcher * BookInfo: add page information (koreader#10255) @hius07 * Center pdf manual zoom mode (koreader#10246) @nairyosangha * File browser: add Folder Menu (koreader#10275) @hius07 * Calendar view: add options to change start time of days (koreader#10254) @weijiuqiao * filechooser: fix crash on "unreadable content" (koreader#10283) @hius07 * Sync book statistics: add to dispatcher (koreader#10285) @ptrm * [plugin] Exporter: use util.getSafeFilename() to remove illegal characters from output filename (koreader#10282) @Mochitto * readerbookmark: fix writing pdf annotation (koreader#10287) @hius07 * Folder Menu: sign for Home folder (koreader#10288) @hius07 * ListMenu: show mark for books with highlights (koreader#10276) @hius07 * Calendar view's day view: thicker separator at 00:00 (koreader#10289) @poire-z * PageBrowser: tweak scrolling behaviour at book start/end (koreader#10289) @poire-z * Make `kodev check` feature complete (koreader#8682) @yparitcher * macOS: support for M1 building (koreader#10291) @ptrm * Reader: do not apply font size and spacing out of range (koreader#10295, koreader#10307) @hius07 * File browser: show Folder Menu on long-press on Home icon (koreader#10298) @hius07 * SSH.koplugin: fix cant stop SSH server bug when pid file's stale (koreader#10300) @weijiuqiao * PM: Optimize task queue handling around standby (koreader#10203) @zwim * statistic.koplugin: fix today's timeline showing next day when within custom offset (koreader#10299) @weijiuqiao * ReaderThumbnails: update cached page thumbnail on bookmark note change (koreader#10303) @hius07 * SDL: add multitouch support (koreader#10334) @Frenzie * SDL: add HiDPI support (koreader#10341) @Frenzie * BookInfo: fix crash on show cover (koreader#10315) @hius07 * Deal with table.pack corner-cases properly (koreader#10350) @NiLuJe * Android: add Tagus Gea support (<koreader/android-luajit-launcher#412>) @Alfedi [Full changelog](koreader/koreader@v2023.03...v2023.04) — [closed milestone issues](https://github.com/koreader/koreader/milestone/63?closed=1) --- Installation instructions: [Android](https://github.com/koreader/koreader/wiki/Installation-on-Android-devices) • [Cervantes](https://github.com/koreader/koreader/wiki/Installation-on-BQ-devices) • [ChromeOS](https://github.com/koreader/koreader/wiki/Installation-on-Chromebook-devices) • [Kindle](https://github.com/koreader/koreader/wiki/Installation-on-Kindle-devices) • [Kobo](https://github.com/koreader/koreader/wiki/Installation-on-Kobo-devices) • [PocketBook](https://github.com/koreader/koreader/wiki/Installation-on-PocketBook-devices) • [ReMarkable](https://github.com/koreader/koreader/wiki/Installation-on-ReMarkable) • [Desktop Linux](https://github.com/koreader/koreader/wiki/Installation-on-desktop-linux) • [MacOS](https://github.com/koreader/koreader/wiki/Installation-on-MacOS)

koreader#10303)

hius07 added 3 commits April 7, 2023 09:40

readerthumbnail: update page thumbnail cache on note change

87821a2

readerbookmark: clean events

2385485

filemanagerutil: remove unused "bookmarks_sorted"

9c99662

hius07 mentioned this pull request Apr 7, 2023

Add Book map and Page browser #8613

Merged

Frenzie added this to the 2023.04 milestone Apr 7, 2023

poire-z reviewed Apr 7, 2023

View reviewed changes

poire-z approved these changes Apr 13, 2023

View reviewed changes

hius07 merged commit af45ec7 into koreader:master Apr 14, 2023

hius07 deleted the fix-page-thumbnails branch April 14, 2023 04:53

0xstillb pushed a commit to 0xstillb/koreader-thai that referenced this pull request May 9, 2026

ReaderThumbnails: update cached page thumbnail on bookmark note change (

968334b

koreader#10303)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

ReaderThumbnails: update cached page thumbnail on bookmark note change#10303

ReaderThumbnails: update cached page thumbnail on bookmark note change#10303
hius07 merged 3 commits into
koreader:masterfrom
hius07:fix-page-thumbnails

hius07 commented Apr 7, 2023 •

edited by Frenzie

Loading

Uh oh!

poire-z Apr 7, 2023

Uh oh!

hius07 Apr 7, 2023

Uh oh!

poire-z Apr 7, 2023

Uh oh!

hius07 Apr 7, 2023

Uh oh!

poire-z Apr 13, 2023

Uh oh!

hius07 Apr 13, 2023

Uh oh!

poire-z Apr 13, 2023

Uh oh!

hius07 Apr 13, 2023

Uh oh!

poire-z left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	function ReaderBookmark:setBookmarkNote(item, from_highlight, is_new_note, new_text)
	local bookmark
	if from_highlight then
	local bm = self.bookmarks[self:getBookmarkIndexFullScan(item)]
	if bm.text == nil or bm.text == "" then
	bm.text = self:getBookmarkAutoText(bm)
	end
	bookmark = util.tableDeepCopy(bm)
	bookmark.type = self:getBookmarkType(bookmark)
	bookmark.text_orig = bm.text or bm.notes
	bookmark.mandatory = self:getBookmarkPageString(bm.page)
	self.ui:handleEvent(Event:new("BookmarkEdited", bm))

Uh oh!

Uh oh!

Conversation

hius07 commented Apr 7, 2023 • edited by Frenzie Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

poire-z left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

hius07 commented Apr 7, 2023 •

edited by Frenzie

Loading