From a45a44ef1150e9c8b3b4c467860ce306ac6e360f Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Wed, 26 Aug 2026 07:13:27 -0400 Subject: [PATCH] android: split the block model from the block UI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit detekt's TooManyFunctions, at exactly the threshold. Worth taking as the signal it is rather than suppressing: the file held the block MODEL — split a body, join it back, add an item, drop one — and the COMPOSABLES that draw it, which are two jobs that happen to share a data class. EditorBlock.kt keeps the model and is pure: no Compose imports beyond the types it stores, and testable on its own if it ever earns tests. BlockBody.kt keeps the four composables. afterEnter, withoutIndex and nextId become internal, since the UI half calls them across the file boundary now. That is the one cost of the split and it is small — same module, same package, and each says why in its doc. --- .../fabledsword/thoughtsync/ui/BlockBody.kt | 220 +++++++++++++++++ .../fabledsword/thoughtsync/ui/EditorBlock.kt | 227 +----------------- 2 files changed, 227 insertions(+), 220 deletions(-) create mode 100644 android/app/src/main/java/com/fabledsword/thoughtsync/ui/BlockBody.kt diff --git a/android/app/src/main/java/com/fabledsword/thoughtsync/ui/BlockBody.kt b/android/app/src/main/java/com/fabledsword/thoughtsync/ui/BlockBody.kt new file mode 100644 index 0000000..4f3a530 --- /dev/null +++ b/android/app/src/main/java/com/fabledsword/thoughtsync/ui/BlockBody.kt @@ -0,0 +1,220 @@ +package com.fabledsword.thoughtsync.ui + +import androidx.annotation.StringRes +import androidx.compose.foundation.layout.Arrangement +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.Row +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.text.BasicTextField +import androidx.compose.foundation.text.KeyboardActions +import androidx.compose.foundation.text.KeyboardOptions +import androidx.compose.material.icons.Icons +import androidx.compose.material.icons.filled.Close +import androidx.compose.material3.Checkbox +import androidx.compose.material3.Icon +import androidx.compose.material3.IconButton +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.Text +import androidx.compose.runtime.Composable +import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.remember +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.focus.FocusRequester +import androidx.compose.ui.focus.focusRequester +import androidx.compose.ui.graphics.SolidColor +import androidx.compose.ui.res.stringResource +import androidx.compose.ui.text.TextStyle +import androidx.compose.ui.text.input.ImeAction +import androidx.compose.ui.text.input.TextFieldValue +import androidx.compose.ui.text.style.TextDecoration +import androidx.compose.ui.unit.dp +import com.fabledsword.thoughtsync.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. + */ +@Composable +fun BlockBody( + blocks: List, + readOnly: Boolean, + focus: Long?, + onChange: (List) -> Unit, + onFocus: (Long?) -> Unit, + modifier: Modifier = Modifier, +) { + // Focus is addressed by block ID, never by position — the id is the only thing + // about a block that survives one being inserted above it. Hoisted to the caller + // rather than kept here, because the TOOLBAR also asks for a focus when its button + // appends an item, and two owners of one cursor is one too many. + val requesters = remember { mutableMapOf() } + + LaunchedEffect(focus) { + val id = focus ?: return@LaunchedEffect + // Honoured after the composition that created the field: a FocusRequester not + // yet attached to anything throws when asked. + requesters[id]?.requestFocus() + onFocus(null) + } + + fun replace( + index: Int, + block: EditorBlock, + ) = onChange(blocks.toMutableList().also { it[index] = block }) + + Column(modifier = modifier, verticalArrangement = Arrangement.spacedBy(2.dp)) { + blocks.forEachIndexed { index, block -> + val requester = requesters.getOrPut(block.id) { FocusRequester() } + if (block.isTask) { + TaskBlock( + block = block, + readOnly = readOnly, + requester = requester, + onChange = { replace(index, it) }, + onEnter = { + val next = blocks.nextId() + onChange(afterEnter(blocks, index, next)) + // The new item if there was one; otherwise the block that just + // became prose, which keeps the caret where the person left it. + onFocus(if (blocks[index].value.text.isBlank()) block.id else next) + }, + onDelete = { + onChange(blocks.withoutIndex(index)) + onFocus(blocks.getOrNull(index - 1)?.id) + }, + ) + } else { + ProseBlock( + block = block, + readOnly = readOnly, + requester = requester, + onChange = { replace(index, it) }, + ) + } + } + } +} + +/** A run of prose: one ordinary multi-line field, exactly as the editor always had. */ +@Composable +private fun ProseBlock( + block: EditorBlock, + readOnly: Boolean, + requester: FocusRequester, + onChange: (EditorBlock) -> Unit, +) { + BlockField( + value = block.value, + onValueChange = { onChange(block.copy(value = it)) }, + modifier = Modifier.focusRequester(requester), + enabled = !readOnly, + hint = R.string.editor_body_hint, + ) +} + +/** + * One checklist item: a real box, and the item's text beside it. + * + * Single-line with [ImeAction.Next], which is what turns the keyboard's return key + * into "next item" — the reason a list can be typed straight through rather than a + * marker at a time. + */ +@Composable +private fun TaskBlock( + block: EditorBlock, + readOnly: Boolean, + requester: FocusRequester, + onChange: (EditorBlock) -> Unit, + onEnter: () -> Unit, + onDelete: () -> Unit, +) { + Row(verticalAlignment = Alignment.CenterVertically) { + Checkbox( + checked = block.checked == true, + onCheckedChange = { onChange(block.copy(checked = it)) }, + enabled = !readOnly, + ) + BlockField( + value = block.value, + onValueChange = { onChange(block.copy(value = it)) }, + modifier = Modifier.weight(1f).focusRequester(requester), + enabled = !readOnly, + singleLine = true, + textStyle = + MaterialTheme.typography.bodyLarge.copy( + // Struck through when done, matching the card and the web. + textDecoration = + if (block.checked == true) TextDecoration.LineThrough else null, + ), + keyboardOptions = KeyboardOptions(imeAction = ImeAction.Next), + keyboardActions = KeyboardActions(onNext = { onEnter() }), + ) + if (!readOnly) { + IconButton(onClick = onDelete) { + Icon( + Icons.Filled.Close, + contentDescription = stringResource(R.string.editor_remove_item), + ) + } + } + } +} + +/** + * 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. + * + * 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: + * + * - 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 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. + * + * `enabled = false` deliberately does not grey the text out: a trashed note renders + * read-only through this and its words are meant to be READ. + */ +@Composable +private fun BlockField( + value: TextFieldValue, + onValueChange: (TextFieldValue) -> Unit, + modifier: Modifier = Modifier, + enabled: Boolean = true, + singleLine: Boolean = false, + @StringRes hint: Int? = null, + textStyle: TextStyle = MaterialTheme.typography.bodyLarge, + keyboardOptions: KeyboardOptions = KeyboardOptions.Default, + keyboardActions: KeyboardActions = KeyboardActions.Default, +) { + val style = textStyle.copy(color = MaterialTheme.colorScheme.onSurface) + Box(modifier = modifier) { + if (hint != null && value.text.isEmpty()) { + Text( + text = stringResource(hint), + style = style, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + } + BasicTextField( + value = value, + onValueChange = onValueChange, + modifier = Modifier.fillMaxWidth(), + enabled = enabled, + singleLine = singleLine, + textStyle = style, + keyboardOptions = keyboardOptions, + keyboardActions = keyboardActions, + cursorBrush = SolidColor(MaterialTheme.colorScheme.primary), + ) + } +} diff --git a/android/app/src/main/java/com/fabledsword/thoughtsync/ui/EditorBlock.kt b/android/app/src/main/java/com/fabledsword/thoughtsync/ui/EditorBlock.kt index d48dc02..68207a1 100644 --- a/android/app/src/main/java/com/fabledsword/thoughtsync/ui/EditorBlock.kt +++ b/android/app/src/main/java/com/fabledsword/thoughtsync/ui/EditorBlock.kt @@ -1,38 +1,8 @@ package com.fabledsword.thoughtsync.ui -import androidx.annotation.StringRes -import androidx.compose.foundation.layout.Arrangement -import androidx.compose.foundation.layout.Box -import androidx.compose.foundation.layout.Column -import androidx.compose.foundation.layout.Row -import androidx.compose.foundation.layout.fillMaxWidth -import androidx.compose.foundation.text.BasicTextField -import androidx.compose.foundation.text.KeyboardActions -import androidx.compose.foundation.text.KeyboardOptions -import androidx.compose.material.icons.Icons -import androidx.compose.material.icons.filled.Close -import androidx.compose.material3.Checkbox -import androidx.compose.material3.Icon -import androidx.compose.material3.IconButton -import androidx.compose.material3.MaterialTheme -import androidx.compose.material3.Text -import androidx.compose.runtime.Composable -import androidx.compose.runtime.LaunchedEffect -import androidx.compose.runtime.remember import androidx.compose.runtime.saveable.Saver -import androidx.compose.ui.Alignment -import androidx.compose.ui.Modifier -import androidx.compose.ui.focus.FocusRequester -import androidx.compose.ui.focus.focusRequester -import androidx.compose.ui.graphics.SolidColor -import androidx.compose.ui.res.stringResource import androidx.compose.ui.text.TextRange -import androidx.compose.ui.text.TextStyle -import androidx.compose.ui.text.input.ImeAction import androidx.compose.ui.text.input.TextFieldValue -import androidx.compose.ui.text.style.TextDecoration -import androidx.compose.ui.unit.dp -import com.fabledsword.thoughtsync.R import com.fabledsword.thoughtsync.core.checklistItems import com.fabledsword.thoughtsync.core.checklistRender @@ -122,196 +92,13 @@ fun joinBlocks(blocks: List): String = val blocksSaver: Saver, String> = Saver(save = { joinBlocks(it) }, restore = { splitBlocks(it) }) -/** - * 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. - */ -@Composable -fun BlockBody( - blocks: List, - readOnly: Boolean, - focus: Long?, - onChange: (List) -> Unit, - onFocus: (Long?) -> Unit, - modifier: Modifier = Modifier, -) { - // Focus is addressed by block ID, never by position — the id is the only thing - // about a block that survives one being inserted above it. Hoisted to the caller - // rather than kept here, because the TOOLBAR also asks for a focus when its button - // appends an item, and two owners of one cursor is one too many. - val requesters = remember { mutableMapOf() } - - LaunchedEffect(focus) { - val id = focus ?: return@LaunchedEffect - // Honoured after the composition that created the field: a FocusRequester not - // yet attached to anything throws when asked. - requesters[id]?.requestFocus() - onFocus(null) - } - - fun replace( - index: Int, - block: EditorBlock, - ) = onChange(blocks.toMutableList().also { it[index] = block }) - - Column(modifier = modifier, verticalArrangement = Arrangement.spacedBy(2.dp)) { - blocks.forEachIndexed { index, block -> - val requester = requesters.getOrPut(block.id) { FocusRequester() } - if (block.isTask) { - TaskBlock( - block = block, - readOnly = readOnly, - requester = requester, - onChange = { replace(index, it) }, - onEnter = { - val next = blocks.nextId() - onChange(afterEnter(blocks, index, next)) - // The new item if there was one; otherwise the block that just - // became prose, which keeps the caret where the person left it. - onFocus(if (blocks[index].value.text.isBlank()) block.id else next) - }, - onDelete = { - onChange(blocks.withoutIndex(index)) - onFocus(blocks.getOrNull(index - 1)?.id) - }, - ) - } else { - ProseBlock( - block = block, - readOnly = readOnly, - requester = requester, - onChange = { replace(index, it) }, - ) - } - } - } -} - -/** A run of prose: one ordinary multi-line field, exactly as the editor always had. */ -@Composable -private fun ProseBlock( - block: EditorBlock, - readOnly: Boolean, - requester: FocusRequester, - onChange: (EditorBlock) -> Unit, -) { - BlockField( - value = block.value, - onValueChange = { onChange(block.copy(value = it)) }, - modifier = Modifier.focusRequester(requester), - enabled = !readOnly, - hint = R.string.editor_body_hint, - ) -} - -/** - * One checklist item: a real box, and the item's text beside it. - * - * Single-line with [ImeAction.Next], which is what turns the keyboard's return key - * into "next item" — the reason a list can be typed straight through rather than a - * marker at a time. - */ -@Composable -private fun TaskBlock( - block: EditorBlock, - readOnly: Boolean, - requester: FocusRequester, - onChange: (EditorBlock) -> Unit, - onEnter: () -> Unit, - onDelete: () -> Unit, -) { - Row(verticalAlignment = Alignment.CenterVertically) { - Checkbox( - checked = block.checked == true, - onCheckedChange = { onChange(block.copy(checked = it)) }, - enabled = !readOnly, - ) - BlockField( - value = block.value, - onValueChange = { onChange(block.copy(value = it)) }, - modifier = Modifier.weight(1f).focusRequester(requester), - enabled = !readOnly, - singleLine = true, - textStyle = - MaterialTheme.typography.bodyLarge.copy( - // Struck through when done, matching the card and the web. - textDecoration = - if (block.checked == true) TextDecoration.LineThrough else null, - ), - keyboardOptions = KeyboardOptions(imeAction = ImeAction.Next), - keyboardActions = KeyboardActions(onNext = { onEnter() }), - ) - if (!readOnly) { - IconButton(onClick = onDelete) { - Icon( - Icons.Filled.Close, - contentDescription = stringResource(R.string.editor_remove_item), - ) - } - } - } -} - -/** - * 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. - * - * 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: - * - * - 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 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. - * - * `enabled = false` deliberately does not grey the text out: a trashed note renders - * read-only through this and its words are meant to be READ. - */ -@Composable -private fun BlockField( - value: TextFieldValue, - onValueChange: (TextFieldValue) -> Unit, - modifier: Modifier = Modifier, - enabled: Boolean = true, - singleLine: Boolean = false, - @StringRes hint: Int? = null, - textStyle: TextStyle = MaterialTheme.typography.bodyLarge, - keyboardOptions: KeyboardOptions = KeyboardOptions.Default, - keyboardActions: KeyboardActions = KeyboardActions.Default, -) { - val style = textStyle.copy(color = MaterialTheme.colorScheme.onSurface) - Box(modifier = modifier) { - if (hint != null && value.text.isEmpty()) { - Text( - text = stringResource(hint), - style = style, - color = MaterialTheme.colorScheme.onSurfaceVariant, - ) - } - BasicTextField( - value = value, - onValueChange = onValueChange, - modifier = Modifier.fillMaxWidth(), - enabled = enabled, - singleLine = singleLine, - textStyle = style, - keyboardOptions = keyboardOptions, - keyboardActions = keyboardActions, - cursorBrush = SolidColor(MaterialTheme.colorScheme.primary), - ) - } -} - /** * 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. + * * 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. @@ -319,7 +106,7 @@ private fun BlockField( * 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. */ -private fun afterEnter( +internal fun afterEnter( blocks: List, index: Int, newId: Long, @@ -335,13 +122,13 @@ private fun afterEnter( } /** Drop a block, leaving at least one field to type into. */ -private fun List.withoutIndex(index: Int): List { +internal fun List.withoutIndex(index: Int): List { val out = toMutableList().also { it.removeAt(index) } return out.ifEmpty { listOf(EditorBlock(nextId(), TextFieldValue(""), null)) } } /** An id nothing else is using. Monotonic within a session, which is all it has to be. */ -private fun List.nextId(): Long = (maxOfOrNull { it.id } ?: -1L) + 1L +internal fun List.nextId(): Long = (maxOfOrNull { it.id } ?: -1L) + 1L /** * One more empty checklist item at the end, and the id to put the caret in.