From 23fa389c0aa62ab77b61de0189fa0ee23b223b21 Mon Sep 17 00:00:00 2001 From: Jason Ross Date: Thu, 2 Jul 2026 22:38:22 -0500 Subject: [PATCH 1/2] feat(compose): add font size control to the formatting toolbar Add a preset-size dropdown (10/12/14/18/24pt, plus Default to clear) to the compose FormattingToolbar via a new FontSizePicker composable, applying RichStyle.FontSize over the selection through the existing generalized applyStyle/clearStyle toggle path (no font-size-specific branching needed). The anchor button shows the selection's current size, or "Default" when unset/mixed. The rich-text foundation already provided RichStyle.FontSize, its pt/px-tolerant HTML round-trip, and pt->sp mapping for in-editor rendering; this ticket wires up the missing UI control. Closes #73 Co-Authored-By: Claude Opus 4.8 --- .../ui/compose/format/FontSizePickerTest.kt | 81 +++++++++++++++++ .../libremail/ui/compose/RichTextEditor.kt | 18 +++- .../ui/compose/format/FontSizePicker.kt | 91 +++++++++++++++++++ app/src/main/res/values/strings.xml | 3 + .../ui/compose/RichTextEditorTest.kt | 8 ++ 5 files changed, 199 insertions(+), 2 deletions(-) create mode 100644 app/src/androidTest/kotlin/org/libremail/ui/compose/format/FontSizePickerTest.kt create mode 100644 app/src/main/kotlin/org/libremail/ui/compose/format/FontSizePicker.kt diff --git a/app/src/androidTest/kotlin/org/libremail/ui/compose/format/FontSizePickerTest.kt b/app/src/androidTest/kotlin/org/libremail/ui/compose/format/FontSizePickerTest.kt new file mode 100644 index 0000000..cc6e281 --- /dev/null +++ b/app/src/androidTest/kotlin/org/libremail/ui/compose/format/FontSizePickerTest.kt @@ -0,0 +1,81 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.ui.compose.format + +import androidx.activity.ComponentActivity +import androidx.compose.ui.test.assertIsDisplayed +import androidx.compose.ui.test.junit4.createAndroidComposeRule +import androidx.compose.ui.test.onNodeWithText +import androidx.compose.ui.test.performClick +import androidx.test.ext.junit.runners.AndroidJUnit4 +import org.junit.Assert.assertEquals +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.libremail.R +import org.libremail.ui.theme.LibreMailTheme + +/** + * UI tests for the compose formatting toolbar's font-size dropdown (#73). [FontSizePicker] is + * presentational, so it is driven directly - independent of the surrounding + * [org.libremail.ui.compose.RichTextBodyField] editor - mirroring how `ContactAutocompleteRowTest` + * exercises its row composable in isolation. + */ +@RunWith(AndroidJUnit4::class) +class FontSizePickerTest { + + @get:Rule + val composeTestRule = createAndroidComposeRule() + + private fun string(resId: Int) = composeTestRule.activity.getString(resId) + + private fun string(resId: Int, vararg args: Any) = composeTestRule.activity.getString(resId, *args) + + private fun setContent(selectedPt: Int?, onSelect: (Int?) -> Unit = {}) { + composeTestRule.setContent { + LibreMailTheme(darkTheme = false, dynamicColor = false) { + FontSizePicker(selectedPt = selectedPt, onSelect = onSelect) + } + } + } + + @Test + fun noSizeSelected_buttonShowsDefaultLabel() { + setContent(selectedPt = null) + composeTestRule.onNodeWithText(string(R.string.format_size_default)).assertIsDisplayed() + } + + @Test + fun aSizeSelected_buttonShowsItsPointValue() { + setContent(selectedPt = 18) + composeTestRule.onNodeWithText(string(R.string.format_size_pt, 18)).assertIsDisplayed() + } + + @Test + fun tappingTheButton_opensAMenuListingDefaultAndEveryPreset() { + setContent(selectedPt = null) + // Before the menu opens, "Default" only labels the anchor button itself - a unique match. + composeTestRule.onNodeWithText(string(R.string.format_size_default)).performClick() + FONT_SIZE_PRESETS_PT.forEach { pt -> + composeTestRule.onNodeWithText(string(R.string.format_size_pt, pt)).assertIsDisplayed() + } + } + + @Test + fun pickingAPresetFromTheMenu_reportsItsPointSize() { + var picked: Int? = -1 + setContent(selectedPt = null) { picked = it } + composeTestRule.onNodeWithText(string(R.string.format_size_default)).performClick() + composeTestRule.onNodeWithText(string(R.string.format_size_pt, 14)).performClick() + assertEquals(14, picked) + } + + @Test + fun pickingDefaultFromTheMenu_clearsBySelectingNull() { + var picked: Int? = 12 + setContent(selectedPt = 12) { picked = it } + // The button reads "12 pt" here, so the menu's own "Default" entry is the only such match. + composeTestRule.onNodeWithText(string(R.string.format_size_pt, 12)).performClick() + composeTestRule.onNodeWithText(string(R.string.format_size_default)).performClick() + assertEquals(null, picked) + } +} diff --git a/app/src/main/kotlin/org/libremail/ui/compose/RichTextEditor.kt b/app/src/main/kotlin/org/libremail/ui/compose/RichTextEditor.kt index 0a3c6cd..71ad4a6 100644 --- a/app/src/main/kotlin/org/libremail/ui/compose/RichTextEditor.kt +++ b/app/src/main/kotlin/org/libremail/ui/compose/RichTextEditor.kt @@ -58,6 +58,7 @@ import org.libremail.richtext.RichTextEditing import org.libremail.richtext.RichTextHtml import org.libremail.ui.compose.format.ColorSwatch import org.libremail.ui.compose.format.ColorSwatchRow +import org.libremail.ui.compose.format.FontSizePicker /** String-annotation tag the editor uses to carry a span's link target inside the [AnnotatedString]. */ private const val URL_TAG = "libremail:url" @@ -74,7 +75,7 @@ internal const val IMAGE_TAG = "libremail:image" /** * A rich-text body editor: a formatting toolbar (bold / italic / underline / strikethrough, font - * color and highlight, bulleted + numbered lists, block quote, and link) above a rounded + * size, font color and highlight, bulleted + numbered lists, block quote, and link) above a rounded * [OutlinedTextField]. It converts its [AnnotatedString] to the app's [RichTextContent] model and * reports both the plaintext form and its HTML — or null HTML when nothing is formatted, so an * unformatted message stays plaintext-only and feels exactly like the old editor. @@ -83,7 +84,8 @@ internal const val IMAGE_TAG = "libremail:image" * work as usual; each toolbar button exposes its accessible action label via `onClickLabel` on its * [Modifier.clickable] (not a `contentDescription`), and still carries toggle state for accessibility. * The font-color and highlight buttons open a [ColorPickerDialog] built on the shared - * [ColorSwatchRow], whose individual swatches carry their own `contentDescription` instead. + * [ColorSwatchRow], whose individual swatches carry their own `contentDescription` instead; the font + * size button opens the self-contained [FontSizePicker] dropdown. * * [resolveFont] maps a CSS font-family stack to a Compose [FontFamily] for display; the default * resolves nothing, leaving the system font (the model still round-trips the CSS value untouched). @@ -134,6 +136,15 @@ fun RichTextBodyField( onLink = { showLinkDialog = true }, onFontColor = { showFontColorPicker = true }, onHighlight = { showHighlightPicker = true }, + onFontSize = { pt -> + emit( + if (pt != null) { + applyStyle(value, RichStyle.FontSize(pt), linkColor, resolveFont) + } else { + clearStyle(value, RichStyle.FontSize::class.java, linkColor, resolveFont) + }, + ) + }, ) OutlinedTextField( value = value, @@ -217,6 +228,7 @@ private fun FormattingToolbar( onLink: () -> Unit, onFontColor: () -> Unit, onHighlight: () -> Unit, + onFontSize: (Int?) -> Unit, ) { val content = value.annotatedString.toRichContent() val start = value.selection.min @@ -257,6 +269,8 @@ private fun FormattingToolbar( strikethrough = true, onClick = { onToggleStyle(RichStyle.Strikethrough) }, ) + val fontSizePt = RichTextEditing.styleAt(content, start, end, RichStyle.FontSize::class.java)?.pt + FontSizePicker(selectedPt = fontSizePt, onSelect = onFontSize) val fontColorArgb = RichTextEditing.styleAt(content, start, end, RichStyle.FontColor::class.java)?.argb FormatButton( label = "A", diff --git a/app/src/main/kotlin/org/libremail/ui/compose/format/FontSizePicker.kt b/app/src/main/kotlin/org/libremail/ui/compose/format/FontSizePicker.kt new file mode 100644 index 0000000..783643d --- /dev/null +++ b/app/src/main/kotlin/org/libremail/ui/compose/format/FontSizePicker.kt @@ -0,0 +1,91 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.ui.compose.format + +import androidx.compose.foundation.background +import androidx.compose.foundation.clickable +import androidx.compose.foundation.layout.Arrangement +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.Row +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.size +import androidx.compose.material.icons.Icons +import androidx.compose.material.icons.filled.ArrowDropDown +import androidx.compose.material3.DropdownMenu +import androidx.compose.material3.DropdownMenuItem +import androidx.compose.material3.Icon +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.Text +import androidx.compose.runtime.Composable +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember +import androidx.compose.runtime.setValue +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.draw.clip +import androidx.compose.ui.graphics.Color +import androidx.compose.ui.res.stringResource +import androidx.compose.ui.semantics.Role +import androidx.compose.ui.unit.dp +import org.libremail.R + +/** The fixed preset sizes [FontSizePicker] offers, in points. */ +internal val FONT_SIZE_PRESETS_PT = listOf(10, 12, 14, 18, 24) + +/** + * A toolbar dropdown for [org.libremail.richtext.RichStyle.FontSize]: the anchor button shows the + * selection's current size, or "Default" when [selectedPt] is null (no size applied, or a mixed + * selection), and opens a menu of [FONT_SIZE_PRESETS_PT] plus a leading "Default" entry that clears + * the style outright. Mirrors [ColorSwatchRow]'s "no color" convention: [onSelect] receives null for + * "Default" and a preset point size otherwise, so the caller routes the choice straight through the + * generalized `applyStyle`/`clearStyle` toggle path with no font-size-specific branching of its own. + */ +@Composable +fun FontSizePicker(selectedPt: Int?, onSelect: (Int?) -> Unit, modifier: Modifier = Modifier) { + var expanded by remember { mutableStateOf(false) } + val colors = MaterialTheme.colorScheme + val description = stringResource(R.string.format_size) + val label = if (selectedPt != null) { + stringResource(R.string.format_size_pt, selectedPt) + } else { + stringResource(R.string.format_size_default) + } + val contentColor = if (selectedPt != null) colors.onSecondaryContainer else colors.onSurfaceVariant + Box(modifier = modifier) { + Row( + modifier = Modifier + .clip(MaterialTheme.shapes.small) + .background(if (selectedPt != null) colors.secondaryContainer else Color.Transparent) + .clickable(onClick = { expanded = true }, role = Role.Button, onClickLabel = description) + .padding(horizontal = 12.dp, vertical = 8.dp), + horizontalArrangement = Arrangement.spacedBy(2.dp), + verticalAlignment = Alignment.CenterVertically, + ) { + Text(text = label, color = contentColor) + Icon( + Icons.Filled.ArrowDropDown, + contentDescription = null, + modifier = Modifier.size(18.dp), + tint = contentColor, + ) + } + DropdownMenu(expanded = expanded, onDismissRequest = { expanded = false }) { + DropdownMenuItem( + text = { Text(stringResource(R.string.format_size_default)) }, + onClick = { + expanded = false + onSelect(null) + }, + ) + FONT_SIZE_PRESETS_PT.forEach { pt -> + DropdownMenuItem( + text = { Text(stringResource(R.string.format_size_pt, pt)) }, + onClick = { + expanded = false + onSelect(pt) + }, + ) + } + } + } +} diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 8134527..8d49bb4 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -121,6 +121,9 @@ Green Cyan Pink + Font size + Default + %1$d pt Drafts diff --git a/app/src/test/kotlin/org/libremail/ui/compose/RichTextEditorTest.kt b/app/src/test/kotlin/org/libremail/ui/compose/RichTextEditorTest.kt index 5af9e47..9eccb38 100644 --- a/app/src/test/kotlin/org/libremail/ui/compose/RichTextEditorTest.kt +++ b/app/src/test/kotlin/org/libremail/ui/compose/RichTextEditorTest.kt @@ -207,6 +207,14 @@ class RichTextEditorTest { assertEquals(colored.annotatedString.toRichContent().spans, result.annotatedString.toRichContent().spans) } + @Test + fun `clearStyle removes a font size span regardless of its value`() { + val value = field("hello", TextRange(0, 5)) + val sized = applyStyle(value, RichStyle.FontSize(18), linkColor) + val cleared = clearStyle(sized, RichStyle.FontSize::class.java, linkColor) + assertTrue(cleared.annotatedString.toRichContent().spans.isEmpty()) + } + // --- applyBlock --- @Test From 9c6a969c17d135d18f1d661217f3c5d966c4892a Mon Sep 17 00:00:00 2001 From: Jason Ross Date: Thu, 2 Jul 2026 23:13:21 -0500 Subject: [PATCH 2/2] fix(compose): keep the bullet button tappable by appending the font-size control last MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The font-size dropdown was inserted before the block-marker buttons, and its wide "Default"/"N pt" anchor pushed the "•" bullet button past the right edge of the horizontally-scrolling toolbar on the Pixel 2 E2E device (411dp wide, minus the compose column's 16dp padding = 379dp usable). ComposeScreenTest's formattingToolbar_bulletButtonMarksTheLineAndSendsItAsHtml taps the bullet without scrolling first, so performClick targeted a center that was clipped off-screen and the tap silently missed — the line was never marked, failing all 8 instrumented legs deterministically (expected "• Buy milk", got "Buy milk"). The block-toggle logic was never touched; this was pure toolbar overflow. Move FontSizePicker to the end of the toolbar (after the link button) so every pre-existing glyph button keeps the exact position it has on main and the bullet stays within the initial viewport. Add a comment recording the ordering constraint for future toolbar tickets. Also add a JVM unit test (RichTextEditorTest) that drives the same bullet-tap flow through applyBlock + RichTextHtml.toHtml, pinning "• Buy milk" and
  • Buy milk
so a regression in that block/HTML path is caught by testDebugUnitTest without an emulator. Co-Authored-By: Claude Opus 4.8 --- .../org/libremail/ui/compose/RichTextEditor.kt | 10 ++++++++-- .../org/libremail/ui/compose/RichTextEditorTest.kt | 14 ++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/app/src/main/kotlin/org/libremail/ui/compose/RichTextEditor.kt b/app/src/main/kotlin/org/libremail/ui/compose/RichTextEditor.kt index 71ad4a6..166858a 100644 --- a/app/src/main/kotlin/org/libremail/ui/compose/RichTextEditor.kt +++ b/app/src/main/kotlin/org/libremail/ui/compose/RichTextEditor.kt @@ -269,8 +269,6 @@ private fun FormattingToolbar( strikethrough = true, onClick = { onToggleStyle(RichStyle.Strikethrough) }, ) - val fontSizePt = RichTextEditing.styleAt(content, start, end, RichStyle.FontSize::class.java)?.pt - FontSizePicker(selectedPt = fontSizePt, onSelect = onFontSize) val fontColorArgb = RichTextEditing.styleAt(content, start, end, RichStyle.FontColor::class.java)?.argb FormatButton( label = "A", @@ -311,6 +309,14 @@ private fun FormattingToolbar( active = false, onClick = onLink, ) + // The font-size dropdown trails every glyph button on purpose. The toolbar overflows the + // screen width and scrolls horizontally, and the compose E2E taps the "•" bullet button + // *without* scrolling first (see ComposeScreenTest.formattingToolbar_bulletButtonMarksTheLine...), + // so its click lands on the button's on-screen center. Any control inserted *before* the block + // buttons shifts them right and can push the bullet past the viewport, making that tap miss — + // so this wider control is appended last, leaving every pre-existing button in its tested spot. + val fontSizePt = RichTextEditing.styleAt(content, start, end, RichStyle.FontSize::class.java)?.pt + FontSizePicker(selectedPt = fontSizePt, onSelect = onFontSize) } } diff --git a/app/src/test/kotlin/org/libremail/ui/compose/RichTextEditorTest.kt b/app/src/test/kotlin/org/libremail/ui/compose/RichTextEditorTest.kt index 9eccb38..7b9b986 100644 --- a/app/src/test/kotlin/org/libremail/ui/compose/RichTextEditorTest.kt +++ b/app/src/test/kotlin/org/libremail/ui/compose/RichTextEditorTest.kt @@ -19,6 +19,7 @@ import org.libremail.richtext.RichSpan import org.libremail.richtext.RichStyle import org.libremail.richtext.RichTextContent import org.libremail.richtext.RichTextEditing +import org.libremail.richtext.RichTextHtml import org.libremail.richtext.imageToken import kotlin.test.assertEquals import kotlin.test.assertFalse @@ -240,6 +241,19 @@ class RichTextEditorTest { assertEquals("1. a\n2. b", result.annotatedString.text) } + @Test + fun `applyBlock bullet on an end-of-text caret marks the line and serializes to ul li html`() { + // The JVM-layer twin of ComposeScreenTest.formattingToolbar_bulletButtonMarksTheLineAndSendsItAsHtml: + // a bullet tap on the end-of-text caret that typing leaves must mark the whole line and serialize + // to a real list. Pinning it here catches a regression in the block-toggle/HTML flow without an + // emulator; the instrumented test additionally guards that the toolbar button stays tappable. + val value = field("Buy milk", TextRange(8)) + val bulleted = applyBlock(value, BlockMarker.BULLET, linkColor, noFont) + val content = bulleted.annotatedString.toRichContent() + assertEquals("• Buy milk", content.text) + assertEquals("
  • Buy milk
", RichTextHtml.toHtml(content)) + } + // --- applyLink --- @Test