From cfdda5b7b158f0556a3edb669903cc2a26f3bdfc Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Thu, 1 Oct 2026 13:26:38 +0200 Subject: [PATCH] Cap title and location at 255 chars (#331) --- .../calendula/ui/edit/EventEditScreen.kt | 72 ++++++++++++++----- .../calendula/ui/edit/EventEditViewModel.kt | 32 +++++++-- .../ui/edit/EventEditViewModelTest.kt | 67 +++++++++++++++++ 3 files changed, 148 insertions(+), 23 deletions(-) diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/edit/EventEditScreen.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/edit/EventEditScreen.kt index cc1a8bc..90dff7a 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/edit/EventEditScreen.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/edit/EventEditScreen.kt @@ -99,6 +99,7 @@ import androidx.compose.ui.res.stringResource import androidx.compose.ui.text.AnnotatedString import androidx.compose.ui.text.TextStyle import androidx.compose.ui.text.font.FontWeight +import androidx.compose.ui.text.style.TextAlign import androidx.compose.ui.text.style.TextOverflow import androidx.compose.ui.text.input.ImeAction import androidx.compose.ui.text.input.KeyboardCapitalization @@ -609,6 +610,11 @@ private fun EventEditContent( .padding(vertical = 4.dp) .focusRequester(titleFocusRequester), ) + FieldLengthCounter( + length = form.title.length, + max = MAX_SINGLE_LINE_FIELD, + modifier = Modifier.fillMaxWidth(), + ) Spacer(Modifier.height(10.dp)) Box( modifier = Modifier @@ -798,25 +804,31 @@ private fun EventEditContent( iconAtTop = true, ) { Row(verticalAlignment = Alignment.Top) { - InlineField( - value = form.location, - onValueChange = viewModel::setLocation, - placeholder = stringResource(R.string.event_detail_location), - // A location is as often a meeting URL as an address, - // and "Zoom.us/j/123" reads wrong (#146). - capitalization = KeyboardCapitalization.None, - // Multi-line so a long address or meeting URL wraps and - // the card grows instead of scrolling off one line - // (#273). The field holds no newlines of its own, so - // the IME's action key closes the keyboard rather than - // inserting a break setLocation would only fold away. - singleLine = false, - imeAction = ImeAction.Done, - modifier = Modifier - .weight(1f) - .animateContentSizeMotion() - .padding(vertical = 4.dp), - ) + Column(modifier = Modifier.weight(1f)) { + InlineField( + value = form.location, + onValueChange = viewModel::setLocation, + placeholder = stringResource(R.string.event_detail_location), + // A location is as often a meeting URL as an address, + // and "Zoom.us/j/123" reads wrong (#146). + capitalization = KeyboardCapitalization.None, + // Multi-line so a long address or meeting URL wraps and + // the card grows instead of scrolling off one line + // (#273). The field holds no newlines of its own, so + // the IME's action key closes the keyboard rather than + // inserting a break setLocation would only fold away. + singleLine = false, + imeAction = ImeAction.Done, + modifier = Modifier + .fillMaxWidth() + .animateContentSizeMotion() + .padding(vertical = 4.dp), + ) + FieldLengthCounter( + length = form.location.length, + max = MAX_SINGLE_LINE_FIELD, + ) + } // The box is exactly one text line tall, so the button // centres on the first line without making a single-line // card taller than its text; requiredSize keeps the @@ -2291,6 +2303,28 @@ private fun InlineField( ) } +/** + * A right-aligned "n/max" counter for a capped [InlineField] (#331), hidden + * until [length] gets close enough to [max] to matter — nobody needs to see + * "12/255" while typing a short title. + */ +@Composable +private fun FieldLengthCounter(length: Int, max: Int, modifier: Modifier = Modifier) { + AnimatedVisibility(visible = length >= max - 55, modifier = modifier) { + Text( + text = "$length/$max", + style = MaterialTheme.typography.bodySmall, + color = if (length >= max) { + MaterialTheme.colorScheme.error + } else { + MaterialTheme.colorScheme.onSurfaceVariant + }, + textAlign = TextAlign.End, + modifier = Modifier.fillMaxWidth(), + ) + } +} + /** * Height of one [InlineField] line: the resolved [TextStyle.lineHeight] plus the * field's 4dp vertical padding. In dp, so it tracks the user's font scale diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/edit/EventEditViewModel.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/edit/EventEditViewModel.kt index 017383d..99894e8 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/edit/EventEditViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/edit/EventEditViewModel.kt @@ -62,6 +62,26 @@ private const val TAG = "EventEdit" /** Any line break, with the whitespace around it, as one match. */ private val LINE_BREAK = Regex("""[ \t]*\R[ \t]*""") +/** + * Title and location are single-line provider columns that several CalDAV + * servers (Open-Xchange among them, #331) store as `VARCHAR(255)` and + * silently truncate past that on sync, with no error surfaced back to the + * app. 255 is the common floor, so it's what Calendula enforces up front. + */ +internal const val MAX_SINGLE_LINE_FIELD = 255 + +/** + * Caps [new] at [max] chars, but only while growing past it — an [old] value + * that already exceeds [max] (imported, or synced in from elsewhere) may + * still shrink freely, it just can't grow any further. Never splits a + * surrogate pair at the cut point. + */ +internal fun capLength(new: String, old: String, max: Int = MAX_SINGLE_LINE_FIELD): String = when { + new.length <= max || new.length <= old.length -> new + old.length >= max -> old + else -> new.take(max).let { if (it.isNotEmpty() && it.last().isHighSurrogate()) it.dropLast(1) else it } +} + /** * Where a prefilled [EventEditViewModel.openImported] form came from. The sources * want different reminder handling (#49), and differ in whether they own the @@ -533,13 +553,17 @@ class EventEditViewModel @Inject constructor( // The title field wraps (multi-line) so long titles stay visible (#33), but // a title is one logical line: drop any newline the IME's Enter key or a // paste would introduce, so it never reaches the provider's TITLE column. - fun setTitle(value: String) = - update { it.copy(title = value.replace("\n", "").replace("\r", "")) } + fun setTitle(value: String) = update { + val cleaned = value.replace("\n", "").replace("\r", "") + it.copy(title = capLength(cleaned, it.title)) + } // The location wraps too (#273) but is likewise one logical line. A pasted // multi-line address joins the way the contact picker's does (#146), so it // never runs together into "12 Main St10115 Berlin". - fun setLocation(value: String) = - update { it.copy(location = value.replace(LINE_BREAK, ", ")) } + fun setLocation(value: String) = update { + val cleaned = value.replace(LINE_BREAK, ", ") + it.copy(location = capLength(cleaned, it.location)) + } fun setDescription(value: String) = update { it.copy(description = value) } fun setAllDay(value: Boolean) { // Going all-day drops any pinned zone: the times become bare dates that diff --git a/app/src/test/java/de/jeanlucmakiola/calendula/ui/edit/EventEditViewModelTest.kt b/app/src/test/java/de/jeanlucmakiola/calendula/ui/edit/EventEditViewModelTest.kt index cb3b8de..8ee8926 100644 --- a/app/src/test/java/de/jeanlucmakiola/calendula/ui/edit/EventEditViewModelTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/calendula/ui/edit/EventEditViewModelTest.kt @@ -340,4 +340,71 @@ class EventEditViewModelTest { assertThat(fake.movedEvents).isEmpty() job.cancel() } + + @Test + fun `setTitle caps a long paste at 255 chars`( + @TempDir tempDir: Path, + ) = runTest(dispatcher) { + val fake = FakeCalendarDataSource().apply { calendarsResult = listOf(cal(1L)) } + val vm = viewModel(tempDir, fake) + val job = activate(vm) + + vm.openNew(LocalDate(2030, 1, 15)) + advanceUntilIdle() + vm.setTitle("a".repeat(300)) + + assertThat(vm.state.value?.form?.title).hasLength(MAX_SINGLE_LINE_FIELD) + job.cancel() + } + + @Test + fun `setTitle refuses to grow past 255 chars once capped`( + @TempDir tempDir: Path, + ) = runTest(dispatcher) { + val fake = FakeCalendarDataSource().apply { calendarsResult = listOf(cal(1L)) } + val vm = viewModel(tempDir, fake) + val job = activate(vm) + + vm.openNew(LocalDate(2030, 1, 15)) + advanceUntilIdle() + vm.setTitle("a".repeat(255)) + vm.setTitle("a".repeat(256)) + + assertThat(vm.state.value?.form?.title).hasLength(255) + job.cancel() + } + + @Test + fun `setLocation joins a multi-line address then caps it`( + @TempDir tempDir: Path, + ) = runTest(dispatcher) { + val fake = FakeCalendarDataSource().apply { calendarsResult = listOf(cal(1L)) } + val vm = viewModel(tempDir, fake) + val job = activate(vm) + + vm.openNew(LocalDate(2030, 1, 15)) + advanceUntilIdle() + vm.setLocation("${"a".repeat(200)}\n${"b".repeat(200)}") + + val location = vm.state.value?.form?.location + assertThat(location).hasLength(MAX_SINGLE_LINE_FIELD) + assertThat(location).contains(", ") + job.cancel() + } + + @Test + fun `capLength lets an already-over-long value shrink`() { + val overLong = "a".repeat(300) + val shortened = overLong.dropLast(1) + + assertThat(capLength(shortened, overLong)).hasLength(299) + } + + @Test + fun `capLength never splits a surrogate pair at the cut point`() { + // U+1F600, a surrogate pair, sits right across the 255 boundary. + val new = "a".repeat(254) + "😀" + + assertThat(capLength(new, "")).isEqualTo("a".repeat(254)) + } }