android: the note editor (M12 step 6)
Tapping a card now opens something. Until this commit the phone could create,
find and navigate; it could not change anything.
A FULL SCREEN, not a sheet. Capture is a sheet because the board behind it is
reassurance that the thought landed; editing is a sustained task with the
keyboard up, and a sheet would spend the whole time fighting the IME for the
bottom half of the display. Full screen also puts the actions in a bottom bar,
which is where a thumb already is. The note's colour paints the whole screen,
so opening one reads as the same object growing to fill the display.
Text saves ONCE, on close — plus on ON_STOP, so app-switching mid-paragraph
doesn't lose it. Not debounced autosave: the core snapshots a revision on every
title/body change, so saving per typing pause would fill version history with
near-identical entries. A baseline check means opening a note and backing out
writes nothing at all, rather than bumping updated_at and marking it dirty for
sync. Same shape the web editor settled on, for the same reason.
The editor speaks in ACTIONS, not callbacks. The first version passed a bundle
of twenty lambdas and the doc comment on it was already worrying about two of
the same-shaped ones getting swapped, with nothing to catch it. `EditorAction`
plus one `(EditorAction) -> Unit` costs a `when` at the far end and buys
exhaustiveness: adding a variant breaks the dispatcher until it is handled.
Checklist rows are live here — real checkboxes, editable text, remove, and an
add row that keeps focus so a list types straight through. That is the answer
to the open question about list entry: the capture sheet stays one-item-per-
line because at capture time the list is already in your head and a tap per row
is the slow part; the editor is where a list is REVISED, and revising is
item-at-a-time. Row text commits on focus loss, not per keystroke — each commit
is a store write that reloads the note.
Colour, labels and reminders are bottom sheets. Reminders lead with presets
(later today / tomorrow / next week) and keep the exact picker one tap down:
the web's raw datetime-local is right for a desktop and three taps too many for
the common case on a phone. Recurrence only appears once there is a reminder to
recur from. The date picker reports UTC midnight of the calendar day tapped and
is read back in UTC — reading it in the device zone is the classic off-by-a-day
in that control.
Pin, labels, archive and delete live in the overflow as WORDS.
`material-icons-core` has no pin, archive or label glyph, and the alternatives
were pulling in the ~1,000-vector extended set for four icons or pressing
unrelated ones into service — a star meaning "pin" is a star meaning "favourite"
to everyone who has used another app. The colour button is a dot in the note's
current colour, which says what the colour IS as well as what the button does.
A trashed note renders read-only. Editing one would silently resurrect work
that was meant to be thrown away; Restore and Delete forever are the only
things to do with it. Deleting for good is the one irreversible action in the
app and gets the one confirmation in it.
`#tag` labels are never sent to `set_labels` and get no remove button. They are
owned by the body text and the core re-derives them on the next edit, so a
cross that undid itself a second later would look broken.
FFI additions: delete_note_forever, add_item, set_item_text, set_item_checked,
delete_item, complete_reminder, snooze_reminder, set_note_labels, create_label.
`set_item_text`/`set_item_checked` are split rather than exposing the core's
{text?, checked?} patch, for the same reason NoteEdit is a list — an
optional-field struct cannot say "leave this alone" in Kotlin without colliding
with "set it to null". Four new tests (11 total in the crate).
Found while extracting shared helpers: the card painted EVERY reminder blue,
so "you missed this" and "coming up Friday" looked identical. Now red when
overdue and neutral otherwise, matching the web card's exact pairs. And the
error banner was renderable only by the board — the one screen that needed it,
where the writes happen, was the one screen without it.
DRY, since three copies each had appeared: PlainTextField (the undecorated
field used by capture, editor, checklist rows and the search bar), Time.kt (the
RFC3339 seam), NoteKind.kt, ErrorBanner.
detekt: LongMethod and LongParameterList now ignore @Composable. Compose breaks
those rules' PREMISE, not just their thresholds — a composable's parameters are
its UI contract and its length tracks how many elements are on screen, not
branching. Two suppressions carry their reasoning at the site instead:
onEditorAction is sixty lines because EditorAction has twenty variants, and
splitting it would need an `else` that throws away the exhaustiveness; and
BoardViewModel stays one class because every editor mutation has to reload the
board behind it.
Verified locally before pushing, per ci-requirements.md: fmt/clippy/test in
ci-tauri:1.97 (89 + 11 + 11 tests, four crates present), ktlint and detekt in
ci-rust-android:1.97, uniffi bindings generated from a host build and read to
confirm every method and field name the Kotlin calls.
Still unbuilt: attachments, link previews, version history, and label
management (rename/recolour/delete). Setting up a server from the phone is next.
Scribe #2777
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -212,6 +212,127 @@ impl ThoughtSync {
|
||||
.map_err(CoreError::store)
|
||||
}
|
||||
|
||||
/// Remove a note permanently.
|
||||
///
|
||||
/// Returns nothing, unlike every other mutation here: there is no note left to
|
||||
/// return. The core also records a pending delete, so a linked device tells the
|
||||
/// server rather than having the next pull resurrect the row.
|
||||
pub fn delete_note_forever(&self, id: String) -> Result<(), CoreError> {
|
||||
let conn = self.db.conn().map_err(CoreError::store)?;
|
||||
local::store::delete_forever(&conn, &id).map_err(CoreError::store)
|
||||
}
|
||||
|
||||
// ──────────────────────────── checklist items ────────────────────────────
|
||||
//
|
||||
// Every one of these returns the whole reloaded note rather than the item it
|
||||
// touched. That is the core's shape, and it is the right one for a UI: ticking
|
||||
// a box changes `updated_at` and can change what the board shows, so handing
|
||||
// back only the item would leave Kotlin to guess at the rest.
|
||||
|
||||
pub fn add_item(&self, note_id: String, text: String) -> Result<Note, CoreError> {
|
||||
let conn = self.db.conn().map_err(CoreError::store)?;
|
||||
local::store::add_item(&conn, ¬e_id, &text)
|
||||
.map(Note::from)
|
||||
.map_err(CoreError::store)
|
||||
}
|
||||
|
||||
/// Retitle one item.
|
||||
///
|
||||
/// Split from `set_item_checked` rather than exposing the core's
|
||||
/// `{text?, checked?}` patch, for the same reason `NoteEdit` exists: an
|
||||
/// optional-field struct cannot say "leave this alone" in Kotlin without
|
||||
/// colliding with "set it to null", and two unambiguous calls beat one
|
||||
/// ambiguous one when each is three lines.
|
||||
pub fn set_item_text(
|
||||
&self,
|
||||
note_id: String,
|
||||
item_id: String,
|
||||
text: String,
|
||||
) -> Result<Note, CoreError> {
|
||||
self.patch_item(¬e_id, &item_id, serde_json::json!({ "text": text }))
|
||||
}
|
||||
|
||||
pub fn set_item_checked(
|
||||
&self,
|
||||
note_id: String,
|
||||
item_id: String,
|
||||
checked: bool,
|
||||
) -> Result<Note, CoreError> {
|
||||
self.patch_item(
|
||||
¬e_id,
|
||||
&item_id,
|
||||
serde_json::json!({ "checked": checked }),
|
||||
)
|
||||
}
|
||||
|
||||
pub fn delete_item(&self, note_id: String, item_id: String) -> Result<Note, CoreError> {
|
||||
let conn = self.db.conn().map_err(CoreError::store)?;
|
||||
local::store::delete_item(&conn, ¬e_id, &item_id)
|
||||
.map(Note::from)
|
||||
.map_err(CoreError::store)
|
||||
}
|
||||
|
||||
// ─────────────────────────────── reminders ───────────────────────────────
|
||||
|
||||
/// Clear the reminder, marking it dealt with.
|
||||
///
|
||||
/// Distinct from `NoteEdit::ClearRemindAt` even though today they do the same
|
||||
/// thing: the core reserves this one for "the reminder fired and is finished",
|
||||
/// which is where recurrence advancement lands when it is built. A UI that
|
||||
/// called the generic clear instead would silently stop recurring reminders
|
||||
/// from recurring the day that changes.
|
||||
pub fn complete_reminder(&self, id: String) -> Result<Note, CoreError> {
|
||||
let conn = self.db.conn().map_err(CoreError::store)?;
|
||||
local::store::complete_reminder(&conn, &id)
|
||||
.map(Note::from)
|
||||
.map_err(CoreError::store)
|
||||
}
|
||||
|
||||
/// Push the reminder out by `minutes` from now.
|
||||
///
|
||||
/// The core computes the new instant from its own clock rather than taking one
|
||||
/// from the caller — so "in an hour" means the same thing on every surface,
|
||||
/// and a phone with a skewed clock can't write a reminder the server reads as
|
||||
/// already past.
|
||||
pub fn snooze_reminder(&self, id: String, minutes: i64) -> Result<Note, CoreError> {
|
||||
let conn = self.db.conn().map_err(CoreError::store)?;
|
||||
local::store::snooze_reminder(&conn, &id, minutes)
|
||||
.map(Note::from)
|
||||
.map_err(CoreError::store)
|
||||
}
|
||||
|
||||
// ───────────────────────────────── labels ────────────────────────────────
|
||||
|
||||
/// Replace the note's MANUAL labels.
|
||||
///
|
||||
/// `#tag` labels are owned by the body text and the core re-derives them on
|
||||
/// every body edit, so they are deliberately untouched here. A picker that
|
||||
/// sent the full visible set would strip a tag label the text still mandates —
|
||||
/// and the next keystroke in the body would put it straight back, which is the
|
||||
/// kind of fight a UI should never pick with its store.
|
||||
pub fn set_note_labels(
|
||||
&self,
|
||||
note_id: String,
|
||||
label_ids: Vec<String>,
|
||||
) -> Result<Note, CoreError> {
|
||||
let conn = self.db.conn().map_err(CoreError::store)?;
|
||||
local::store::set_labels(&conn, ¬e_id, &label_ids)
|
||||
.map(Note::from)
|
||||
.map_err(CoreError::store)
|
||||
}
|
||||
|
||||
/// Find or create a label by name, returning it either way.
|
||||
///
|
||||
/// Find-or-create rather than create: the core matches case-insensitively, so
|
||||
/// typing "Errands" when "errands" exists has to attach the existing label
|
||||
/// instead of minting a near-duplicate that then diverges on colour.
|
||||
pub fn create_label(&self, name: String) -> Result<Label, CoreError> {
|
||||
let conn = self.db.conn().map_err(CoreError::store)?;
|
||||
local::store::create_label(&conn, &name)
|
||||
.map(Label::from)
|
||||
.map_err(CoreError::store)
|
||||
}
|
||||
|
||||
// ─────────────────────────────── sync ────────────────────────────────
|
||||
|
||||
pub fn sync_status(&self) -> Result<SyncStatus, CoreError> {
|
||||
@@ -333,6 +454,22 @@ impl ThoughtSync {
|
||||
/// Helpers, deliberately NOT exported — uniffi only binds what an `#[uniffi::export]`
|
||||
/// block names, so these stay Rust-side.
|
||||
impl ThoughtSync {
|
||||
/// Apply a `{text}` or `{checked}` patch to one checklist item.
|
||||
///
|
||||
/// The two public setters differ only in the key they write, and the lock +
|
||||
/// convert + map-error dance around it is identical, so it lives once here.
|
||||
fn patch_item(
|
||||
&self,
|
||||
note_id: &str,
|
||||
item_id: &str,
|
||||
changes: serde_json::Value,
|
||||
) -> Result<Note, CoreError> {
|
||||
let conn = self.db.conn().map_err(CoreError::store)?;
|
||||
local::store::update_item(&conn, note_id, item_id, &changes)
|
||||
.map(Note::from)
|
||||
.map_err(CoreError::store)
|
||||
}
|
||||
|
||||
/// The server URL + token, or the `NotLinked` state. Every networked call needs
|
||||
/// exactly this, and none of them may hold the lock past it.
|
||||
fn credentials(&self) -> Result<(String, String), CoreError> {
|
||||
@@ -479,4 +616,152 @@ mod tests {
|
||||
|
||||
std::fs::remove_dir_all(&dir).ok();
|
||||
}
|
||||
|
||||
/// The editor's whole checklist loop, in one pass: add a row, tick it, retitle
|
||||
/// it, drop it. Each call returns the reloaded note, which is what the UI
|
||||
/// splices back into the board rather than re-querying.
|
||||
#[test]
|
||||
fn checklist_items_can_be_added_ticked_retitled_and_removed() {
|
||||
let dir = scratch_dir();
|
||||
let app = ThoughtSync::new(dir.clone()).expect("a fresh data dir should open");
|
||||
let note = app
|
||||
.create_note(NoteDraft {
|
||||
title: "Packing".to_string(),
|
||||
body: String::new(),
|
||||
color: "default".to_string(),
|
||||
kind: Some("list".to_string()),
|
||||
items: Some(vec!["socks".to_string()]),
|
||||
})
|
||||
.expect("create");
|
||||
assert_eq!(note.items.len(), 1);
|
||||
|
||||
let with_two = app
|
||||
.add_item(note.id.clone(), "charger".to_string())
|
||||
.expect("add");
|
||||
assert_eq!(with_two.items.len(), 2);
|
||||
// Appended, not prepended — a new row belongs at the bottom of the list the
|
||||
// user is looking at.
|
||||
assert_eq!(with_two.items[1].text, "charger");
|
||||
|
||||
let item_id = with_two.items[1].id.clone();
|
||||
let ticked = app
|
||||
.set_item_checked(note.id.clone(), item_id.clone(), true)
|
||||
.expect("tick");
|
||||
assert!(ticked.items[1].checked);
|
||||
assert_eq!(
|
||||
ticked.items[1].text, "charger",
|
||||
"ticking a box must not disturb its text — the two setters write \
|
||||
different columns and neither may clear the other"
|
||||
);
|
||||
|
||||
let renamed = app
|
||||
.set_item_text(note.id.clone(), item_id.clone(), "usb-c cable".to_string())
|
||||
.expect("rename");
|
||||
assert_eq!(renamed.items[1].text, "usb-c cable");
|
||||
assert!(
|
||||
renamed.items[1].checked,
|
||||
"and the same in the other direction"
|
||||
);
|
||||
|
||||
let trimmed = app
|
||||
.delete_item(note.id.clone(), item_id)
|
||||
.expect("delete item");
|
||||
assert_eq!(trimmed.items.len(), 1);
|
||||
assert_eq!(trimmed.items[0].text, "socks");
|
||||
|
||||
std::fs::remove_dir_all(&dir).ok();
|
||||
}
|
||||
|
||||
/// A `#tag` in the body owns its label. The picker replaces MANUAL labels only,
|
||||
/// so sending an empty set must not strip one the text still mandates —
|
||||
/// otherwise the next body edit would re-derive it and the UI would appear to
|
||||
/// fight itself.
|
||||
#[test]
|
||||
fn setting_labels_leaves_tag_derived_ones_alone() {
|
||||
let dir = scratch_dir();
|
||||
let app = ThoughtSync::new(dir.clone()).expect("a fresh data dir should open");
|
||||
|
||||
let note = app
|
||||
.create_note(draft("Trip", "book the ferry #travel"))
|
||||
.expect("create");
|
||||
assert_eq!(
|
||||
note.labels.len(),
|
||||
1,
|
||||
"the #tag should have attached a label"
|
||||
);
|
||||
assert!(note.labels[0].via_tag);
|
||||
|
||||
let errands = app
|
||||
.create_label("errands".to_string())
|
||||
.expect("create label");
|
||||
let tagged = app
|
||||
.set_note_labels(note.id.clone(), vec![errands.id.clone()])
|
||||
.expect("set labels");
|
||||
assert_eq!(tagged.labels.len(), 2);
|
||||
|
||||
let cleared = app
|
||||
.set_note_labels(note.id.clone(), vec![])
|
||||
.expect("clear manual labels");
|
||||
assert_eq!(cleared.labels.len(), 1);
|
||||
assert!(cleared.labels[0].via_tag);
|
||||
|
||||
// Find-or-create, not create: a second "Errands" must be the same label,
|
||||
// or the picker mints near-duplicates that then diverge on colour.
|
||||
let again = app
|
||||
.create_label("Errands".to_string())
|
||||
.expect("create label again");
|
||||
assert_eq!(again.id, errands.id);
|
||||
|
||||
std::fs::remove_dir_all(&dir).ok();
|
||||
}
|
||||
|
||||
/// Deleting forever has to actually remove the row, and the note must then be
|
||||
/// unreadable rather than merely hidden.
|
||||
#[test]
|
||||
fn deleting_forever_removes_the_note() {
|
||||
let dir = scratch_dir();
|
||||
let app = ThoughtSync::new(dir.clone()).expect("a fresh data dir should open");
|
||||
let note = app.create_note(draft("Ephemeral", "body")).expect("create");
|
||||
|
||||
app.delete_note_forever(note.id.clone())
|
||||
.expect("delete forever");
|
||||
assert!(
|
||||
app.get_note(note.id.clone()).is_err(),
|
||||
"a permanently deleted note must not still load"
|
||||
);
|
||||
|
||||
std::fs::remove_dir_all(&dir).ok();
|
||||
}
|
||||
|
||||
/// Snooze writes a future instant from the CORE's clock; complete clears it.
|
||||
#[test]
|
||||
fn reminders_can_be_snoozed_and_completed() {
|
||||
let dir = scratch_dir();
|
||||
let app = ThoughtSync::new(dir.clone()).expect("a fresh data dir should open");
|
||||
let note = app.create_note(draft("Call back", "")).expect("create");
|
||||
assert_eq!(note.remind_at, None);
|
||||
|
||||
let snoozed = app.snooze_reminder(note.id.clone(), 60).expect("snooze");
|
||||
let at = snoozed.remind_at.expect("snoozing must set a reminder");
|
||||
let parsed = chrono_free_parse(&at);
|
||||
assert!(
|
||||
parsed > 0,
|
||||
"the reminder must be a parseable RFC3339 instant, got {at:?}"
|
||||
);
|
||||
|
||||
let done = app.complete_reminder(note.id.clone()).expect("complete");
|
||||
assert_eq!(done.remind_at, None);
|
||||
|
||||
std::fs::remove_dir_all(&dir).ok();
|
||||
}
|
||||
|
||||
/// A crude RFC3339 sanity check that doesn't pull a date crate into this
|
||||
/// crate's dev-dependencies to assert one field is well-formed.
|
||||
fn chrono_free_parse(raw: &str) -> usize {
|
||||
if raw.len() >= 20 && raw.as_bytes()[4] == b'-' && raw.contains('T') {
|
||||
raw.len()
|
||||
} else {
|
||||
0
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user