Android comments describe the code to a stranger (#6223)
CI & Build / Python lint (push) Successful in 2s
CI & Build / Build now, or wait for Android? (push) Successful in 3s
Android / Build, or is the channel already serving this? (push) Successful in 3s
Desktop (Tauri) / Build, or is the channel already serving this? (push) Successful in 2s
Desktop (Tauri) / Web tests, clippy, Rust tests and rustfmt (push) Skipped
Desktop (Tauri) / Tauri desktop (Linux) (push) Skipped
Desktop (Tauri) / Windows installer (cross-compiled) (push) Skipped
Desktop (Tauri) / Update manifest (push) Skipped
CI & Build / Web typecheck and unit tests (push) Successful in 9s
CI & Build / Python tests (push) Successful in 15s
Android / Core and FFI clippy and tests (push) Successful in 27s
CI & Build / integration (push) Successful in 1m39s
CI & Build / Build & push image (push) Skipped
Android / Kotlin + Rust (APK) (push) Successful in 7m55s
Android / Build the server image (push) Successful in 6s

Milestone 546 "Bring the repo to the comment standard (rule 32) and
compose defaults (rule 198)", step #6223 "Comment standard sweep:
Android client".

KDoc, comments, manifest and resource notes, Gradle and TOML comments
and the ffi crate's docs now state what the code does and the
non-obvious why in the present tense. Issue, Scribe-note, milestone
(M13, M304, M315, M12) and family-idea references are gone, as are
comparisons to Minstrel and the narration of what used to be: the
capture sheet the editor replaced, the per-note colour story, the
operator's device-pass feedback, the bindgen-as-[[bin]] failure, the
0.60 edge alpha before it was re-solved, and the checker bugs behind
check-strings/check-symbols. Hazards stay as facts: the signing key that
can't be rotated, the alias that keeps the earlier name, PKCS12's single
password, unwind over abort, the UTC date-picker read, the
DerivedTint/colors.ts mirror.

One stale claim is gone: DerivedTint said the web had no test runner,
but notes/grammar.test.ts runs the same tint cases. detekt.yml said
"these two places" over a list of five.

49 files, comments only: a comment-stripping diff of every changed file
against HEAD is identical.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
2026-10-21 20:24:52 -04:00
co-authored by Claude Opus 5.5
parent 715b648b95
commit 0a631219ee
49 changed files with 332 additions and 491 deletions
+25 -35
View File
@@ -10,17 +10,16 @@ plugins {
val workspaceRoot: Directory = layout.projectDirectory.dir("../..")
// The ABIs a release APK carries. arm64 is essentially every real device; armv7
// covers older 32-bit hardware; the two x86 targets are what emulators run on, and
// dropping them would make the app untestable on a desktop emulator (the reason
// x86_64 was added to the old Tauri lane in task 1864).
// covers older 32-bit hardware; the two x86 targets are what emulators run, so the
// app stays testable on a desktop emulator.
val androidAbis = listOf("arm64-v8a", "armeabi-v7a", "x86", "x86_64")
/**
* Cross-compile `inkwell-ffi` for each Android ABI and drop the resulting
* `.so` into jniLibs, where AGP packages it.
*
* `ExecOperations` injected rather than `project.exec`: the latter was REMOVED in
* Gradle 9, and reaching for `project` at execution time is also what breaks the
* `ExecOperations` injected rather than `project.exec`: Gradle 9 has no
* `project.exec`, and reaching for `project` at execution time breaks the
* configuration cache this build has enabled.
*/
abstract class CargoNdkBuild : DefaultTask() {
@@ -60,16 +59,15 @@ abstract class CargoNdkBuild : DefaultTask() {
commandLine(listOf("cargo") + args)
workingDir = workspaceDir.get().asFile
if (release) {
// The workspace's release profile with two of its settings put back,
// for this build only. Environment overrides rather than a profile of
// our own, because cargo-ndk copies its `-o` output from the
// `release` directory and nothing says it understands any other one;
// the desktop's binaries are untouched either way.
// The workspace's release profile with two settings put back, for this
// build only. Environment overrides rather than a profile of our own,
// because cargo-ndk copies its `-o` output from the `release`
// directory; the desktop's binaries are untouched either way.
//
// Keep the symbol table: `strip = true` removes the symbols uniffi's
// `--library` mode reads the interface out of, and bindgen then fails
// with "No UniFFI metadata found" (run 4077). There is no debug info
// in a release build to strip, so this keeps symbols and nothing else.
// with "No UniFFI metadata found". A release build has no debug info to
// strip, so this keeps symbols and nothing else.
environment("CARGO_PROFILE_RELEASE_STRIP", "debuginfo")
// Unwind, so a panic in the core reaches Kotlin as an exception the
// board can show rather than ending the app. See the ffi crate's docs.
@@ -141,17 +139,13 @@ val rustInputs =
/**
* Which Cargo profile the `.so` is built with.
*
* A property rather than a debug/release task PAIR, deliberately. This runner has
* no working Gradle or Cargo cache (`reserveCache failed` on every run), so a cold
* cross-compile of four ABIs costs about four minutes — and a lane that both
* type-checks and packages would pay that twice if the two used different
* profiles. `android.yml` picks one profile and uses it for every Gradle call in
* the run.
* A property rather than a debug/release task pair: this runner has no working
* Gradle or Cargo cache, so a cold cross-compile of four ABIs costs about four
* minutes, and a lane that type-checks and packages with different profiles would
* pay it twice. `android.yml` picks one profile for every Gradle call in the run.
*
* CI passes `release` for a signed APK, so a phone runs an optimised store and sync
* engine. Until Scribe #2810 it passed `debug`, because the release profile's
* `strip = true` broke bindgen; `CargoNdkBuild` now keeps the symbols for this
* build alone.
* engine; `CargoNdkBuild` keeps the symbols bindgen needs.
*/
val rustProfile =
(project.findProperty("INKWELL_CARGO_PROFILE") as String?)?.takeIf { it.isNotBlank() }
@@ -187,8 +181,7 @@ android {
defaultConfig {
applicationId = "com.fabledsword.inkwell"
// 26 (Android 8, 2017) matches Minstrel and clears the NDK's floor with
// room to spare.
// 26 (Android 8, 2017) clears the NDK's floor with room to spare.
minSdk = 26
targetSdk = 36
// Injected by CI from the git tag + commit count for a release; "dev"
@@ -212,11 +205,10 @@ android {
}
// The signing key reaches this build only through the environment: CI decodes
// it from a secret into a file and points ANDROID_KEYSTORE_FILE at that path.
// It is never in the repo and never in this file. Generated by the operator
// and never seen by an agent session, because an Android signing key cannot be
// rotated without the original — v3 lineage needs it — so a leaked or lost one
// means every install has to be removed and replaced by hand.
// it from a secret into a file and points ANDROID_KEYSTORE_FILE at that path. It
// is never in the repo. An Android signing key cannot be rotated without the
// original (v3 lineage needs it), so a leaked or lost one means every install
// has to be removed and replaced by hand.
val keystoreFile = System.getenv("ANDROID_KEYSTORE_FILE")?.takeIf { it.isNotBlank() }
val keystorePassword = System.getenv("ANDROID_KEYSTORE_PASSWORD")?.takeIf { it.isNotBlank() }
@@ -225,13 +217,11 @@ android {
create("release") {
storeFile = File(keystoreFile)
storePassword = keystorePassword
// Hardcoded, and NOT a secret: the alias is fixed for the life of
// this app and is written into the certificate every install
// already carries. Hiding it would buy nothing and stop this file
// describing its own signing setup. It still says `thoughtsync`
// after the rename to Inkwell, because it names the key inside the
// existing keystore, and renaming it would only stop that key being
// found. Same key, so the signing certificate did not change either.
// Hardcoded, and NOT a secret: the alias is fixed for the life of this
// app and written into the certificate every install carries. It says
// `thoughtsync`, the app's earlier name, because it names the key
// inside the existing keystore; renaming it would stop the key being
// found.
keyAlias = "thoughtsync"
// PKCS12 cannot hold a key password distinct from the store
// password — keytool refuses to set one — so this is the same
+24 -29
View File
@@ -25,9 +25,8 @@
FOREGROUND_SERVICE used only for expedited work; nothing here asks
for it, and it arrives with the library
Verified against the built APK's merged manifest, not assumed. Noted here
because all four appear in the app's permission list and nothing else in
this file would explain where they came from.
Listed because all four appear in the app's permission list and nothing
else in this file would explain where they came from.
-->
<!--
@@ -64,24 +63,22 @@
refusing costs precision, not the feature — see Reminders.scheduleNext.
RECEIVE_BOOT_COMPLETED already arrives via WorkManager (below), but is
declared here too because ReminderReceiver now depends on it directly. A
declared here too because ReminderReceiver depends on it directly. A
permission this file relies on should be visible in this file.
-->
<!--
Updating this app from the server it syncs with (M12 step 7).
Updating this app from the server it syncs with.
REQUEST_INSTALL_PACKAGES lets the app hand an APK to the system installer at
all. It is NOT what makes an install look suspicious to on-device heuristics
— Mihon declares it too — the legacy ACTION_VIEW install intent was, and this
app uses a PackageInstaller session instead. See AppUpdate.kt and Scribe note
2437. The person must additionally grant "install unknown apps" in system
settings; the update card asks before downloading anything.
all. It is NOT what makes an install look suspicious to on-device heuristics;
the legacy ACTION_VIEW install intent is, and this app uses a PackageInstaller
session instead (AppUpdate.kt). The person must also grant "install unknown
apps" in system settings; the update card asks before downloading anything.
UPDATE_PACKAGES_WITHOUT_USER_ACTION (API 31+) is what removes the install
confirmation on the UPDATE path, and only there — Android will not let an app
silently put a NEW package on a device, which is correct. It also only applies
when the new build is signed with the same key as the installed one, which is
why signing had to land before any of this could work.
UPDATE_PACKAGES_WITHOUT_USER_ACTION (API 31+) removes the install confirmation
on the UPDATE path only: Android will not let an app silently put a NEW package
on a device. It applies only when the new build is signed with the same key as
the installed one.
-->
<uses-permission android:name="android.permission.REQUEST_INSTALL_PACKAGES" />
<uses-permission android:name="android.permission.UPDATE_PACKAGES_WITHOUT_USER_ACTION" />
@@ -91,12 +88,11 @@
<uses-permission android:name="android.permission.RECEIVE_BOOT_COMPLETED" />
<!--
allowBackup stays true, which family idea #5105 (practice 12) says to turn
off. Inkwell is local-first, so an unlinked phone's notes exist only on the
phone, and Android's backup is their one other copy. The device token is what
the practice protects, and it is stored sealed under a Keystore key that never
leaves the phone (KeystoreTokenSeal.kt). A backup therefore carries the notes
and a token nothing can open, and a restored app asks to sign in again.
allowBackup stays true. Inkwell is local-first, so an unlinked phone's notes
exist only on the phone, and Android's backup is their one other copy. The
device token is stored sealed under a Keystore key that never leaves the phone
(KeystoreTokenSeal.kt), so a backup carries the notes and a token nothing can
open, and a restored app asks to sign in again.
-->
<application
android:name=".InkwellApplication"
@@ -111,12 +107,11 @@
launchMode="singleTop" exists for the SHARE filters below.
The reminder notification adds FLAG_ACTIVITY_SINGLE_TOP to its own
intent, so onNewIntent already worked for that one. A share intent is
built by the OTHER app — Chrome, a reader, the text-selection toolbar —
and nothing here can add a flag to it. Without singleTop declared on the
activity itself, every share while the app is running would stack a
second MainActivity on top of the first: a second view model, a second
board, and a back press that lands on a stale copy of the same app.
intent, but a share intent is built by the OTHER app (Chrome, a reader,
the text-selection toolbar) and nothing here can add a flag to it.
Without singleTop on the activity, every share while the app is running
would stack a second MainActivity: a second view model, a second board,
and a back press that lands on a stale copy of the app.
-->
<activity
android:name=".MainActivity"
@@ -135,7 +130,7 @@
Text, and images one at a time or several at once. A shared image
becomes a note carrying it as an attachment, stored on the phone
and uploaded on the next sync that reaches a server (#5169).
and uploaded on the next sync that reaches a server.
image/* rather than */*: a photo or a screenshot is what people
share into a notes app. Claiming every type would put Inkwell in the
@@ -180,7 +175,7 @@
Where the system reports what happened to an install we committed. Not
exported: the only sender is the PendingIntent this app handed to
PackageInstaller. Without it a failed install would be indistinguishable
from someone declining the dialog (Scribe #2438).
from someone declining the dialog.
-->
<!--
Hands an attachment to the app that opens its type. Not exported, as a
@@ -18,24 +18,20 @@ import java.io.File
*
* ## A PackageInstaller session, not an install intent
*
* The obvious route — `ACTION_VIEW` on the APK with
* `application/vnd.android.package-archive` — is the one on-device install
* heuristics are tuned against, and it is what produces the "bypassing Android
* security" warning the operator saw on Minstrel (Scribe note 2437). It also never
* tells the OS that this app is the legitimate updater of its own package, and it
* returns nothing: a failed install is indistinguishable from a person dismissing
* the dialog.
* The obvious route, `ACTION_VIEW` on the APK with
* `application/vnd.android.package-archive`, is the one on-device install
* heuristics are tuned against, and it produces a "bypassing Android security"
* warning. It also never tells the OS that this app is the legitimate updater of its
* own package, and it returns nothing: a failed install is indistinguishable from a
* person dismissing the dialog.
*
* A session says who is doing what. On Android 12+ it can also declare that no user
* action is required, which — paired with `UPDATE_PACKAGES_WITHOUT_USER_ACTION` —
* removes the confirmation dialog entirely on the UPDATE path. Not on a first
* install: the OS will not let an app quietly put a NEW package on a device, which
* is right.
* action is required, which, paired with `UPDATE_PACKAGES_WITHOUT_USER_ACTION`,
* removes the confirmation dialog on the UPDATE path. Not on a first install: the
* OS will not let an app quietly put a NEW package on a device.
*
* Two things from that same research that are NOT done here, deliberately:
* `setRequestUpdateOwnership` was chased and turned out to be a red herring, and
* `REQUEST_INSTALL_PACKAGES` is not the differentiator either — Mihon declares it
* too. The mechanism was the whole difference.
* Neither `setRequestUpdateOwnership` nor `REQUEST_INSTALL_PACKAGES` is what avoids
* the warning; the session is.
*
* ## The outcome comes back
*
@@ -57,8 +53,7 @@ object AppUpdate {
* A debuggable build is signed with a debug key (CI's unsigned path, or a local
* build), and Android refuses the release-signed APK over it with
* INSTALL_FAILED_UPDATE_INCOMPATIBLE. Offering that update would be a download
* that can only fail, so a debug build says so instead (family idea #5103,
* practice 9).
* that can only fail, so a debug build says so instead.
*/
fun selfUpdates(context: Context): Boolean =
(context.applicationInfo.flags and ApplicationInfo.FLAG_DEBUGGABLE) == 0
@@ -125,10 +120,9 @@ object AppUpdate {
val params =
PackageInstaller.SessionParams(PackageInstaller.SessionParams.MODE_FULL_INSTALL)
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S) {
// Only honoured on an UPDATE of an app signed with the same key —
// exactly our case, and the reason the signing work had to land
// first. Android ignores it for anything else rather than failing,
// so there is no need to guard on which it is.
// Only honoured on an UPDATE of an app signed with the same key, which
// is this case. Android ignores it for anything else rather than
// failing, so there is no need to guard on which it is.
params.setRequireUserAction(PackageInstaller.SessionParams.USER_ACTION_NOT_REQUIRED)
}
@@ -150,11 +144,8 @@ object AppUpdate {
}
/**
* Stream the APK into the session.
*
* Its own function only because the two nested `use` blocks read badly inline —
* and detekt agreed, which is fair: a stream inside a session inside a try is
* three things to hold at once.
* Stream the APK into the session. Its own function so a stream inside a session
* inside a try isn't three nested things to hold at once.
*/
private fun writeApk(
session: PackageInstaller.Session,
@@ -12,10 +12,9 @@ import android.content.Context
*
* Returns null rather than a fallback string, because the two callers want
* different ones: the UI wants a localized "unknown" from string resources, the
* client header wants the literal the core recognizes. Note 3127 §5 governs both —
* with no version tags, the artifact's self-report is the only answer to "which
* build is this?", so a missing name must read as missing and never as a plausible
* default that nothing can contradict.
* client header wants the literal the core recognizes. With no version tags, the
* artifact's self-report is the only answer to "which build is this?", so a missing
* name must read as missing, never as a plausible default.
*/
fun Context.installedVersionName(): String? =
runCatching { packageManager.getPackageInfo(packageName, 0).versionName }.getOrNull()
@@ -32,11 +32,9 @@ class InkwellApplication : Application() {
override fun onCreate() {
super.onCreate()
// Introduce this app to any server it links to, before anything can sync.
// The core cannot name us — the same crate is compiled into the desktop app,
// and it used to announce every phone as `inkwell-desktop` carrying the
// core crate's own version. "unknown" rather than a guess when the package
// manager will not say (note 3127 §5).
// Introduce this app to any server it links to, before anything can sync. The
// core can't name its host app, since the same crate is compiled into the
// desktop app. "unknown" rather than a guess when the package manager won't say.
setClientAgent("inkwell-android", installedVersionName() ?: "unknown")
try {
@@ -12,8 +12,8 @@ import javax.crypto.SecretKey
import javax.crypto.spec.GCMParameterSpec
/**
* Keeps the device token sealed under a key in the Android Keystore (family idea
* #5105, practice 12). The core stores what this returns, never the token itself.
* Keeps the device token sealed under a key in the Android Keystore. The core
* stores what this returns, never the token itself.
*
* The key never leaves this phone, so a copy of the app's files yields a token
* nothing can open. That includes Android's backup, which stays on: an unlinked
@@ -23,8 +23,6 @@ import javax.crypto.spec.GCMParameterSpec
* Both methods answer null rather than throw, because an exception crossing into
* the core would be a panic there. The core reads null from [sealToken] as "store
* it as is for now" and from [openToken] as "sign in again".
*
* Shaped after Minstrel's KeystoreSessionVault (Scribe snippet #5025).
*/
class KeystoreTokenSeal : TokenSeal {
override fun sealToken(token: String): String? =
@@ -205,7 +205,7 @@ private enum class Screen { BOARD, EDITOR, SYNC, TAGS }
* both cover the display completely, so keeping the board's two-column grid
* measuring and recomposing underneath one would be pure waste.
*
* Still no navigation library. Four destinations, each entered from exactly one
* No navigation library. Four destinations, each entered from exactly one
* place and left by back — a nav graph would be ceremony around an enum, and the
* state that actually matters (which note is open, whether this device is linked)
* already lives in view models.
@@ -257,9 +257,8 @@ private fun App(
// the display, and nothing in the store cares. Saveable so a rotation does not
// close it.
//
// The capture sheet used to keep its own flag here too. It is gone: the + button
// opens the editor on an unsaved draft, so writing a note and editing one are the
// same surface with the same toolbar.
// There is no separate capture sheet: the + button opens the editor on an unsaved
// draft, so writing a note and editing one are the same surface.
var showingSync by rememberSaveable { mutableStateOf(false) }
var showingTags by rememberSaveable { mutableStateOf(false) }
@@ -446,16 +445,12 @@ private fun ReminderAlarms(core: Inkwell) {
}
/**
* Looking for an app update without being asked.
*
* Until this existed, `check()` had exactly one caller: a button on the sync screen.
* So a new build was found only by someone who went looking for one, and the operator
* had to remember to go looking — which is the same as not being told.
* Looking for an app update without being asked, so a new build is found without
* anyone going to look for one.
*
* On coming forward rather than on a timer: it is the moment the person is present,
* and the view model rate-limits so flicking between two apps is not a re-check.
* Unlinked devices are skipped entirely — updates come from a linked server, and
* there is nothing to ask.
* Unlinked devices are skipped, since updates come from a linked server.
*/
@Composable
private fun AutomaticUpdate(
@@ -26,7 +26,7 @@ import com.fabledsword.inkwell.ui.epochMillis
* ## AlarmManager, not WorkManager
*
* The background sync runs on WorkManager and is right to: nobody minds whether it
* happens at 3:05 or 3:19. A reminder minded very much. WorkManager's periodic
* happens at 3:05 or 3:19. A reminder does. WorkManager's periodic
* floor is fifteen minutes and it batches work into maintenance windows, so
* "remind me at 09:00" would routinely arrive at 09:14 — which is not a reminder,
* it is a rebuke.
@@ -3,8 +3,8 @@ package com.fabledsword.inkwell
import android.content.Context
/**
* When each tag's lens was last opened on this phone, for the drawer's recent tags
* (#5931). Opening a tag counts as using it, alongside the notes that carry it.
* When each tag's lens was last opened on this phone, for the drawer's recent tags.
* Opening a tag counts as using it, alongside the notes that carry it.
*
* Kept on the device rather than synced: it is a convenience about where this
* person's thumb goes on this screen, and losing it costs one tap.
@@ -12,8 +12,7 @@ import android.util.Log
* Without this the app would `commit` and learn nothing — a failed install would
* look exactly like a person deciding not to go ahead, and the update card would
* sit there claiming an update is available with no explanation of why nothing
* happened. That was the specific complaint recorded against Minstrel's first
* attempt (Scribe #2438).
* happened.
*
* The result is written to [UpdateOutcome] rather than notified: the app is on
* screen when this fires — someone just tapped Update — so the place to say it is
@@ -10,7 +10,7 @@ import kotlin.math.roundToInt
/**
* Whether a type is drawn as a picture rather than offered as a file. SVG is
* excluded on every surface: it is a document that can carry script (#1981).
* excluded on every surface: it is a document that can carry script.
* The same rule as `rendersInline` in the web's `notes/attachments.ts`.
*/
fun rendersInline(mime: String): Boolean {
@@ -43,7 +43,7 @@ import com.fabledsword.inkwell.R
* The note's body, as fields and checkboxes rather than as markup.
*
* The point of the whole shape: a box you can tick while looking at the note, rather
* than `- [ ] ` to read and edit around. What the note IS never changed.
* than `- [ ] ` to read and edit around. The note's stored markup is the same.
*/
@Composable
fun BlockBody(
@@ -93,8 +93,8 @@ fun BlockBody(
val remaining = blocks.withoutIndex(index)
onChange(remaining)
// The row above — or, for the FIRST row, whichever one takes
// its place. `index - 1` alone is -1 there, which left the
// keyboard up with nothing focused.
// its place. `index - 1` alone is -1 there, which would leave
// the keyboard up with nothing focused.
onFocus(remaining.getOrNull((index - 1).coerceAtLeast(0))?.id)
},
)
@@ -118,7 +118,7 @@ fun BlockBody(
}
/**
* A run of prose: one ordinary multi-line field, exactly as the editor always had.
* A run of prose: one ordinary multi-line field.
*
* Leaving it is when a `- [ ] ` typed by hand becomes a real checklist item — see
* [promotingTasks] for why blur is the only safe moment to do that.
@@ -165,8 +165,7 @@ private fun TaskBlock(
onDelete: () -> Unit,
) {
// Material sizes every interactive component to a 48dp touch target, and on a
// checklist that IS the row height — which is why six items filled a phone screen
// even after the field's own padding came off.
// checklist that IS the row height.
CompositionLocalProvider(LocalMinimumInteractiveComponentSize provides ROW_TOUCH) {
Row(verticalAlignment = Alignment.CenterVertically) {
Checkbox(
@@ -207,25 +206,23 @@ private fun TaskBlock(
* Material's floor is 48dp and this is deliberately under it. That floor is sized for
* a control somebody has to find; a checklist box sits in a predictable column with an
* identical box directly above and below, and the cost of a near miss is ticking the
* neighbouring item — visible, and undone by tapping again. Trading twelve of those
* dp for a list that fits on a screen is what was asked for, twice.
* neighbouring item — visible, and undone by tapping again. Twelve of those dp buy a
* list that fits on a screen.
*/
private val ROW_TOUCH = 36.dp
/**
* The field a block is typed into.
*
* `BasicTextField`, not the Material one [PlainTextField] wraps, and the reason is
* density. Material's TextField puts 16dp above and below its text — padding that
* makes a FORM field comfortable to hit, and that on a checklist IS the row height. It
* made six items twice as tall as the six items, which is what the operator saw.
* `BasicTextField`, not the Material one [PlainTextField] wraps, for density.
* Material's TextField puts 16dp above and below its text: padding that makes a FORM
* field comfortable to hit, and that on a checklist IS the row height.
*
* Nothing is lost by dropping down a layer. `PlainTextField` exists to strip a
* container and an indicator; `BasicTextField` never had either, so there is no box
* here to drift back into existence. What it does not supply and this must:
* `BasicTextField` has no container or indicator to strip. What it does not supply
* and this must:
*
* - the text COLOUR. It defaults to `Color.Unspecified`, which draws BLACK — the same
* default that made the editor's toolbar invisible in dark mode. Set, not inherited.
* - the text COLOUR. It defaults to `Color.Unspecified`, which draws BLACK and is
* invisible in dark mode. Set, not inherited.
* - the cursor brush, which would otherwise be black for the same reason.
* - the placeholder, which is a plain Text behind the field rather than a slot.
*
@@ -13,10 +13,10 @@ import androidx.compose.ui.input.pointer.pointerInput
import androidx.compose.ui.layout.LayoutCoordinates
import com.fabledsword.inkwell.core.Note
// Reordering the board by dragging a card, as on the web (#5176): hold a card, then
// move it. The hold is the same long press that opens the card's menu, so the menu
// appears first and closes as the card starts to move. Lifting without moving
// leaves the menu open, as before. Moving BEFORE the hold completes is a scroll.
// Reordering the board by dragging a card, as on the web: hold a card, then move it.
// The hold is the same long press that opens the card's menu, so the menu appears
// first and closes as the card starts to move. Lifting without moving leaves the
// menu open. Moving BEFORE the hold completes is a scroll.
//
// Watched on the Initial pass, ahead of the grid's scrolling and the card's own
// click handling. Nothing is consumed until the hold has completed and the finger
@@ -23,9 +23,9 @@ import com.fabledsword.inkwell.core.Label
import com.fabledsword.inkwell.core.NoteFacets
/**
* The board's filters: the same three the web's Filters panel offers (FilterBar.vue,
* trimmed to these in #5180). Tags a note must ALL carry, has an attachment, and
* shared with me. Reminders and dates have their own lenses, here as on the web.
* The board's filters: the same three the web's Filters panel offers (FilterBar.vue).
* Tags a note must ALL carry, has an attachment, and shared with me. Reminders and
* dates have their own lenses, here as on the web.
*
* Only the main board filters, as on the web. Opening Archive, Trash or a tag from
* the drawer starts that view unfiltered. A search narrows whichever view is open,
@@ -356,10 +356,9 @@ private fun SearchBar(
) {
Surface(
// No `statusBarsPadding()` here. The Scaffold this sits in already applies
// the system-bar insets to its content padding, so adding them again put
// the whole status bar's height of empty space above the search field —
// roughly a centimetre of nothing at the top of the first screen anyone
// sees. Insets get consumed once, by whichever component owns the edge.
// the system-bar insets to its content padding, so adding them again would
// put a status bar's height of empty space above the search field. Insets
// get consumed once, by whichever component owns the edge.
modifier =
Modifier
.fillMaxWidth()
@@ -410,7 +409,7 @@ private fun NavigationDrawer(
onOpenSync: () -> Unit,
onManageTags: () -> Unit,
) {
// Every tag, once asked for; until then the few in use (#5931).
// Every tag, once asked for; until then the few in use.
var allTags by rememberSaveable { mutableStateOf(false) }
val shown = if (allTags) labels else recentTags(labels, { recency[it.id] ?: 0L }, { it.name })
@@ -444,7 +443,7 @@ private fun NavigationDrawer(
modifier = Modifier.padding(horizontal = 12.dp),
)
// Tags last, as on the web (#5915): it is the one list here that grows
// Tags last, as on the web: it is the one list here that grows
// without limit, so everything with a fixed place sits above it.
//
// The header renders even with no tags, unlike the rows below it: the
@@ -78,8 +78,8 @@ data class BoardState(
/**
* A search narrows the board in view rather than replacing it, as on the web,
* where the box writes `?q=` beside the Filters panel's facets and one query
* answers both (Scribe #2942). "These words, in notes tagged grocery" is the
* question tags exist to help ask (note 2930).
* answers both. "These words, in notes tagged grocery" is the question tags exist
* to help ask.
*/
val searching: Boolean get() = query.isNotBlank()
@@ -297,10 +297,8 @@ class BoardViewModel(
/**
* Open the editor on a note that does not exist yet.
*
* The + button used to raise a separate capture sheet, which meant a note being
* WRITTEN could not be given a colour, a reminder or a checklist — those live on
* the editor's toolbar, and the sheet had none. Writing and editing are now the
* same surface.
* The + button opens this rather than a separate capture sheet, so a note being
* WRITTEN can be given a colour, a reminder or a checklist from the same toolbar.
*
* The draft is a real [Note] carrying [DRAFT_ID] rather than a null, so the
* editor renders it without knowing that "not saved yet" is a state it can be
@@ -462,7 +460,7 @@ class BoardViewModel(
EditorAction.DismissError -> dismissError()
// Sent on an idle debounce while typing, and again on close. Writing
// this often is affordable because a body write no longer snapshots a
// this often is affordable because a body write doesn't snapshot a
// revision — the core keeps one per editing session, not one per save.
is EditorAction.SaveText ->
mutate { it.updateNote(id, listOf(NoteEdit.Body(action.body))) }
@@ -567,12 +565,9 @@ class BoardViewModel(
* lie than showing it one frame stale.
*
* While a search is running the QUERY is re-run rather than the board's
* destination — running `load` here would replace the hits with the whole
* board, which is why this branch exists at all. It used to keep the existing
* list instead, and that was right for a note whose place in the pile changed
* and wrong for one that left it: trashing a hit left the card sitting there,
* with a snackbar saying it was gone, until the query happened to re-run
* (#3111). Re-asking is still the answer to the query, just a current one.
* destination: running `load` here would replace the hits with the whole board,
* and keeping the old list would leave a trashed hit on screen beside a snackbar
* saying it was gone.
*/
private fun mutate(
closeEditor: Boolean = false,
@@ -596,9 +591,8 @@ class BoardViewModel(
notes = notes,
// Only REFRESHES an open editor; it must never open one.
// `editing != null` IS "the editor is on screen", so writing
// the reloaded note in unconditionally meant any mutation
// started from the BOARD threw the editor open on top of it —
// which is exactly what ticking a checkbox on a card did.
// the reloaded note in unconditionally would throw the editor
// open over the board whenever a card's checkbox is ticked.
editing = if (closeEditor) null else state.editing?.let { updated ?: it },
saving = false,
error = notice(),
@@ -619,7 +613,7 @@ class BoardViewModel(
* rather than the pure text helpers the editor uses: nothing here is holding a
* half-typed body, so the reloaded note is simply the truth.
*
* `index` is the item's ordinal, which is what its id is now (M304).
* `index` is the item's ordinal, which is its id.
*/
fun toggleItem(
note: Note,
@@ -6,11 +6,7 @@ package com.fabledsword.inkwell.ui
// identical grey chips. Hashing the tag's NAME is deterministic, identical on every
// surface, costs no column and no migration, and a tag keeps its colour for life.
//
// THIS WAS THE CARD'S COLOUR TOO, ONCE. It is not any more (M315): a note's fill is
// one neutral and only its tags carry hue. The hash survived that removal because the
// job it still does — give a name a stable colour — was never the job that failed.
// What failed was asking a colour that means "which tag" to also mean nothing at all
// on an untagged note, at which point the board had two vocabularies and neither read.
// A note's fill is one neutral; only its tags carry hue.
//
// THIS IS HALF A MIRRORED PAIR. `frontend/src/notes/colors.ts` computes the same hash
// over the same key order, and the two must agree exactly or a tag is one colour on
@@ -20,9 +16,8 @@ package com.fabledsword.inkwell.ui
//
// NO COMPOSE IN THIS FILE, deliberately. It is the half of the pair that CAN be
// pinned by a host-JVM test, and staying free of `androidx.compose` is what keeps
// `DerivedTintTest` runnable in the Unit tests step rather than on an emulator. The
// web side has no test runner at all, so this test is the only mechanical guard the
// mirror gets — see the fixture comment in colors.ts.
// `DerivedTintTest` runnable in the Unit tests step rather than on an emulator. See
// the fixture comment in colors.ts.
/**
* The colours a derived hue can land on: `NOTE_TINTS`' keys minus `default`, which is
@@ -79,12 +74,10 @@ fun derivedTint(id: String): String {
* `default`: the server mints one as `Label(owner_id=…, name=name)` with no colour,
* so without deriving, a board of tags would be a board of identical grey chips.
*
* DERIVED RATHER THAN PERSISTED AT MINT TIME, reversing #2965's plan. That plan wanted
* a hashed colour written at each of the four places a label can be born — and named
* the risk itself: `find_or_create_label` is "easy to miss, and it is the common one",
* since most tags are born from typing `#grocery`, not from a management screen.
* Deriving has no mint points to miss and no backfill for the tags already out there.
* The cost is that renaming a tag recolours it, which is fair: the name IS the tag.
* Derived rather than stored when a label is created: most tags are born from typing
* `#grocery` through `find_or_create_label`, not from a management screen, and
* deriving has no creation point to miss and nothing to backfill. The cost is that
* renaming a tag recolours it, which is fair: the name IS the tag.
*
* Folded because `#Todo` renamed to `#todo` is the same tag and should not change
* colour. [foldTag] and the web's are both locale-independent lowercasing, so the
@@ -104,6 +97,6 @@ fun resolvedLabelColor(
/**
* How two tag names compare: equal once every letter is lowercased, accented ones
* included, so `#Café` and `#CAFÉ` are one tag. The core's `derive::fold`, the server's
* `labeling.named` and the web's `foldTag` fold the same way (#5385).
* `labeling.named` and the web's `foldTag` fold the same way.
*/
fun foldTag(name: String): String = name.lowercase()
@@ -5,12 +5,10 @@ import android.net.Uri
/**
* Everything the editor can ask for, as one type.
*
* The alternative was a bundle of twenty callbacks, and it was a bad one: twenty
* same-shaped `(String, String) -> Unit` parameters is a place for two of them to
* get swapped, with nothing to catch it. One `(EditorAction) -> Unit` costs a
* `when` at the far end and gets EXHAUSTIVENESS in exchange — adding a variant
* here breaks the dispatcher until it is handled, which is precisely the guarantee
* the callback bundle could not offer.
* One `(EditorAction) -> Unit` rather than twenty same-shaped `(String, String) ->
* Unit` callbacks, where two could get swapped with nothing to catch it. It costs a
* `when` at the far end and buys EXHAUSTIVENESS: adding a variant here breaks the
* dispatcher until it is handled.
*
* No variant carries a note id. The editor is open on exactly one note and the
* dispatcher already has it, so threading it through every action would only
@@ -41,12 +39,10 @@ sealed interface EditorAction {
data object DeleteForever : EditorAction
// No checklist actions at all any more (M304). An item is a `- [ ] ` line of the
// body, so adding, renaming, ticking or deleting one is editing text — which the
// editor already does, through SaveText, with the same autosave and the same
// revision window as any other edit. Routing them through the store would have
// meant the store handing back a note whose body disagreed with the field the
// person was typing in.
// No checklist actions. An item is a `- [ ] ` line of the body, so adding,
// renaming, ticking or deleting one is editing text, through SaveText, with the
// same autosave and revision window as any other edit. Routing them through the
// store would hand back a note whose body disagrees with the field being typed in.
/**
* The note's MANUAL labels, replacing whatever was there.
@@ -9,7 +9,7 @@ import com.fabledsword.inkwell.core.checklistRender
/**
* One piece of a note body, as the editor DRAWS it.
*
* The note is still one markdown string underneath (M304) — this is a rendering and
* The note is still one markdown string underneath — this is a rendering and
* input shape, and nothing below the editor can tell it exists. [joinBlocks] puts the
* string back together on every edit.
*
@@ -36,8 +36,8 @@ data class EditorBlock(
/**
* Split a body into blocks, numbering them from [firstId].
*
* Which lines are items comes from the core, not from a pattern here — the grammar is
* written three times already and Kotlin is not going to be the fourth.
* Which lines are items comes from the core, not from a pattern here: the grammar has
* three implementations already, and this file isn't a fourth.
*/
fun splitBlocks(
body: String,
@@ -95,13 +95,11 @@ val blocksSaver: Saver<List<EditorBlock>, String> =
/**
* What the return key does on a checklist item.
*
* `internal` rather than private because BlockBody.kt calls it. These three helpers
* are the block MODEL and the composables are the block UI — one file was doing both,
* which detekt noticed by counting functions before anybody noticed by reading.
* `internal` rather than private because BlockBody.kt calls it: these helpers are the
* block MODEL, and the composables in BlockBody.kt are the block UI.
*
* On one with words in it, a new empty item below. On an EMPTY one, the item becomes
* prose — which is how a list ENDS, and the same rule the plain text field used
* before this: without it a list is impossible to get out of.
* prose, which is how a list ENDS: without it a list is impossible to get out of.
*
* Deliberately appends rather than splitting at the caret. Splitting an item in two is
* a rarity, and the caret is at the end for every ordinary use of this key.
@@ -47,17 +47,11 @@ import com.fabledsword.inkwell.core.Note
/**
* The editor's action bar, along the top of the surface.
*
* It sits exactly where the capture sheet's drag handle used to. The handle cost
* this strip of screen and did nothing that a back gesture does not already do, so
* the strip carries the actions instead.
*
* Top rather than bottom, now that this one surface is used for WRITING as well as
* editing: the keyboard owns the bottom of the display for most of a note's life,
* so a bar down there spends its time riding on the IME. That is the right place
* for a send button and the wrong one for a colour picker, which is reached for
* between thoughts rather than at the end of them. The cost is honest — the top of
* a phone is further from a thumb than the bottom — and it buys a bar that does not
* move while you type.
* Top rather than bottom because this surface is used for WRITING: the keyboard owns
* the bottom of the display for most of a note's life, so a bar down there would ride
* on the IME. That suits a send button, not a colour picker reached for between
* thoughts. The top is further from a thumb, and buys a bar that does not move while
* you type.
*
* The three affordances with a permanent slot are the ones reached for while still
* writing — colour, reminder, note-or-list. Everything structural (pin, labels,
@@ -84,17 +78,16 @@ fun EditorTopBar(
onConfirmDelete: () -> Unit,
onAction: (EditorAction) -> Unit,
) {
// Someone else's note (#5175): its text, at `edit`, and this account's own pin
// and archive (#5176) are all that may change here, so the bar keeps the
// Someone else's note: its text, at `edit`, and this account's own pin
// and archive are all that may change here, so the bar keeps the
// checklist button and an overflow of those two, and nothing that is the owner's.
val owner = access == NoteAccess.OWNER
val dark = isSystemInDarkTheme()
TopAppBar(
title = {},
navigationIcon = {
// The only way out, and the only thing that needed a "save" button
// before writes became continuous. Leaving IS saving now, which is what
// the line in the bottom corner is there to say out loud.
// The only way out. Writes are continuous, so leaving IS saving, which is
// what the line in the bottom corner says out loud.
BackButton(onClose, R.string.editor_back)
},
actions = {
@@ -108,7 +101,7 @@ fun EditorTopBar(
}
if (!readOnly) {
// Inserts `- [ ] ` at the caret. Always available, and never hidden:
// a checklist is text now (M304), so there is no section to be
// a checklist is text, so there is no section to be
// already-showing and no reason a second list cannot start further
// down the same note.
IconButton(onClick = onStartChecklist) {
@@ -139,14 +132,11 @@ fun EditorTopBar(
)
}
},
// EXPLICIT, and not optional — the same lesson the old bottom bar learned.
// Material derives a bar's content colour from its container via
// contentColorFor(), which maps a colour-SCHEME ROLE to its `on-` pair and
// returns Unspecified for anything else. The card surface is a plain constant
// and not a role, so the icons drew with no colour filter: black vectors on a
// near-black bar, a toolbar that rendered the whole time and was invisible in
// dark mode. STILL TRUE with one neutral surface — it is the same kind of
// value, so this stays exactly as it is.
// EXPLICIT, and not optional. Material derives a bar's content colour from its
// container via contentColorFor(), which maps a colour-SCHEME ROLE to its `on-`
// pair and returns Unspecified for anything else. The card surface is a plain
// constant, not a role, so without this the icons draw with no colour filter:
// black vectors on a near-black bar, invisible in dark mode.
//
// onSurface for the actions too, not the default onSurfaceVariant: the bar has
// to read against the card rather than the board, and the muted variant does
@@ -164,36 +154,24 @@ fun EditorTopBar(
/**
* The footer: when the note was last written, and the way out.
*
* **Where the note stands.** There is no save button, and there should not be — a
* note is saved continuously, so a button offering to do what already happened is a
* lie with a tap attached. But that left nothing on screen saying the work is safe,
* and "closing this keeps it" is not a thing anyone should have to be told twice. So
* the state says it, as a fact rather than an instruction: Not saved yet → Saving… →
* Edited just now is the whole lifecycle, and someone who watches it once never has
* to wonder again.
* **Where the note stands.** There is no save button: a note is saved continuously,
* so a button offering to do what already happened would be a lie with a tap
* attached. The state says the work is safe instead, as a fact: Not saved yet →
* Saving… → Edited just now is the whole lifecycle.
*
* **The way out.** Down here because of where hands are. Moving the toolbar to the
* top took the back arrow with it, which left the only exit from a full-screen
* editor in the top-left corner — the furthest point on the display from a
* right-handed thumb, and reached over the whole note to get to. The operator hit
* that on the first device pass and was right to. So the exit lives in the bottom
* corner, which with the keyboard up sits directly above it.
* **The way out.** Down here because of where hands are. The top-left corner is the
* furthest point on the display from a right-handed thumb, so the exit lives in the
* bottom corner, which with the keyboard up sits directly above it. The top-left
* arrow stays as well: it is what habit, the system back gesture and TalkBack all
* expect of a full-screen surface.
*
* The top-left arrow stays as well. Two affordances for one action is usually
* clutter, but this is the case that earns it: the arrow is what habit, the system
* back gesture and TalkBack all expect of a full-screen surface, and removing it
* would strand the reflex to strike a duplicate that costs one icon slot.
*
* A checkmark, at the operator's ask. I had shipped the word "Done" here on the
* argument that a tick in a NOTES app reads as a checklist item; overruled, and the
* filled treatment is what settles it — a tonal button in the note's own colour is
* plainly a control, where a bare glyph beside a checklist would not be. It carries
* "Done" as its content description, so the reasoning survives where it actually
* mattered: read aloud.
* A checkmark in a filled tonal button, so it reads plainly as a control rather than
* as a checklist tick. Its content description is "Done", which is what it says read
* aloud.
*
* [DateUtils] rather than a hand-rolled formatter: it is localised, it already
* knows the difference between minutes, hours and yesterday, and getting plurals
* right in every language is not this app's problem to solve twice.
* knows the difference between minutes, hours and yesterday, and it gets plurals
* right in every language.
*/
@Composable
fun EditorFooter(
@@ -206,7 +184,7 @@ fun EditorFooter(
modifier =
modifier
.fillMaxWidth()
// Rides above the keyboard, like the bar that used to be here. The
// Rides above the keyboard. The
// content Column deliberately does not also inset for the IME:
// Scaffold measures this row at its lifted height and passes the
// inset down.
@@ -224,16 +202,10 @@ fun EditorFooter(
)
FilledTonalIconButton(
onClick = onClose,
// The BRAND, matching the board's compose FAB — the app's one existing
// statement of "this is the affirmative action here", now reused rather
// than a second one invented.
//
// This wore the note's own tint until M315, on the argument that it would
// otherwise be the one element on a tinted card ignoring the tint. There is
// no tint to ignore any more, and the alternative — Material's default
// secondaryContainer — is a baseline M3 colour this theme never sets, so
// taking the default would put an off-brand lilac in the corner of the
// editor.
// The BRAND, matching the board's compose FAB: the app's one statement of
// "this is the affirmative action here". Material's default
// secondaryContainer is a baseline M3 colour this theme never sets, so the
// default would put an off-brand lilac in the corner of the editor.
colors =
IconButtonDefaults.filledTonalIconButtonColors(
containerColor = MaterialTheme.colorScheme.primary,
@@ -51,10 +51,6 @@ import java.time.temporal.TemporalAdjusters
// The two things you pick rather than type: a set of labels, and a time.
//
// It was three. The colour sheet went with `note.color` in M315 — a card is one neutral
// surface now and colour lives on the tag, so the swatch grid was a control with nothing
// behind it.
//
// All bottom sheets rather than dialogs. A dialog takes the middle of the screen
// and asks to be dismissed; a sheet rises from the bottom, under the thumb, with
// the note still visible above it — which matters when the choice you are making
@@ -21,9 +21,8 @@ import com.fabledsword.inkwell.R
// Shared by the board and the editor.
//
// A failed save is most likely to happen WHILE the editor is open — that is where
// the writes are — so a banner only the board could render meant the one screen
// that needed it was the one screen without it.
// A failed save is most likely WHILE the editor is open, since that is where the
// writes are, so the editor needs the banner as much as the board does.
@Composable
fun ErrorBanner(
@@ -49,7 +49,7 @@ private val TRAILING_PUNCTUATION = charArrayOf('.', ',', ';', ':', '!', '?')
* The link a note that is nothing but a link files its preview under, or null.
*
* Trimmed the way the server's `unfurl_queue.detect_urls` trims it, since that is the
* key the preview arrives under (#5399). The web's `loneUrl` is the same rule, and
* key the preview arrives under. The web's `loneUrl` is the same rule, and
* both run the `urls` cases in `core/testdata/grammar.json` (here, by hand).
*/
fun loneUrl(body: String): String? =
@@ -92,9 +92,8 @@ fun isLoneUrl(note: Note): Boolean = loneUrl(note.body) != null
* The image sits in a strip down the left edge, cropped to the card's height, as in
* the web's `LinkPreview.vue`. `image_url` is a REMOTE third-party address — the
* linked page's og:image — so the phone fetches it from whatever host the link
* points at, exactly as a browser does for the web card. The operator chose that
* parity knowingly (Scribe #3307). Coil's disk cache means a card scrolled past
* twice costs one download, not two.
* points at, exactly as a browser does for the web card. Coil's disk cache means a
* card scrolled past twice costs one download, not two.
*/
@Composable
fun LinkPreviewCard(
@@ -106,7 +105,7 @@ fun LinkPreviewCard(
// and the two paddings are named first. `standard:chain-method-continuation`
// wants a `.` that follows a MULTILINE element glued to its closing paren —
// `).padding(…)` — which is unreadable, so the multiline element is avoided
// instead (Scribe #3110).
// instead.
val shape = RoundedCornerShape(PREVIEW_RADIUS)
val padH = if (compact) 8.dp else 10.dp
val padV = if (compact) 6.dp else 8.dp
@@ -3,10 +3,10 @@ package com.fabledsword.inkwell.ui
import com.fabledsword.inkwell.core.Note
/**
* How this account holds a note (#5175), read from the core's `permission`.
* How this account holds a note, read from the core's `permission`.
*
* Someone else's note at [EDIT] may have its TEXT changed here; at [VIEW] it may
* not. Either way its pin and archive are this account's own (#5176). The core
* not. Either way its pin and archive are this account's own. The core
* refuses the rest anyway — this only keeps the editor from offering what would be
* refused.
*/
@@ -67,8 +67,7 @@ fun NoteCard(
// NAMED rather than written inline in the chain below, and not for taste: ktlint's
// chain-method-continuation wants the next `.` glued to the closing paren of a
// multiline element — `).background(…)` — which is worse to read than a modifier
// with a name. Every other multiline element in this codebase happens to be last
// in its chain, so this is the first place the rule bites.
// with a name.
val opening =
Modifier.combinedClickable(
onClickLabel = stringResource(R.string.board_open_note),
@@ -147,13 +146,12 @@ private fun CardContents(
// NoteBody, which shows the URL. Never a blank card.
val lonePreview = remember(note.body, note.previews) { loneUrlPreview(note) }
// Body then checklist, in order — a note can carry both (M13 step 2). The
// first line of the body IS the note's name, at the same weight as the rest of
// it (M13 steps 3 and 4).
// Body then checklist, in order — a note can carry both. The first line of the
// body IS the note's name, at the same weight as the rest of it.
if (lonePreview != null) {
LinkPreviewCard(preview = lonePreview, compact = false)
} else if (note.body.isNotBlank()) {
// A note shared with us to view has inert boxes (#5175): ticking one is
// A note shared with us to view has inert boxes: ticking one is
// changing its text, which is not ours to do.
NoteBody(
note = note,
@@ -164,7 +162,7 @@ private fun CardContents(
// Links mentioned INSIDE a note: a compact strip at the foot of the card,
// under the note's own words rather than stacked on top of them. Putting
// them above would set a stranger's headline where the note's first line
// should be — the web learned that in M13 and moved them down.
// should be. The web does the same.
if (!isLoneUrl(note) && note.previews.isNotEmpty()) {
Spacer(Modifier.height(8.dp))
note.previews.forEach { preview ->
@@ -218,7 +216,7 @@ private fun NoteMenu(
onAction: (EditorAction) -> Unit,
onConfirmDelete: () -> Unit,
) {
// Pin and archive are this account's own on someone else's note too (#5176);
// Pin and archive are this account's own on someone else's note too;
// trash is the owner's. A note shared with us never reaches our Trash, so the
// trashed set is only ever the owner's.
val owner = note.access == NoteAccess.OWNER
@@ -239,12 +237,12 @@ private fun NoteMenu(
* The note's body, with its checklist drawn where it actually sits.
*
* Rendered line by line rather than as one block of text, because an item is a line
* of the body now (M304) and a card that showed the prose and then the list would put
* every list in the wrong place — and, since the body already contains those lines,
* would show each one twice.
* of the body, and a card that showed the prose and then the list would put every
* list in the wrong place — and, since the body already contains those lines, would
* show each one twice.
*
* Which lines are items is asked of the core rather than matched here. The grammar is
* already written three times; a fourth in Compose would be a fourth place for a
* Which lines are items is asked of the core rather than matched here. The grammar has
* three implementations already; a fourth in Compose would be a fourth place for a
* checklist to change shape when it syncs.
*/
@Composable
@@ -333,12 +331,8 @@ private fun ChecklistRow(
/**
* One string of a note's own words, with every `#tag` in it drawn in that tag's colour.
*
* This is what replaced the chip for a tag still living in the prose. The card used to
* print such a tag twice — once where it was typed and once in the row above — and the
* duplicate was the loud copy, which made a tagged note read as "tag, then some text
* that happens to start with the same word". Colouring it in place says the same thing
* with no furniture, and says it more honestly: the token you can see IS the text you
* would delete to remove the tag.
* A tag still living in the prose is coloured in place rather than repeated as a chip:
* the token you can see IS the text you would delete to remove the tag.
*
* WHICH characters are a tag is asked of the core, exactly as [NoteBody] asks it which
* lines are checklist items. The grammar already exists three times (Rust, Python,
@@ -415,9 +409,8 @@ private fun LabelChips(labels: List<NoteLabel>) {
* The reminder, red once it has passed.
*
* Red for overdue and neutral otherwise, matching the web card exactly — the same
* red-100/red-700 and black/5 pairs, resolved through the shared tint table. It
* used to be blue for every reminder here, which made "you missed this" and
* "coming up on Friday" look identical on a board full of both.
* red-100/red-700 and black/5 pairs, resolved through the shared tint table, so
* "you missed this" and "coming up on Friday" don't look alike on a board of both.
*/
@Composable
private fun ReminderChip(
@@ -428,7 +421,7 @@ private fun ReminderChip(
}
/**
* Whose note this is, when it is not only ours (#5175): "From Robin" on a note someone
* Whose note this is, when it is not only ours: "From Robin" on a note someone
* shared with us, "Shared" on one of ours that we shared. Nothing on a private note.
*/
@Composable
@@ -463,31 +456,21 @@ private val CARD_ELEVATION = 1.dp
// too, so a note opened is the same object as the note on the board.
/**
* THE CARD SURFACE — one neutral per theme (M315).
* THE CARD SURFACE — one neutral per theme.
*
* This used to be a function of the note: a palette fill for a tagged one, a colour
* generated from the id for the rest. Both are gone. The operator's verdict after four
* passes — "my coloring attempt has failed and nothing looks right… we've tried a lot
* to make the color work and somehow it never seems to land" — and the diagnosis under
* it is that a card's fill was being asked to carry meaning it could not carry. Nine
* keys is too few to identify anything on a board of any size, and a generated fill
* identifies nothing by construction, so a coloured board taught the eye to read hue
* as significant and then handed it noise. Colour lives on the TAG now, where the
* thing it names is right beside it.
* A card's fill carries no meaning: nine keys are too few to identify anything on a
* board of any size, and a coloured board teaches the eye to read hue as significant.
* Colour lives on the TAG, beside the thing it names.
*
* The values are `neutral-900` on dark and white on light — exactly what the palette's
* `default` always was, and exactly what the web card and both editors already use, so
* this is a collapse onto a surface every surface already had rather than a new colour
* anybody has to like. Mirrored as `NOTE_CARD_SURFACE` in `frontend/src/notes/colors.ts`
* The values are `neutral-900` on dark and white on light, the same as the web card
* and both editors. Mirrored as `NOTE_CARD_SURFACE` in `frontend/src/notes/colors.ts`
* (`bg-white dark:bg-neutral-900`).
*
* NOT a colour-scheme role: `surface` is the BOARD in this theme (neutral-50 / -950),
* and Material's `surfaceContainer` roles are unset here so they would resolve to
* baseline M3 greys rather than to the web's neutrals. Two hexes matching the web beats
* a role that nearly does.
* baseline M3 greys rather than to the web's neutrals.
*
* Measured, against the operator's "not the same color as their background but close
* to it" — the card fill is deliberately the WEAKEST number on the card:
* The card fill is deliberately the WEAKEST contrast on the card:
*
* card vs board light #FFFFFF on #FAFAFA 1.04
* dark #171717 on #0A0A0A 1.10
@@ -498,10 +481,9 @@ private val CARD_ELEVATION = 1.dp
* muted vs card light #404040 on #FFFFFF 10.37
* dark #E5E5E5 on #171717 14.23
*
* A card is not separated from the board by its fill and never was — the edge and the
* shadow do that, which is why 1.04 is enough and why it has to stay near 1. A fill
* that separated on its own would be a panel, and a board of panels is the wall this
* whole line of work started from.
* The edge and the shadow separate a card from the board, not its fill, which is why
* 1.04 is enough and why it has to stay near 1: a fill that separated on its own would
* make a board of panels.
*/
fun noteCardSurface(dark: Boolean): Color = if (dark) CARD_SURFACE_DARK else CARD_SURFACE_LIGHT
@@ -34,16 +34,14 @@ import kotlinx.coroutines.delay
/**
* The one writing surface: a new note and an existing one are the same screen.
*
* Shaped like the capture sheet it replaced — a rounded card that begins below the
* status bar — so opening a note still reads as something rising over the board
* rather than a place you navigated to. It is full height rather than a real
* `ModalBottomSheet`, and that is the whole trade: a sheet spends a writing session
* negotiating with the IME for the bottom half of the display, and the swipe-down it
* buys is a gesture back already does. The shape is what was worth keeping.
* Shaped like a sheet — a rounded card that begins below the status bar — so opening
* a note reads as something rising over the board rather than a place you navigated
* to. It is full height rather than a real `ModalBottomSheet`: a sheet spends a
* writing session negotiating with the IME for the bottom half of the display, and
* the swipe-down it buys is a gesture back already does.
*
* The card's surface paints the WHOLE sheet rather than a panel inside it, so opening
* a note reads as the same object growing to fill the display — the more literally
* true since M315, where the board and the editor became the same one neutral.
* a note reads as the same object growing to fill the display.
*
* No save button, deliberately. Writes are continuous, so a button offering to do
* what already happened would be a lie with a tap attached; [EditorFooter] in the
@@ -69,7 +67,7 @@ fun NoteEditorScreen(
// throwing away every character typed during the write.
//
// BLOCKS rather than one string, because a checklist item is drawn as a real
// checkbox now and a widget cannot live inside a text field. The note is still one
// checkbox and a widget cannot live inside a text field. The note is still one
// markdown body underneath — see EditorBlock.kt — and `bodyText` is what is saved.
//
// Saveable, because a new note has nothing to fall back on if the phone rotates
@@ -93,7 +91,7 @@ fun NoteEditorScreen(
// Restore and Delete forever as the only things to do with it.
//
// A note shared with us to view is read-only for the same reason it is on the
// web: nothing about it is ours to change (#5175). At edit, the text is, and the
// web: nothing about it is ours to change. At edit, the text is, and the
// rows that are the owner's (tags, reminder, files, previews) stay inert.
val access = note.access
val readOnly = note.readOnlyHere
@@ -125,10 +123,8 @@ fun NoteEditorScreen(
// Idle-debounced autosave. LaunchedEffect cancels and restarts on every
// keystroke, so the delay only ever elapses once typing stops.
//
// Saving this often is affordable because a body write no longer costs a
// revision: history snapshots once per editing session rather than once per
// save. Before that, writing was expensive enough that this editor hoarded
// text until it closed — and an app kill mid-session lost the lot.
// Saving this often is affordable because a body write doesn't cost a revision:
// history snapshots once per editing session rather than once per save.
//
// For a note that does not exist yet this is also what CREATES it, which is why
// every toolbar button works moments after the first keystroke rather than
@@ -173,8 +169,7 @@ fun NoteEditorScreen(
// container), which returns Unspecified for anything that is not a
// colour-SCHEME ROLE. The card surface is not one, so the default publishes
// Unspecified as LocalContentColor and everything inside that does not
// set its own colour draws black — which is how the last toolbar became
// invisible in dark mode.
// set its own colour draws black, invisible in dark mode.
contentColor = MaterialTheme.colorScheme.onSurface,
) {
Scaffold(
@@ -201,7 +196,7 @@ fun NoteEditorScreen(
onAction = onAction,
)
},
// Where the action bar used to be, carrying the two things that
// The two things that
// belong within reach of a thumb: whether the note is safe, and the
// way out. See [EditorFooter] for why the exit is down here and not
// only in the top-left corner.
@@ -242,8 +237,7 @@ fun NoteEditorScreen(
// A note is its body; its NAME is that body's first line, so there
// is nothing separate to type into and nothing rendered bolder than
// the line beneath it (M13 steps 3 and 4). What 2992 changed is only
// how the body is DRAWN — checklist items as boxes rather than as
// the line beneath it. Checklist items draw as boxes rather than as
// the markup for boxes.
BlockBody(
blocks = blocks,
@@ -253,9 +247,9 @@ fun NoteEditorScreen(
onFocus = { focus = it },
)
// No checklist section. The items ARE lines of the field above
// (M304) — rendering them again down here is what would put every
// list on screen twice.
// No checklist section. The items ARE lines of the field above;
// rendering them again down here would put every list on screen
// twice.
if (note.labels.isNotEmpty()) {
EditorLabelRow(note = note, readOnly = !ownerControls, onAction = onAction)
@@ -350,6 +344,6 @@ private const val ANY_TYPE = "*/*"
/**
* The card's top corner radius — Material's extra-large, which is what a bottom
* sheet uses. Same shape as the capture surface this replaced, on purpose.
* sheet uses.
*/
private val SHEET_CORNER = 28.dp
@@ -19,8 +19,8 @@ import com.fabledsword.inkwell.R
* a translucent colour over what's beneath exactly as CSS does, so a panel sits on the
* background the same way in both.
*
* This table is the palette of MEANINGFUL colours: a tag's. A NOTE no longer has one
* at all (M315) — the card is one neutral per theme, held in NoteCard.kt beside the
* This table is the palette of MEANINGFUL colours: a tag's. A NOTE has none — the
* card is one neutral per theme, held in NoteCard.kt beside the
* edge, where the palette cannot reach either of them. What is left here is the chip,
* the inline `#tag`, and the panels and banners that borrow a hue to say what they
* are.
@@ -55,8 +55,8 @@ data class NoteTint(
) {
/**
* The pale fill of a PANEL, a banner, an update card or a picker swatch — the
* places that borrow a hue to say what they are. Not a note's: since M315 a card
* has one neutral surface and does not come through this table at all.
* places that borrow a hue to say what they are. Not a note's: a card has one
* neutral surface and does not come through this table at all.
*/
fun background(dark: Boolean): Color = if (dark) darkBackground else lightBackground
@@ -69,19 +69,16 @@ data class NoteTint(
*
* Kept apart from [tagInk] because the reminder pill is not a tag: it borrows the
* chip's shape and its `red-100`/`black-5` fills, and its `red-700`/`neutral-600`
* text is transcribed from NoteCard.vue's literal classes. The two happened to be
* one value; making the tag ink one step deeper (M315) is where they parted, and
* moving the reminder with it would have silently broken that mirror instead.
* text is transcribed from NoteCard.vue's literal classes, so it follows those,
* not [tagInk].
*/
fun chipForeground(dark: Boolean): Color = if (dark) darkChipForeground else lightChipForeground
/**
* The colour a `#tag` is drawn in — in the note's own words, or as a chip.
*
* A tag whose text is in the body is no longer repeated as a chip (the card was
* printing every tag twice — once where it was typed, once at the top). It is
* tinted in place instead, which is both less furniture and a more honest card:
* the thing you see IS the thing you would delete to remove the tag.
* A tag whose text is in the body is tinted in place rather than repeated as a
* chip: the thing you see IS the thing you would delete to remove the tag.
*/
fun tagInk(dark: Boolean): Color = if (dark) darkTagInk else lightTagInk
@@ -94,7 +91,7 @@ data class NoteTint(
* The fill only tints a shape the edge is drawing.
*
* An edge rather than a heavier fill, because a fill loud enough to hold its own
* shape would be the loudest thing on a board whose whole point is now that the tag
* shape would be the loudest thing on a board whose whole point is that the tag
* is the one coloured thing on it.
*/
fun chipBorder(dark: Boolean): Color = tagInk(dark).copy(alpha = CHIP_EDGE_ALPHA)
@@ -102,18 +99,9 @@ data class NoteTint(
// How strongly a chip's edge is drawn, as a fraction of its own ink.
//
// SOLVED FOR, NOT GUESSED — and re-solved once the answer became solvable. 0.60 was
// picked against the worst case of the time: a chip on a card of its OWN colour, back
// when a note took its first tag's fill. It gave 2.32:1 there, missed the 3:1 of WCAG
// 1.4.11, and the comment here reasoned its way out of that on the grounds that a
// chip's information is its text.
//
// M315 removed that worst case. The edge is now the ink at alpha over a KNOWN fill, so
// the smallest alpha clearing 3:1 for all ten hues is arithmetic rather than judgment:
// 0.60 gives 2.75-3.82 in light and misses for six of the ten, 0.65 gives 3.03-4.36 and
// misses for none. Dark runs 4.52-5.76. The old comment named 0.80 as the fallback if
// the judgment were ever overruled; it is not needed, and it draws a hard outline where
// a hairline does the job.
// SOLVED FOR, NOT GUESSED. The edge is the ink at alpha over the known card fill, and
// 0.65 is the smallest alpha clearing WCAG 1.4.11's 3:1 for all ten hues: 3.03-4.36 in
// light, 4.52-5.76 in dark. 0.60 misses for six of the ten in light.
//
// Mirrored on the web as the `/65` in LABEL_CHIP_SHELL's ring.
private const val CHIP_EDGE_ALPHA = 0.65f
@@ -89,7 +89,7 @@ fun Notice(
/**
* One row of a dropdown menu, closing the menu before it acts.
*
* Lives here rather than beside either menu because there are two now — the
* Lives here rather than beside either menu because there are two — the
* editor's overflow and the board's long-press menu — and they offer the same
* actions in the same words. A second copy of this would be a second place for the
* closing order to be got wrong.
@@ -29,10 +29,9 @@ import androidx.compose.ui.text.input.VisualTransformation
* container and the indicator at each site independently is how they drift apart, so
* it happens once, here.
*
* The note EDITOR no longer comes through this. It dropped to `BasicTextField`
* (see `EditorBlock.kt`) for density: Material's field puts 16dp above and below its
* text, which is right for a form and is the whole row height on a checklist. Nothing
* about "no box" was lost there — BasicTextField never had one.
* The note EDITOR doesn't use this: it uses `BasicTextField` (see `EditorBlock.kt`)
* for density, since Material's field puts 16dp above and below its text, which is
* right for a form and is the whole row height on a checklist.
*
* The disabled colours are stripped too: a trashed note is shown through this
* field read-only, and Material's disabled treatment would grey out text the user
@@ -12,7 +12,7 @@ import com.fabledsword.inkwell.core.Label
import com.fabledsword.inkwell.core.Note
/**
* The drawer lists the few tags in use rather than every tag (#5931), so a long
* The drawer lists the few tags in use rather than every tag, so a long
* list never pushes Archive, Trash and Sync out of reach. Mirrors
* `frontend/src/notes/recentTags.ts`.
*/
@@ -59,7 +59,7 @@ private fun String?.millis(): Long = this?.let(::epochMillis) ?: 0L
/**
* When each tag's lens was last opened here. Opening one counts as using it,
* however it was opened: from the drawer, or a tag on a card (#5931).
* however it was opened: from the drawer, or a tag on a card.
*/
@Composable
internal fun rememberTagViews(destination: Destination): Map<String, Long> {
@@ -23,7 +23,7 @@ data class ShareState(
val noteId: String? = null,
val loading: Boolean = false,
val members: List<Member> = emptyList(),
/** The groups the admin made (#5177); a note shared with one reaches its members. */
/** The groups the admin made; a note shared with one reaches its members. */
val groups: List<ShareGroup> = emptyList(),
val shares: List<NoteShare> = emptyList(),
/** This device has no server, and sharing is between people on one. */
@@ -65,7 +65,7 @@ private fun targetOf(
): ShareTarget = group?.let { ShareTarget.Group(it.id) } ?: ShareTarget.Member(member?.id.orEmpty())
/**
* Sharing a note with other people on the linked server (#5175).
* Sharing a note with other people on the linked server.
*
* A peer of the web's `ShareDialog.vue`. Shares belong to the server, so every call
* here is a network round-trip through the core — `suspend` functions on uniffi's
@@ -118,12 +118,11 @@ fun SyncScreen(
/**
* The build, dim, at the foot of Sync — the same thing the web UI puts at the
* bottom of its rail (#3181).
* bottom of its rail.
*
* Note 3127 §5 is why it is here at all. With version tags gone, an artifact's own
* self-report is the only answer to "which build is this?" — so it renders
* "unknown" rather than nothing when the name is absent, because a blank line looks
* like a layout bug and a plausible default cannot be caught by anything.
* With no version tags, an artifact's own self-report is the only answer to "which
* build is this?", so it renders "unknown" rather than nothing when the name is
* absent: a blank line looks like a layout bug.
*
* The read itself is `installedVersionName()`, shared with the client header the
* app sends its server: one answer to "which build is on this phone", so the line
@@ -27,8 +27,8 @@ enum class LinkMode { PASSWORD, TOKEN }
* A sealed type rather than six loose strings, and not only for the parameter
* count: the two modes genuinely carry different things. `link_with_token` takes
* NO device name — the token was already minted against a named device in the web
* app — so a flat argument list meant collecting one in token mode and silently
* dropping it. Modelling it this way made that impossible to express.
* app — so a flat argument list would collect one in token mode and silently drop
* it.
*/
sealed interface Credentials {
data class Password(
@@ -112,7 +112,7 @@ data class SyncState(
* mid-sync. That is safe by construction rather than by luck: no async path in the
* core holds the store lock across an await, and `last_sync_at` is stamped only
* after both halves of a cycle succeed, so an interrupted sync resumes from the
* stored cursor next time (Scribe #2736).
* stored cursor next time.
*/
class SyncViewModel(
private val core: Inkwell,
@@ -82,9 +82,9 @@ private sealed interface TagDialog {
* because a desktop has room to float one over the board; on a phone this is a
* place you visit to tidy up, and a full screen is what that is.
*
* It is also, since the per-note colour picker was removed, the ONLY colour
* control in the product. That is why the swatch is a first-class tap target on
* every row rather than something behind the overflow menu.
* It is also the ONLY colour control in the product, which is why the swatch is a
* first-class tap target on every row rather than something behind the overflow
* menu.
*/
@OptIn(ExperimentalMaterial3Api::class)
@Composable
@@ -289,9 +289,9 @@ private fun NewTagField(
}
/**
* Said out loud rather than left as a blank screen — and it names the `#` route,
* because the operator did not know `#tag` extraction existed at all (Scribe
* #2949) and this is the natural place to say so.
* Said out loud rather than left as a blank screen, and it names the `#` route:
* typing `#tag` in a note is the easiest way to make a tag, and easy not to know
* about.
*/
@Composable
private fun EmptyTags() {
@@ -491,8 +491,7 @@ private fun MergeDialog(
}
/**
* The palette, and since the per-note picker was removed this is the only place in
* the product a colour is chosen.
* The palette: the only place in the product a colour is chosen.
*
* Every key from [NOTE_TINTS], `default` included: a tag whose colour is
* `default` gets a hue derived from its name (`DerivedTint.kt`), so "default" here
@@ -47,7 +47,7 @@ data class TagsState(
* ## Renaming can merge
*
* `rename_label` folds two tags together when the new name is one another tag
* already holds, and the OLDER row survives (Scribe #3324). So it can return a
* already holds, and the OLDER row survives. So it can return a
* tag whose id is not the one passed in, and it can make another tag stop
* existing. The screen asks first; this view model does not, because a
* confirmation belongs to the surface with a person in front of it.
@@ -61,8 +61,8 @@ private val DarkColors =
/**
* Material 3 in Inkwell's own colours, following the system light/dark setting.
*
* DELIBERATELY NOT Material You dynamic colour, which this used until the operator
* saw the first build. Dynamic colour is the more Android-native choice and it
* DELIBERATELY NOT Material You dynamic colour. Dynamic colour is the more
* Android-native choice and it
* makes the app look like a different product on the phone than on the desktop and
* the web — on a stock device with no wallpaper it renders as undifferentiated
* grey. The three surfaces are peers held to one quality bar, so they share one
@@ -51,7 +51,7 @@ fun UpdateCard(
// Re-read on every return to the app, as ReminderNotice does: the grant is given
// in a system screen this app cannot observe. Read once, the warning would still
// be up after someone granted it and came back, and the offer below it would
// look blocked when it is not (family idea #5103, practice 9).
// look blocked when it is not.
var canInstall by remember { mutableStateOf(AppUpdate.canInstall(context)) }
ForegroundTransitions(onForeground = { canInstall = AppUpdate.canInstall(context) })
@@ -125,11 +125,8 @@ fun UpdateCard(
}
/**
* The nag: an update is waiting, said where someone will actually see it.
*
* Until this existed the only way to learn about a new build was to open the sync
* screen and press Check — so the updates that got installed were the ones somebody
* went looking for, and the rest were simply never found.
* The nag: an update is waiting, said where someone will actually see it rather than
* only on the sync screen.
*
* Only ever shown once the build is DOWNLOADED, so the offer is a single tap rather
* than the start of a wait — and so nothing is said at all until the app has been on
@@ -175,8 +172,7 @@ private val BANNER_SPINNER = 18.dp
fun UnlinkedUpdateNote() {
Hint(
text = stringResource(R.string.update_needs_server),
// Carries the bottom breathing room the connect button used to provide,
// now that it is the last thing on the unlinked screen.
// The bottom breathing room: this is the last thing on the unlinked screen.
modifier = Modifier.padding(top = 8.dp, bottom = 24.dp),
)
}
@@ -58,7 +58,7 @@ data class UpdateState(
* **Linked-only, and said out loud.** The app is local-first and completely usable
* having never touched a server, so an unlinked install has no update path at all.
* The card says that rather than offering a Check button that silently finds
* nothing — the same lesson as the desktop's unlink copy (issue 2110).
* nothing.
*
* The core does the network work, not this class: the device token lives in the
* Rust store and pulling it into Kotlin to make an HTTP call would spread the one
@@ -89,9 +89,8 @@ class UpdateViewModel(
/**
* The automatic path: look, fetch, then nag.
*
* Called when the app comes forward. Until this existed an update was only ever
* found by someone opening the sync screen and pressing a button — so the ones
* that mattered were the ones nobody went looking for.
* Called when the app comes forward, so an update is found without anyone opening
* the sync screen to look.
*
* Skipped when a check is already in flight, when a build is already waiting, and
* when one ran recently: flicking between two apps is not a request to re-check.
@@ -106,8 +105,7 @@ class UpdateViewModel(
// Already fetched and waved away — say so again. "Later" is for that
// sitting, not forever, and without this branch a single dismissal would
// silence the update permanently. Which is precisely the "lost" this whole
// path exists to prevent.
// silence the update permanently.
state.ready -> if (state.nagDismissed) state = state.copy(nagDismissed = false)
// Found one and never fetched it — almost always because the last look
+5 -5
View File
@@ -82,7 +82,7 @@
<string name="cancel">Cancel</string>
<string name="editor_share">Share…</string>
<!-- Sharing (#5175). Shares live on the server, so the sheet says so when this
<!-- Sharing. Shares live on the server, so the sheet says so when this
phone isn't linked to one. -->
<string name="share_title">Share</string>
<string name="share_loading">Loading…</string>
@@ -134,7 +134,7 @@
who it happens in. -->
<string name="capture_process_text">New note</string>
<!-- Tag management. The whole vocabulary is "tag" (see Scribe #2966); the
<!-- Tag management. The whole vocabulary is "tag"; the
schema still says Label, and no string here needs to know that. -->
<string name="tags_manage">Manage tags</string>
<string name="tags_show_all">All tags (%1$d)</string>
@@ -157,9 +157,9 @@
<string name="tags_rename">Rename</string>
<string name="tags_rename_title">Rename %1$s</string>
<string name="tags_rename_confirm">Rename</string>
<!-- Renaming onto an existing tag merges the two, older survives (Scribe
#3324). A merge cannot be undone by repeating it and is reachable here by
a typo, so it says so before it happens — same reasoning as #2116. -->
<!-- Renaming onto an existing tag merges the two, older survives. A merge
cannot be undone by repeating it and is reachable here by a typo, so it
says so before it happens. -->
<string name="tags_rename_merges_title">Merge with %1$s?</string>
<string name="tags_rename_merges_body">A tag called %1$s already exists. Renaming will merge these two into one, carrying every note from both. The notes are kept; one of the two tags stops existing, and that cannot be undone.</string>
<string name="tags_rename_merges_confirm">Merge</string>
@@ -12,11 +12,8 @@ import org.junit.Test
* writes them out by hand, so changing the hash or the key order means changing the
* fixture and this file together, or one of the two suites goes red.
*
* SMALLER SINCE M315. Half of what this file used to pin — the generated card fill, and
* the resolution order that chose between a picked colour, a tag's and a generated one
* — went with the code it guarded when the card became one neutral. The four UUID
* hashes stay because they are what the hash ITSELF is pinned by; nothing derives a
* colour from an id any more, only from a tag's name.
* The four UUID hashes pin the hash ITSELF; colours are derived only from a tag's
* name.
*/
class DerivedTintTest {
@Test
+6 -10
View File
@@ -7,16 +7,12 @@ edition = "2021"
# A crate whose ONLY dependency is uniffi itself.
#
# This started life as a `[[bin]]` inside inkwell-ffi, which failed: building
# it compiled that crate and therefore the core, reqwest, native-tls and
# openssl-sys — for the HOST. The vendored-OpenSSL block in core/Cargo.toml is
# scoped to `cfg(target_os = "android")`, so a host build looks for a system
# OpenSSL that ci-rust-android has no reason to carry, and the generator died
# with "failed to run custom build command for openssl-sys".
#
# Adding libssl-dev to the image would have worked and been wrong: a code
# generator should not link the app's TLS stack to emit Kotlin. Splitting it out
# means the generator compiles ~15 small crates and nothing else.
# Not a `[[bin]]` inside inkwell-ffi: building that compiles the core, reqwest,
# native-tls and openssl-sys for the HOST, and the vendored-OpenSSL block in
# core/Cargo.toml is scoped to `cfg(target_os = "android")`, so a host build
# looks for a system OpenSSL that ci-rust-android doesn't carry. A code
# generator should not link the app's TLS stack to emit Kotlin; on its own it
# compiles ~15 small crates.
#
# Still a WORKSPACE MEMBER, deliberately. That is what keeps `uniffi` here and
# `uniffi` linked into the .so on one version from one lockfile — they are two
+2 -2
View File
@@ -1,8 +1,8 @@
plugins {
alias(libs.plugins.android.application) apply false
// kotlin-android is NOT registered: AGP 9 enables built-in Kotlin, and the
// older plugin can't cast AGP 9's ApplicationExtension to the removed
// BaseExtension. Same conclusion Minstrel reached on this toolchain pair.
// older plugin can't cast AGP 9's ApplicationExtension to the BaseExtension
// AGP 9 no longer has.
alias(libs.plugins.compose.compiler) apply false
// ktlint/detekt are run from the CI image's pinned CLIs, not as Gradle
// plugins — see the note in gradle/libs.versions.toml.
+3 -3
View File
@@ -1,8 +1,8 @@
# Per-rule overrides layered on top of detekt's defaults
# (`--build-upon-default-config` on the CLI invocation in the Android lane).
#
# The pre-2.0 `build:` top-level was removed; failure is controlled by the CLI's
# exit code instead.
# Failure is controlled by the CLI's exit code; detekt 2 has no `build:`
# top-level.
naming:
# Composables conventionally use PascalCase function names. Matches every
@@ -49,7 +49,7 @@ complexity:
exceptions:
TooGenericExceptionCaught:
# Catching broadly is DELIBERATE in these two places, and each site says so.
# Catching broadly is DELIBERATE in these places, and each site says so.
#
# * the ViewModel — a note that fails to save must become a visible error
# banner, never a crash. Narrowing this would mean an unanticipated
+16 -21
View File
@@ -3,7 +3,7 @@
//! This crate is to Android what `desktop/src-tauri/src/commands/` is to the desktop
//! — a thin shim over the shared core, holding no logic of its own. If something here
//! starts making decisions about notes or sync, it belongs in the core where the
//! desktop gets it too (Scribe note 2730).
//! desktop gets it too.
//!
//! ## Shape
//!
@@ -30,9 +30,7 @@
//! The Android build overrides it back to `unwind` (`CargoNdkBuild` in
//! `app/build.gradle.kts`), so uniffi catches a panic crossing the FFI boundary and
//! raises it in Kotlin as an exception, which the board shows as an error. Under
//! `abort` the same panic would end the app. Every phone build had unwind before
//! the release profile was used (debug unwinds), so keeping it changes nothing a
//! phone does. The cost is library size (Scribe #2810).
//! `abort` the same panic would end the app. The cost is library size.
pub mod models;
@@ -105,7 +103,7 @@ impl CoreError {
}
/// Seals the device token before it is stored, and opens it when it is read: Kotlin's
/// half of [`state::TokenSeal`] (family idea #5105, practice 12).
/// half of [`state::TokenSeal`].
///
/// Implemented with a key in the Android Keystore. The key never leaves the phone,
/// so a copy of the app's files carries a token nothing else can open. That includes
@@ -255,7 +253,7 @@ impl Inkwell {
///
/// Returns nothing, as the board reloads its list afterwards anyway. The core marks
/// each note dirty so the new positions sync, and a note shared with this account
/// moves on this board only (#5176).
/// moves on this board only.
pub fn reorder_notes(&self, ordered_ids: Vec<String>) -> Result<(), CoreError> {
let conn = self.db.conn().map_err(CoreError::store)?;
local::store::reorder(&conn, &ordered_ids).map_err(CoreError::store)
@@ -600,11 +598,11 @@ impl Inkwell {
// ────────────────────────────── sharing ──────────────────────────────
//
// The Share dialog asks the server directly (#5175). Unlinked, each answers
// The Share dialog asks the server directly. Unlinked, each answers
// `NotLinked`, which the dialog turns into "sharing needs a server".
//
// Each passes the token `credentials()` OPENED. The stored one is sealed, and the
// server refuses `sealed:…` (#5381).
// server refuses `sealed:…`.
/// Everyone on the instance a note can be shared with, and every group.
pub async fn share_directory(&self) -> Result<Directory, CoreError> {
@@ -682,10 +680,8 @@ pub fn checklist_render(text: String, checked: bool) -> String {
/// Tell the core which app it is running inside, and which build of it.
///
/// Android has to say so because the core cannot: the same crate is compiled into
/// the desktop app, and it used to announce every phone in the field as
/// `inkwell-desktop` carrying the CORE crate's version — a number no build
/// stamps and nobody has seen. The honest value is the installed package's own
/// `versionName`, which is what Kotlin passes here.
/// the desktop app. The honest value is the installed package's own `versionName`,
/// which is what Kotlin passes here.
///
/// Called once from `InkwellApplication.onCreate`, before anything can sync.
#[uniffi::export]
@@ -820,7 +816,7 @@ mod tests {
.get_note(created.id.clone())
.expect("get should succeed");
assert_eq!(fetched.id, created.id);
// The NAME is the first line — there is no title field to have set (M13 step 3).
// The NAME is the first line — there is no title field to have set.
assert_eq!(fetched.display_title, "Groceries");
std::fs::remove_dir_all(&dir).ok();
@@ -1041,9 +1037,8 @@ mod tests {
/// The path the notification's Done button takes.
///
/// Completing a RECURRING reminder must move it, not end it — this is the
/// behaviour the web has had all along and the clients did not, which made
/// "Done" on a daily reminder quietly the last time it ever fired.
/// Completing a RECURRING reminder must move it, not end it, as on the web;
/// otherwise "Done" on a daily reminder would be the last time it ever fired.
#[test]
fn completing_a_recurring_reminder_moves_it_rather_than_ending_it() {
let dir = scratch_dir();
@@ -1103,9 +1098,9 @@ mod tests {
/// Renaming a tag onto a name another tag already holds MERGES the two, and the
/// OLDER row is the survivor.
///
/// Before this, the bare UPDATE met `idx_labels_name` — unique on `lower(name)` —
/// and the user got a raw "UNIQUE constraint failed" from SQLite. Merging is what
/// a person means by typing an existing tag's name onto this one.
/// A bare UPDATE would meet `idx_labels_name` (unique on `lower(name)`) and hand
/// the user a raw "UNIQUE constraint failed"; merging is what a person means by
/// typing an existing tag's name onto this one.
#[test]
fn renaming_onto_an_existing_tag_merges_into_the_older_one() {
let dir = scratch_dir();
@@ -1215,8 +1210,8 @@ mod tests {
(url, handle)
}
/// #5381: the token is stored sealed, so a share call has to send the OPENED one.
/// It used to read the store itself and send `sealed:…`, which the server refused.
/// The token is stored sealed, so a share call has to send the OPENED one; the
/// server refuses `sealed:…`.
#[test]
fn sharing_sends_the_opened_token_not_the_stored_one() {
let (url, server) = one_reply(r#"{"members":[],"groups":[]}"#);
+5 -5
View File
@@ -46,7 +46,7 @@ pub struct Note {
pub previews: Vec<LinkPreview>,
pub created_at: Option<String>,
pub updated_at: Option<String>,
/// How this account holds the note (#5175): "owner", or "edit"/"view" for one
/// How this account holds the note: "owner", or "edit"/"view" for one
/// someone shared with it.
pub permission: String,
/// Whether the owner has shared it with anyone.
@@ -335,7 +335,7 @@ pub struct Label {
/// matching the REST single-label responses.
pub count: Option<i64>,
/// The newest edit among the untrashed notes carrying it, as the store writes
/// timestamps; None when nothing does. Ranks the drawer's recent tags (#5931).
/// timestamps; None when nothing does. Ranks the drawer's recent tags.
pub last_used: Option<String>,
}
@@ -424,7 +424,7 @@ impl From<NoteFacets> for core_models::Facets {
#[derive(Debug, Clone, uniffi::Record)]
pub struct NoteDraft {
pub body: String,
/// Checklist lines. A note can carry both a body and items (M13 step 2), so this
/// Checklist lines. A note can carry both a body and items, so this
/// is not an alternative to `body` — it is an addition to it.
pub items: Option<Vec<String>>,
}
@@ -616,7 +616,7 @@ impl From<core_client::Identity> for Identity {
}
}
/// Someone on the instance a note can be shared with (#5175).
/// Someone on the instance a note can be shared with.
#[derive(Debug, Clone, uniffi::Record)]
pub struct Member {
pub id: String,
@@ -639,7 +639,7 @@ impl From<core_client::Member> for Member {
}
}
/// A group the admin made, which a note can be shared with (#5177).
/// A group the admin made, which a note can be shared with.
#[derive(Debug, Clone, uniffi::Record)]
pub struct ShareGroup {
pub id: String,
+5 -5
View File
@@ -1,7 +1,7 @@
# --enable-native-access=ALL-UNNAMED silences the JDK 22+ "restricted method in
# java.lang.System has been called" warning that Gradle 9.1's bundled
# native-platform jar trips via System.load(). Same opt-in Minstrel needs on the
# same Gradle/JDK pair; future JDKs promote the warning to an error.
# native-platform jar trips via System.load(); future JDKs promote the warning
# to an error.
org.gradle.jvmargs=-Xmx4g -Dfile.encoding=UTF-8 --enable-native-access=ALL-UNNAMED
org.gradle.parallel=true
org.gradle.caching=true
@@ -9,8 +9,8 @@ org.gradle.configuration-cache=true
android.useAndroidX=true
android.nonTransitiveRClass=true
# Matches Minstrel: detekt 2.0-alpha and the ktlint Gradle plugin still have
# intermittent configuration-cache holes. Warn rather than fail so the CC speedup
# applies where it can.
# detekt 2.0-alpha and the ktlint Gradle plugin have intermittent
# configuration-cache holes. Warn rather than fail so the CC speedup applies
# where it can.
org.gradle.configuration-cache.problems=warn
kotlin.code.style=official
+7 -10
View File
@@ -1,5 +1,5 @@
[versions]
# Pinned as a MATRIX, matching Minstrel's proven combination on the same JDK:
# Pinned as a MATRIX:
# - Gradle 9.1.0 supports JDK 25 (see gradle-wrapper.properties)
# - AGP 9.0.1 requires Gradle 9.1.0+
# - Kotlin 2.3.x is AGP 9's built-in Kotlin path
@@ -14,16 +14,13 @@ coroutines = "1.9.0"
# WorkManager runs the background sync. `work-runtime-ktx` is NOT used: as of
# 2.11 it is a 6 KB stub and every Kotlin extension (`PeriodicWorkRequestBuilder`,
# `CoroutineWorker`) has moved into `work-runtime` itself. Verified by unpacking
# both artifacts, not from memory.
# `CoroutineWorker`) has moved into `work-runtime` itself.
work = "2.11.2"
# ktlint and detekt are NOT Gradle plugins here. ci-rust-android already ships
# both as pinned CLIs (M12 step 3), and the CI lane invokes those directly. Adding
# the Gradle plugins would mean a SECOND pinned version of each tool, resolved at
# build time, that has to be kept in lockstep with the image's by hand — and the
# first attempt at it failed outright, because the detekt version Minstrel pins
# (2.0.0-alpha.3) is not published to Maven Central or the plugin portal at all.
# ktlint and detekt are NOT Gradle plugins here. ci-rust-android ships both as
# pinned CLIs, and the CI lane invokes those directly. The Gradle plugins would be
# a SECOND pinned version of each tool, resolved at build time, that has to be
# kept in lockstep with the image's by hand.
# JNA is not optional: uniffi's Kotlin bindings call into the .so through it.
# The @aar classifier matters — the plain jar has no Android native payload and
@@ -32,7 +29,7 @@ jna = "5.14.0"
junit = "4.13.2"
# Coil draws a link preview's image (Scribe #3307), the first image this client
# Coil draws a link preview's image, the first image this client
# loads. Coil 3 ships its networking separately; the OkHttp fetcher is the one its
# docs name as the default, and it registers itself, so no loader is configured.
# Held at 3.5.0: 3.6.0 moved to compileSdk 37 and Compose 1.12, which need AGP 9.1.
+1 -2
View File
@@ -44,8 +44,7 @@ def supplied_args(rest):
"""Count top-level commas in an argument tail.
Kotlin permits a TRAILING comma before the closing paren, which is not an
argument — counting it inflated every multi-line call by one the first time
this was written, and made three correct call sites look broken. Braces count
argument; counting it would inflate every multi-line call by one. Braces count
toward depth as well as parens, or a comma inside a lambda would be read as
another argument.
"""
+5 -11
View File
@@ -1,11 +1,8 @@
#!/usr/bin/env python3
"""Flag capitalised identifiers that are neither imported nor declared locally.
This exists because ktlint and detekt are both structurally blind to it: they
parse Kotlin without resolving symbols, so a *missing import* is invisible to
them and both pass a file that cannot compile. The first sync-screen push failed
in CI on exactly that (`Unresolved reference 'Build'` — `android.os.Build` was
lost in a file split), after a clean local analyzer run.
ktlint and detekt parse Kotlin without resolving symbols, so a *missing import*
is invisible to them and both pass a file that cannot compile.
Not a type checker and not trying to be. `compileDebugKotlin` in CI is the real
one; this is a cheap pre-push filter for the single mistake that survives every
@@ -13,10 +10,8 @@ other local gate. It errs toward false positives — anything it cannot account
for is reported rather than assumed fine.
It also checks MEMBERS of this package's own `object` declarations — `Foo.bar()`
where `Foo` is an object declared here. That case was added after moving a
function between two objects and forgetting to paste it into the second: the
call site read `Other.thing()`, resolved fine as far as the leading token, and
failed in CI (785ebdb).
where `Foo` is an object declared here — so a function moved between two objects
and missed at a call site is caught.
Still NOT caught, so a clean run is not over-read: members of anything declared
outside this package, members reached through a variable rather than a type
@@ -106,8 +101,7 @@ def object_members(src: str) -> dict:
if depth == 0:
# Nested TYPES count as members too: `Foo.Bar` where Bar is a
# data class inside object Foo is an ordinary reference, and
# leaving them out made the checker report four false positives
# the first time an object held one.
# leaving them out would report each one as unknown.
decl = re.match(
r"\s*(?:@\w+\s+)*(?:public |private |internal |protected )?"
r"(?:const |lateinit |inline |suspend |data |sealed |enum |value |abstract |open )*"