-
Notifications
You must be signed in to change notification settings - Fork 9
Feature Request: Keyboard-first text selection & improved annotations #18 #21
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
base: main
Are you sure you want to change the base?
Conversation
| { | ||
| "permissions": { | ||
| "allow": [ | ||
| "Bash(cargo fmt:*)", | ||
| "Bash(cargo build:*)", | ||
| "Bash(rustup default:*)", | ||
| "Bash(cargo check:*)" | ||
| ] | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i don't think this needs to be committed.
| fn default_annotation_highlight_color() -> String { | ||
| "7FB4CA".to_string() // Cyan (base0C) from Kanagawa theme | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This introduces a separate color configuration that bypasses the existing Base16Palette theme system. Could we use a color from the palette instead? That way it would automatically work with all themes.
the current approach looks really harsh in most of predefined themes.

--
Also i think it would make it more visually neutral (and easier to conform to themes) to use underline for the text being annotated.
|
|
||
| /// Apply multiple word-range highlights to spans for annotated text | ||
| /// This highlights the text that has been annotated with a background color | ||
| fn apply_word_ranges_highlighting( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function is doing a lot - settings retrieval, color parsing, word-to-char conversion, range merging, and span splitting.
i believe this needs to be simplified
| pub chapter_href: String, | ||
| pub target: CommentTarget, | ||
| pub content: String, | ||
| pub selected_text: Option<String>, // The text that was highlighted/selected |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This stores the annotated text, but we can always derive it from the word range and the book content at render time. I don't this is really needed. this can always be derived from original epub.
| && self.text_reader.start_comment_input(); | ||
|
|
||
| if success { | ||
| debug!("Started comment input mode from visual selection"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
consider removing useless logging. this doesn't help in debugging that much
| } | ||
| FocusedPanel::Main(MainPanel::Content) => { | ||
| "j/k: Scroll | h/l: Chapter | Ctrl+d/u: Half-screen | Tab: Switch | Space+o: Open | q: Quit" | ||
| "j/k: Scroll | h/l: Chapter | Ctrl+d/u: Half-screen | v/V: Visual Mode | Tab: Switch | Space+o: Open | q: Quit" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets not add this :) this will ruin a lot test for no good reason
marc-alexis-com
opened 2 days ago
Pain points
Text selection requires mouse (drag/double-click/triple-click). Breaks vim workflow.
Want: Vim visual mode (v, V) for keyboard selection.
Annotated text isn't highlighted in the reader - can't see what I've already noted.
Want: Highlight annotated text with a distinct color.
For this to work you must edit and add the line
annotation_highlight_color: "076678"
or whatever highlighting color you would like to your .bookokrat_settings.yaml
YAML only stores position, not the actual text - can't export without re-parsing EPUB.
Want: Store highlighted_text in YAML alongside position.
No way to export annotations to markdown.
Want: Space+e to export all annotations as markdown.
Example edited /.bookokrat_settings.yaml file
.bookokrat_settings.yaml