Add "annotated bookmarks"

[jzohrab]

Lute has multi-page books. It would be nice to add "bookmarks" to pages, for a few reasons:

• it makes sense, you can do this in a Kobo, why not in lute?
• a page might have a nice construction you want to review later
• mark chapters, etc
• Currently you can go to the prev or next page, or jump by 10, but it would be nice to go to an arbitrary page, per request #29. But you'd want to return to where you were ...

Adding annotated bookmarks would let you add a bookmark, plus a brief optional note about what that bookmark is for. Then maybe you could see a listing of bookmarks, and click it to go to the bookmark.

Implementation notes:

• db change - this could either be a new "TxBookmark" nullable text field in the texts table, or a separate table textbookmarks with fields TbID, TbTxID, TxBookmark ... the latter makes more sense db design wise, because most text records will not be bookmarked, but joins and deletes etc are a hassle so just adding TxBookmark would be fine by me.
• add "add bookmark" menu item in the slideout, and potentially "update bookmark" to update annotation, and "remove bookmark"
• have a "show bookmarks" page, which shows the annotations and links and page number
• a page with a bookmark could potentially show a "bookmark" image in the top right, to indicate that it's bookmarked

[cblanken]

@jzohrab I've been looking into this issue since I'd really like to have a bookmark feature. I agree that a separate textbookmarks table is probably the way to go. Especially if it ever needs to be expanded later on. Do you have anything in the works for this, or should I be good to try my hand at it?

[jzohrab]

Hi @cblanken - nothing in the works for this.

I was leaning towards not having a separate table, just to reduce join hassles (b/c sometimes doing these mappings with sqlalchemy is a pain, and having separate tables means that we need to ensure deletion of the other records etc etc). If we add a nullable bookmark field to the texts table we're fine, and since it will be much less code around the object management, I think that's a much better idea. :-) LMK if you have any hard disagreements on that one, maybe there is something I'm missing. (EDIT - particularly with the note "Especially if it ever needs to be expanded later on" -- I can't think of what that expansion might be though, so am inclined to go with the Simplest Thing That Could Possibly Work, aka the single field, at present)

This feature adds a bunch of operations for bookmark management to the UI as noted above. I wonder if there is a clever way that we can break this feature down so that it can be incrementally delivered ... Maybe we do something like this, to split up the work too if needed:

Part one: all backend work

• add field to texts table, necessary tests (e.g. domain to db object mapping, if any!)
• add routes for adding/editing/removing book bookmarks

Part 2: UI slideout "bookmarks" subsection

All "bookmark" actions are in a "bookmark >" sub-slide-out in the reading menu, like the Edit menu:

• "add bookmark" link if page not bookmarked -- opens a javascript alert box to do simple data entry, ajax posts back to server
• have "edit bookmark" link if page already bookmarked -- opens same alert box, pre-pop'd with current bookmark
• have "delete bookmark" if already bookmarked
• have "list bookmarks" for whole book, goes to page with listing, links with all bookmarks

Part X?

... add little bookmark icon top right? Maybe not needed at all, just screen clutter ...

[jzohrab]

Happy to chat about this here or on Discord as needed :wave: cheers!

[jzohrab]

Some more thoughts about this:

• users may want multiple bookmarks for a page? -- with a physical book this is possible (multiple strips of paper) but not with an e-readers I've seen. Kobo, PDF reader, etc.
• possibility of "system bookmarks" being added, eg for chapter headings ... and with some books, chapter may be very short.

I could be getting ahead of myself with this, and would rather design for the "now" than for the possible future. Re "a page might have a nice construction you want to review later" -- that sort of relates to #41 .

If we did allow multiple bookmarks for a page, I'm not sure how the reading UI would be organized. The "edit bookmark"/"delete bookmark" would need to indicate which one to edit/delete somehow.

[cblanken]

I can't think of what that expansion might be though, so am inclined to go with the Simplest Thing That Could Possibly Work, aka the single field, at present)

I was thinking along the lines of the following possible features:

1. Multiple bookmarks per page (probably marking at the sentence level)
2. Bookmark hierarchical structure like you might expect from a textbook. Chapter > Section > Subsection. In particular, for my use case I'm reading Lord of the Rings which splits down to Volume > Book > Chapter, and it might be nice to organize them as such in the bookmark display.
3. Bookmark categories. Just spitballing here, but it might be nice to distinguish between types of bookmarks even across books. For example I could sort my bookmarks under several categories like "cool words", "idioms", or "character quotes" Of course, this sort of assumes sentence or word-level bookmarks. Then a separate page could be used to search bookmarks by category similar to searching for terms by language.

These may be better implemented as separate features outside of bookmarks. I'm not sure tbh, but that's why I was thinking it would be better to isolate the bookmarks from texts. If there's any overlap with the simpler TxBookmark column then it would need to be refactored later on, since otherwise anything like a bookmark type, hierarchy, sentenceId would need to be added to texts.

users may want multiple bookmarks for a page? -- with a physical book this is possible (multiple strips of paper) but not with an e-readers I've seen. Kobo, PDF reader, etc.

I haven't used an e-reader in a while, but I think you're right, bookmarks are usually page level. It might make sense to have a separate feature for adding "highlights" per page and not lump them together with bookmarks.

If we did allow multiple bookmarks for a page, I'm not sure how the reading UI would be organized. The "edit bookmark"/"delete bookmark" would need to indicate which one to edit/delete somehow.

I actually had some thoughts about the bookmark UI I wanted to run by you. I know it probably makes sense to add interactions in the left slideout menu, however what do you think about integrating the bookmark display into the read_right_pane? If a user wants to add/edit/delete bookmarks, but they probably don't need to do term lookup at the same time. That would also allow the display of a lot of bookmarks at the same time and allow the user to skip back and forth to different bookmarks quickly without needing to re-open the slideout menu. I think that would be a much better user experience than needing to swap back and forth to a separate page to see bookmarks.

On top of that, the add/edit/delete functionality could be embedded in a few icons next to each bookmark without a need to select which bookmark is the target. Sort of like this.

All that said, I'm not strongly opposed to taking the easier road of just adding a column to texts. I've never worked with SQLAlchemy, so I'll take your word that joins and such are pain. However if you think any of those features I mentioned are possibility in the future, it may save more time in the future to give more consideration to the DB design from the beginning.

... add little bookmark icon top right? Maybe not needed at all, just screen clutter ...

I definitely like the idea of a bookmark icon. Either as a interactive element/form to bookmark a particular page or open the aforementioned bookmark pane in the read_right_pane. One icon shouldn't be much clutter and definitely a positive for user experience in my option so swapping over to slideout isn't necessary.

[jzohrab]

Hey there, thanks for the detail above.

** Re the joins, my mistake, they're actually trivial -- I was confusing these joins with another idea. e.g. Book to BookTag is a similar idea, and already exists in the code.

• "add/edit/delete icons in the listing" yep this makes sense. Currently the various listings (books, languages, terms) don't have such icons, maybe we'd want some consistency -- but that's just refining the idea.
• "I've never worked with SQLAlchemy" In that case I'd probably do the initial back-end stuff for you, just to keep it clear :-) and I'd add the necessary data checks and tests etc.

Things we should defer from an initial iteration, due to complexity:

• "Integrating into read_right_pane I do like this idea for the longer term, but currently the frame/dictionary javascript code is messy, and swapping in a separate view for bookmarks is going to create problems, at least AFAICT. Mobile moves that right pane to the bottom of the screen for the editing form as well. For an initial iteration, opening a separate screen of bookmarks is much easier. This is similar to what current eReaders do, and should still deliver a lot of value, at the expense of a bit of screen thrash.
• "bookmarks ... at the sentence level" -- this won't be easily possible, as sentences can be added/removed if a page is edited, so the ids change. You could track bookmarks by some other mechanism, such as the md5 of the sentence text, but that's getting tougher -- I think some of that is outlined in issue #41 (sentence notes).
• "word level bookmarks" -- There's already the "term definition" form with tags, and there's potentially another thing, #235 (term notes) that could handle this. If we keep the "bookmark" metaphor in mind, it reduces scope for the current iteration.

[cblanken]

In that case I'd probably do the initial back-end stuff for you, just to keep it clear :-) and I'd add the necessary data checks and tests etc.

Works for me. I can still experiment with the backend stuff a bit if you're busy, but that's probably for the best until I'm more familiar with the codebase :)

I'll start working on a template for the bookmarks page.

[cblanken]

Okay, I basically just ripped the table from the books page to display bookmarks. Is this the basic sort of user flow you were thinking?

"add/edit/delete icons in the listing" yep this makes sense. Currently the various listings (books, languages, terms) don't have such icons, maybe we'd want some consistency -- but that's just refining the idea.

I was actually rethinking the add/edit/delete actions for the bookmarks. Do you think it'd be better to just adapt the same actions dropdown from the books table so that it's consistent? I don't think you really gain a lot from having inline buttons/icons. Spelled out actions are usually better UX in my opinion since there's less opportunity for misinterpretation of what an icon will do even if it does look a bit nicer.

[jzohrab]

Hah yep that was it exactly :-) As an initial implementation it could add a lot of value and would sort out a lot of back end code.

[jzohrab]

The only challenge here is that the delete and edit bookmark menu items don’t work if there are multiple bookmarks on a page, which we probably should support. (I think) so perhaps the bookmark listing has the “edit/delete” actions spelled out as you said, and the reading pane menu has “add bookmark” and “listing” only.

[jzohrab]

Hi @cblanken -- let me know here (or discord) if you're awaiting anything from me for this issue. I've marked it as "to-do" in the Lute project board so that I keep tabs on it. If you're not considering hacking at it, I'll put it back into the backlog. No rush and no pushing from me, by the way. :-) This is a free open-source project so we all do what we can. Cheers!

[cblanken]

Hey @jzohrab, I was still planning to hack on it a bit more. I'll look at it more this weekend if I have time or later this week.

[cblanken]

@jzohrab I fixed up the popup menu options like you suggested and remade the Bookmarks page using a DataTable so that the "Actions" will be consistent with the other tables in the app.

If that looks good, then I'll start digging into some of the backend stuff unless you want to take over for that. In which case, I can draft a PR for the frontend stuff. Just let me know.

[jzohrab]

Hi @cblanken , looks great!

In terms of the back end, we should sort that out. I have a few pressing things I need to get done w/ Lute, a couple of annoying bugs that were reported, and of course there is real life stuff to handle. But we should get this feature in as you need it, and it's a good one.

Following is a rough outline of the work I think is necessary for this. I'm not sure how up to speed you are with python, sqlalchemy etc, and/or how much time you want to invest trying to add this. :-) It would be super to have you contribute, but sometimes that actually creates more net work for me, as I have to rework or tune stuff contributed. If you want to dive in, super! But if you don't/can't, that's ok too -- it will obviously be faster for me to work on things, b/c I'm experienced with the code and where things go, but it's work. So, a question back to you: what do you feel is the right thing for both you and me, and for the project overall? If you're on Discord and want to chat, give me a ping @jz (I should put this in a wiki page!)

basic back-end stuff

• create table textbookmarks db migration (see lute/db/schema/migrations for examples) to add the new table and relationship back to the texts table, with necessary fields
• add db.Table mapping class for class TextBookmark in lute/models/book.py
• add smoke unit tests in tests/orm/test_Book.py or /test_Text.py to ensure that db mappings are valid
  • TextBookmark is saved correctly with the Text
  • Text can be deleted if it has a TextBookmark (b/c users can delete pages)
  • deleting Book deletes any TextBookmark associated with its pages

basic smoke testing for the UI

• need a smoke test for the controller that returns the datatables data for textbookmark data for a given book (?)
• ideally there would be an acceptance test to check that everything is created correctly -- that's a big thing to start looking at though

[cblanken]

@jzohrab I'd like to take a stab at it if that's okay.

I'm pretty familiar with Python and as long as this issue isn't urgent, I'd like to use it as an opportunity to learn the codebase better. That way it will be easier for me to contribute more in the future. I should have some time to dig into the initial backend stuff this weekend.

[jzohrab]

Hi @cblanken totally ok. Ping me on discord if you have questions, hopefully I will have time to answer. Thanks!

[jzohrab]

Launched in 3.5.0. :tada: