From 80bf96b7dc4e90818d28b7084c78d822fae3e9b8 Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Mon, 7 Sep 2026 16:47:40 +0200 Subject: [PATCH] Wrap the dragged copy's time as the block does (#267) (#269) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The floating drag copy hardcoded `maxLines = 1` on both its time label and its title, so picking up a block whose text wrapped clipped it to one line for as long as it was held, and it snapped back on drop. - `DragCopy` now measures the range at its own width and height, exactly as the block does — same `MAX_TIME_LINES` cap, same spare-height rule. - The block's measured title budget rides along on `TimelineDrag`, so the copy spends its height the same way the block did; the copy draws through `BlockTitle` now rather than a bare `Text`, so a wrapped title clips the way the block's does. - That measurement moves into a shared `blockTimeLines`, which Week and Day each had their own copy of. Deviation from the issue: it names only the time label, but the title had the identical hardcode and the two share one height budget — fixing the range alone would just have moved the mismatch to the title. The copy still shows the range on a block too short to carry one; that is the feedback the drag exists for and is left as is. Closes #267 Co-authored-by: Jean-Luc Makiola Reviewed-on: https://codeberg.org/jlmakiola/calendula/pulls/269 --- .../calendula/ui/common/BlockPlacement.kt | 120 +++++------- .../calendula/ui/common/EventTitleOverflow.kt | 26 ++- .../calendula/ui/common/TimelineDrag.kt | 176 ++++++++++++++---- .../calendula/ui/common/TimelineScale.kt | 11 -- .../calendula/ui/day/DayScreen.kt | 48 +++-- .../calendula/ui/week/WeekScreen.kt | 58 +++--- .../ui/common/EventTitleOverflowTest.kt | 8 +- 7 files changed, 246 insertions(+), 201 deletions(-) diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/BlockPlacement.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/BlockPlacement.kt index 5ddc2b3..5605e13 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/BlockPlacement.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/BlockPlacement.kt @@ -4,7 +4,6 @@ import androidx.compose.animation.Crossfade import androidx.compose.animation.core.FiniteAnimationSpec import androidx.compose.animation.core.animateDpAsState import androidx.compose.animation.core.snap -import androidx.compose.foundation.layout.Column import androidx.compose.material3.ExperimentalMaterial3ExpressiveApi import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text @@ -15,14 +14,11 @@ import androidx.compose.runtime.remember import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.Color import androidx.compose.ui.platform.LocalDensity -import androidx.compose.ui.platform.LocalLayoutDirection import androidx.compose.ui.text.TextStyle import androidx.compose.ui.text.font.FontWeight import androidx.compose.ui.text.rememberTextMeasurer import androidx.compose.ui.text.style.TextDecoration -import androidx.compose.ui.text.style.TextOverflow import androidx.compose.ui.unit.Constraints -import androidx.compose.ui.unit.LayoutDirection import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.dp import de.jeanlucmakiola.floret.identity.rememberReduceMotion @@ -33,6 +29,9 @@ val BLOCK_OUTER_INSET = 1.dp /** Padding between a timed block's edge and its text. */ val BLOCK_TEXT_PADDING = 4.dp +/** The same, above and below — what a block's height has to pay before any text. */ +val BLOCK_TEXT_INSET = 2.dp + /** Most lines a time label may wrap over before it is worth more than a title line. */ const val MAX_TIME_LINES = 2 @@ -62,88 +61,61 @@ fun blockTextLines(text: String, style: TextStyle, textWidth: Dp, max: Int): Int } /** - * A timed block's title, over at most [maxLines]. + * Lines the time label may take at [textWidth], out of the [spare] height left + * once the title and the label's own first line are paid for. * - * Wrapping and clipping pull against each other, which is why #164 left the - * ellipsis on multi-line chips: with `softWrap` on, the last visible line ends - * at a word boundary, so "Farmers Market" in a six-character column would clip - * to "Farmer" / "s" where the ellipsis at least reached "s Mar…". + * A week column is narrower than a "09:30–11:00" range, so the label takes a + * second line rather than lose its end — but only out of a line the title + * measured itself as not needing, never one it would have filled. + */ +@Composable +fun blockTimeLines(label: String, textWidth: Dp, spare: Dp): Int { + val timeLineHeight = with(LocalDensity.current) { + MaterialTheme.typography.labelSmall.lineHeight.toDp() + } + return if (spare >= timeLineHeight) { + blockTextLines( + text = label, + // The style it is drawn in, or the budget measures a line the label + // never uses (#219). + style = MaterialTheme.typography.labelSmall.asEventTime(), + textWidth = textWidth, + max = MAX_TIME_LINES, + ) + } else { + 1 + } +} + +/** + * A timed block's title, over at most [maxLines], breaking at word boundaries. * - * So the block wraps every line but the last through one `Text` and hands the - * remainder to a second that clips mid-glyph the way a single-line chip does. - * Every line is then full and none of them spends two of its few characters on - * a "…". RTL keeps the ellipsis for the reason [eventTitleOverflowFor] gives. + * #164 clipped the last line mid-glyph so none of a narrow chip's few + * characters went on an ellipsis. On a block that can wrap, whole words read + * better than full lines do: "Farmers Market" over two lines beats "Farmer" / + * "s Marke". A single line still clips, having nowhere to wrap to. */ @Composable fun BlockTitle( title: String, maxLines: Int, - textWidth: Dp, color: Color, modifier: Modifier = Modifier, textDecoration: TextDecoration? = null, fontWeight: FontWeight? = null, ) { - // Folded into the style rather than passed to the Text, so the wrap measured - // below is the wrap that gets drawn. - val style = MaterialTheme.typography.labelMedium - .let { if (fontWeight == null) it else it.copy(fontWeight = fontWeight) } - val rtl = LocalLayoutDirection.current == LayoutDirection.Rtl - val measurer = rememberTextMeasurer() - val widthPx = with(LocalDensity.current) { textWidth.roundToPx() } - // Where the wrapped lines stop and the clipped tail starts — null when the - // title fits, and nothing needs splitting. - val headEnd = remember(title, style, widthPx, maxLines, rtl, measurer) { - if (rtl || maxLines < 2 || widthPx <= 0) { - null - } else { - val layout = measurer.measure( - text = title, - style = style, - constraints = Constraints(maxWidth = widthPx), - ) - if (layout.lineCount <= maxLines) { - null - } else { - layout.getLineEnd(maxLines - 2, visibleEnd = true) - } - } - } - if (headEnd == null) { - val overflow = eventTitleOverflow(singleLine = maxLines == 1) - Text( - text = title, - modifier = modifier, - style = style, - maxLines = maxLines, - overflow = overflow.overflow, - softWrap = overflow.softWrap, - color = color, - textDecoration = textDecoration, - ) - } else { - val tail = eventTitleOverflow(singleLine = true) - Column(modifier = modifier) { - Text( - text = title.substring(0, headEnd), - style = style, - maxLines = maxLines - 1, - overflow = TextOverflow.Clip, - softWrap = true, - color = color, - textDecoration = textDecoration, - ) - Text( - text = title.substring(headEnd).trimStart(), - style = style, - maxLines = 1, - overflow = tail.overflow, - softWrap = tail.softWrap, - color = color, - textDecoration = textDecoration, - ) - } - } + val overflow = eventTitleOverflow(singleLine = maxLines == 1) + Text( + text = title, + modifier = modifier, + style = MaterialTheme.typography.labelMedium + .let { if (fontWeight == null) it else it.copy(fontWeight = fontWeight) }, + maxLines = maxLines, + overflow = overflow.overflow, + softWrap = overflow.softWrap, + color = color, + textDecoration = textDecoration, + ) } /** diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/EventTitleOverflow.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/EventTitleOverflow.kt index 89304a3..1542165 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/EventTitleOverflow.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/EventTitleOverflow.kt @@ -11,23 +11,21 @@ data class EventTitleOverflow(val overflow: TextOverflow, val softWrap: Boolean) /** * Overflow for an event chip's title (#164). Chips are narrow enough that the * "…" costs a couple of readable characters, so the title runs to the chip's - * edge and clips mid-glyph instead. + * edge and clips instead — mid-glyph on one line, at the last whole word it + * could fit on several. * - * Two cases keep the ellipsis: - * - * - **[rtl].** With `softWrap` off Compose lays the line out at its full - * intrinsic width and clips to the node's left edge, which in RTL is the *end* - * of the string — an Arabic title would lose its beginning. The ellipsis - * truncates at the logical end in both directions. - * - **More than one line** ([singleLine] false). Wrapping needs `softWrap` on, - * and clipping with it on breaks the last line at the last whole word — less - * title than the ellipsis showed, not more. + * One case keeps the ellipsis: a single **[rtl]** line. With `softWrap` off + * Compose lays the line out at its full intrinsic width and clips to the node's + * left edge, which in RTL is the *end* of the string — an Arabic title would + * lose its beginning. The ellipsis truncates at the logical end in both + * directions. Wrapping needs `softWrap` on, which clips the right end either + * way, so more than one line needs no such exception. */ fun eventTitleOverflowFor(rtl: Boolean, singleLine: Boolean): EventTitleOverflow = - if (rtl || !singleLine) { - EventTitleOverflow(TextOverflow.Ellipsis, softWrap = true) - } else { - EventTitleOverflow(TextOverflow.Clip, softWrap = false) + when { + !singleLine -> EventTitleOverflow(TextOverflow.Clip, softWrap = true) + rtl -> EventTitleOverflow(TextOverflow.Ellipsis, softWrap = true) + else -> EventTitleOverflow(TextOverflow.Clip, softWrap = false) } /** [eventTitleOverflowFor] against the current layout direction. */ diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/TimelineDrag.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/TimelineDrag.kt index 5c79f3a..8fa21c7 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/TimelineDrag.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/TimelineDrag.kt @@ -11,6 +11,7 @@ import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.absoluteOffset import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.requiredSize import androidx.compose.foundation.layout.size import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.material3.MaterialTheme @@ -24,7 +25,9 @@ import androidx.compose.runtime.remember import androidx.compose.runtime.setValue import androidx.compose.runtime.withFrameNanos import androidx.compose.ui.Modifier +import androidx.compose.ui.draw.clipToBounds import androidx.compose.ui.geometry.Offset +import androidx.compose.ui.graphics.TransformOrigin import androidx.compose.ui.graphics.graphicsLayer import androidx.compose.ui.layout.LayoutCoordinates import androidx.compose.ui.layout.boundsInRoot @@ -81,6 +84,13 @@ data class TimelineDrag( val eventSpanMin: Int, /** The event as the grid would draw it, one piece per day column it covers. */ val pieces: List, + /** + * Lines the block the finger picked up drew its title over, zero for one too + * short to have drawn it at all. The floating copy is that block's size, so + * it has to spend its height the same way or the text re-wraps under the + * finger and snaps back on drop (#267). + */ + val titleLines: Int, /** * True when [pieces] is only [edgeDragSlice]'s stand-in — the event has left * every column this timeline shows. A hint under the finger, not where the @@ -311,6 +321,9 @@ class TimelineDragController { */ private var clipOffsetMin = 0 + /** What the picked-up block drew its title over — see [TimelineDrag.titleLines]. */ + private var titleLines = 0 + /** * The slot the block already occupied when it was picked up — not the same * as its start, since the target snaps to the grid (09:07 lifts to 09:00). @@ -320,11 +333,13 @@ class TimelineDragController { fun begin( block: TimedBlock, clipOffsetMin: Int, + titleLines: Int, pointerInRoot: Offset, blockInRoot: Offset, ) { source = block this.clipOffsetMin = clipOffsetMin + this.titleLines = titleLines settling = null isDragging = true liftedInstanceId = block.event.instanceId @@ -343,6 +358,7 @@ class TimelineDragController { fun cancel() { source = null clipOffsetMin = 0 + titleLines = 0 isDragging = false liftedInstanceId = null originSlot = null @@ -465,6 +481,7 @@ class TimelineDragController { startMin = startMin, eventStartMin = eventStartMin, eventSpanMin = eventSpan, + titleLines = titleLines, // Bounded to the columns this timeline actually shows: a day it // doesn't has no piece to draw. The write is unaffected. pieces = slices @@ -602,6 +619,7 @@ const val SETTLE_FADE_MILLIS: Int = 250 @Composable fun TimelineDragOverlay(controller: TimelineDragController, modifier: Modifier = Modifier) { var origin by remember { mutableStateOf(Offset.Zero) } + val density = LocalDensity.current val dark = isSystemInDarkTheme() val use24Hour = LocalUse24HourFormat.current val locale = currentLocale() @@ -663,23 +681,61 @@ fun TimelineDragOverlay(controller: TimelineDragController, modifier: Modifier = val endMin = if (rawEnd > MINUTES_PER_DAY) rawEnd % MINUTES_PER_DAY else rawEnd val label = "${formatMinuteOfDay(startMin, use24Hour, locale)}–" + formatMinuteOfDay(endMin, use24Hour, locale) - // The tallest piece carries the range: on the smallest it would be - // clipped away, which is exactly the case when a short tail is held. - val labelled = drag.pieces.indices.maxByOrNull { drag.pieces[it].sizePx.height } - drag.pieces.forEachIndexed { index, piece -> - DragCopy( - topLeftInRoot = piece.topLeftInRoot, - overlayOrigin = origin, - sizePx = piece.sizePx, - paint = paint, - shape = timedBlockShape(piece.continuesBefore, piece.continuesAfter), - cuts = timedBlockCuts(piece.continuesBefore, piece.continuesAfter), - lift = lift, - alpha = copyAlpha, - title = title, - // Once for the whole event: repeated, it would name it per day. - label = label.takeIf { index == labelled }, - ) + // The timeline's own bounds. The copy is drawn over the whole calendar + // so no column clip or rounded corner cuts it, but it still belongs to + // the timeline: a piece a whole day tall reaches far past both ends of + // the viewport, and unclipped it paints over the headers above (#267). + val port = controller.geometry.viewport?.takeIf { it.isAttached }?.boundsInRoot() + // How much of each piece the viewport actually shows. A multi-day event + // has a piece per day, and the widest one is a full 24 hours — taller + // than the screen, its top at a midnight scrolled out of sight. Picking + // the range's piece by raw height put it there, off the top of the + // timeline; picking by *visible* height puts it where it can be read. + val shown = drag.pieces.map { piece -> + if (port == null) { + piece.sizePx.height.toFloat() + } else { + val top = maxOf(piece.topLeftInRoot.y, port.top) + val bottom = minOf(piece.topLeftInRoot.y + piece.sizePx.height, port.bottom) + bottom - top + } + } + val labelled = shown.indices.maxByOrNull { shown[it] } + Box( + modifier = if (port == null) { + Modifier + } else { + Modifier + .absoluteOffset { + IntOffset( + (port.left - origin.x).roundToInt(), + (port.top - origin.y).roundToInt(), + ) + } + .size( + width = with(density) { port.width.toDp() }, + height = with(density) { port.height.toDp() }, + ) + .clipToBounds() + }, + ) { + val pieceOrigin = port?.topLeft ?: origin + drag.pieces.forEachIndexed { index, piece -> + DragCopy( + topLeftInRoot = piece.topLeftInRoot, + overlayOrigin = pieceOrigin, + sizePx = piece.sizePx, + paint = paint, + shape = timedBlockShape(piece.continuesBefore, piece.continuesAfter), + cuts = timedBlockCuts(piece.continuesBefore, piece.continuesAfter), + lift = lift, + alpha = copyAlpha, + title = title, + titleLines = drag.titleLines, + // Once for the whole event: repeated, it would name it per day. + label = label.takeIf { index == labelled }, + ) + } } } } @@ -696,9 +752,48 @@ private fun DragCopy( lift: Float, alpha: Float, title: String, + titleLines: Int, label: String?, ) { val density = LocalDensity.current + val width = with(density) { sizePx.width.toDp() } + val height = with(density) { sizePx.height.toDp() } + val textWidth = width - (BLOCK_OUTER_INSET + BLOCK_TEXT_PADDING) * 2 + val titleLineHeight = with(density) { + MaterialTheme.typography.labelMedium.lineHeight.toDp() + } + val timeLineHeight = with(density) { + MaterialTheme.typography.labelSmall.lineHeight.toDp() + } + // The block's size, spent the block's way — text sits at the top as it does + // on the block, so the copy hands back to the grid without shifting (#267). + // The title is served in full first and the range lives off what is left: + // the hour gutter down the side still says where the copy sits, so the + // range is the half that can afford to go. + val available = height - BLOCK_TEXT_INSET * 2 + val titleBudget = (available / titleLineHeight).toInt().coerceAtLeast(0) + val allowed = titleLines.coerceAtMost(titleBudget) + // Re-measured at the copy's own width rather than spent on the source's + // count: a block sharing its column with another is a lane wide where the + // copy is a whole column, so the source's second line is one the copy never + // draws — and reserving it took the range away with it (#267). + val lines = if (allowed <= 0) { + 0 + } else { + blockTextLines( + text = title, + style = MaterialTheme.typography.labelMedium.withTitleWeight(paint), + textWidth = textWidth, + max = allowed, + ) + } + val left = available - titleLineHeight * lines + val showTime = label != null && left >= timeLineHeight + val timeMaxLines = if (showTime) { + blockTimeLines(label!!, textWidth, left - timeLineHeight) + } else { + 1 + } Box( modifier = Modifier // Absolute: these are root coordinates, and the direction-aware @@ -709,12 +804,20 @@ private fun DragCopy( (topLeftInRoot.y - overlayOrigin.y).roundToInt(), ) } - .size( - width = with(density) { sizePx.width.toDp() }, - height = with(density) { sizePx.height.toDp() }, - ) + // Required: the parent is the viewport's size, and a plain `size` + // would let it cap a piece a whole day tall at one screen — drawn + // from a midnight scrolled off the top, that ended the column a + // scroll offset short of its own bottom (#267). The parent clips it. + .requiredSize(width = width, height = height) .padding(horizontal = BLOCK_OUTER_INSET) .graphicsLayer { + // Anchored at the top edge, not the middle: scaled about the + // centre, the 2% lift raises the top by 1% of the height — a few + // pixels on an hour, but a steady upward creep of the text as a + // dragged multi-day piece grows, and a shear against the + // timeline's clip. It snapped back when the lift animated out + // on drop (#267). + transformOrigin = TransformOrigin(0.5f, 0f) scaleX = 1f + 0.02f * lift scaleY = 1f + 0.02f * lift shadowElevation = 8.dp.toPx() * lift @@ -723,28 +826,27 @@ private fun DragCopy( clip = false } .eventSurface(paint, shape, cuts) - .padding(horizontal = BLOCK_TEXT_PADDING, vertical = 2.dp), + .padding(horizontal = BLOCK_TEXT_PADDING, vertical = BLOCK_TEXT_INSET), ) { Column { - val titleOverflow = eventTitleOverflow() - Text( - text = title, - style = MaterialTheme.typography.labelMedium, - maxLines = 1, - overflow = titleOverflow.overflow, - softWrap = titleOverflow.softWrap, - color = paint.titleInk, - fontWeight = paint.titleWeight, - textDecoration = paint.decoration, - ) - if (label != null) { + if (lines > 0) { + BlockTitle( + title = title, + maxLines = lines, + color = paint.titleInk, + textDecoration = paint.decoration, + fontWeight = paint.titleWeight, + ) + } + if (showTime) { + val overflow = eventTitleOverflow(singleLine = timeMaxLines == 1) Text( text = label, // As the block it lifted off sets its own time (#219). style = MaterialTheme.typography.labelSmall.asEventTime(), - maxLines = 1, - overflow = titleOverflow.overflow, - softWrap = titleOverflow.softWrap, + maxLines = timeMaxLines, + overflow = overflow.overflow, + softWrap = overflow.softWrap, color = paint.secondaryInk, ) } diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/TimelineScale.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/TimelineScale.kt index b236aef..d7c69f1 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/TimelineScale.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/TimelineScale.kt @@ -96,17 +96,6 @@ fun tappedMinuteOfDay(offsetY: Float, hourPx: Float): Int = */ const val MIN_EVENT_FRACTION = 26f / 60f -/** - * Narrowest an event block may be and still wrap its title over several lines. - * - * Wrapping is driven by the block's height, so a tall block on a lane-split - * column would otherwise stack two or three characters per line — "Da/ily", - * "Fa/rmer/s…" — which reads worse than one ellipsised line. A full week column - * clears this on any phone; a split one never does, while the day view's much - * wider columns keep wrapping even several lanes deep. - */ -val MIN_TITLE_WRAP_WIDTH = 36.dp - /** Smallest hour height [TimelineScale.FitDay] will resolve to. */ val FIT_DAY_MIN = 24.dp diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/day/DayScreen.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/day/DayScreen.kt index 53bd0a4..bbd2669 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/day/DayScreen.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/day/DayScreen.kt @@ -83,10 +83,10 @@ import de.jeanlucmakiola.calendula.ui.common.CalendarView import de.jeanlucmakiola.calendula.ui.common.IMPLEMENTED_VIEWS import de.jeanlucmakiola.calendula.ui.common.BLOCK_OUTER_INSET import de.jeanlucmakiola.calendula.ui.common.BLOCK_TEXT_PADDING +import de.jeanlucmakiola.calendula.ui.common.BLOCK_TEXT_INSET import de.jeanlucmakiola.calendula.ui.common.BlockTimeLabel import de.jeanlucmakiola.calendula.ui.common.BlockTitle -import de.jeanlucmakiola.calendula.ui.common.MAX_TIME_LINES -import de.jeanlucmakiola.calendula.ui.common.blockTextLines +import de.jeanlucmakiola.calendula.ui.common.blockTimeLines import de.jeanlucmakiola.calendula.ui.common.animatedBlockPlacement import de.jeanlucmakiola.calendula.ui.common.ghostAlpha import de.jeanlucmakiola.calendula.ui.common.LocalEventMove @@ -120,7 +120,6 @@ import de.jeanlucmakiola.calendula.ui.common.LocalUse24HourFormat import de.jeanlucmakiola.calendula.ui.common.LocalShowHourLines import de.jeanlucmakiola.calendula.ui.common.LocalTimelineZoom import de.jeanlucmakiola.calendula.ui.common.MIN_EVENT_FRACTION -import de.jeanlucmakiola.calendula.ui.common.asEventTime import de.jeanlucmakiola.calendula.ui.common.hourHeight import de.jeanlucmakiola.calendula.ui.common.rememberTimelinePinchZoom import de.jeanlucmakiola.calendula.ui.common.formatMinuteOfDay @@ -738,33 +737,25 @@ private fun EventBlock( val timeLineHeight = with(density) { MaterialTheme.typography.labelSmall.lineHeight.toDp() } - // What's left for text once the 2.dp top/bottom padding is paid for. A block - // that cannot afford both lines spends its space on the title, and one too - // short even for that drops the title rather than serving a sliced one. + // A block that cannot afford both lines spends its space on the title, and + // one too short even for that drops the title rather than serving a sliced one. // Height alone decides: a duration threshold would keep hiding the time on a // half-hour block the user has pinched open to three times the room it needs. - val available = height - 4.dp + val available = height - BLOCK_TEXT_INSET * 2 val showTime = available >= titleLineHeight + timeLineHeight val showTitle = available >= titleLineHeight val textWidth = width - (BLOCK_OUTER_INSET + BLOCK_TEXT_PADDING) * 2 - val titleMaxLines = if (showTime) 1 else 2 - // The range only wraps out of a line the title has not claimed, which on a - // day column — wide enough for "09:30–11:00" several times over — means it - // never does, until lanes cut the column down. + // Only lines the block can actually draw: a block too short for the time is + // too short for a second title line too, and asking for one served a sliced + // one — as well as handing the drag copy a count it couldn't honour, so the + // title re-wrapped the moment the block was lifted (#267). + val titleBudget = (available / titleLineHeight).toInt().coerceAtLeast(1) + val titleMaxLines = if (showTime) 1 else titleBudget.coerceAtMost(2) + // On a day column — wide enough for "09:30–11:00" several times over — the + // range never needs the second line, until lanes cut the column down. val spare = available - titleLineHeight * titleMaxLines - if (showTime) timeLineHeight else 0.dp - val timeMaxLines = if (showTime && spare >= timeLineHeight) { - blockTextLines( - text = timeLabel, - // The style it is drawn in, or the budget measures a line the label - // never uses (#219). - style = MaterialTheme.typography.labelSmall.asEventTime(), - textWidth = textWidth, - max = MAX_TIME_LINES, - ) - } else { - 1 - } + val timeMaxLines = if (showTime) blockTimeLines(timeLabel, textWidth, spare) else 1 val paint = eventPaint(block.event, dark) val zone = remember { TimeZone.currentSystemDefault() } val moveAction = eventMoveAction(block.event) @@ -780,7 +771,13 @@ private fun EventBlock( enabled = draggable, key = block.event.instanceId, onPickUp = { pointer, blockRoot, _ -> - dragController.begin(block, clipOffset, pointer, blockRoot) + dragController.begin( + block = block, + clipOffsetMin = clipOffset, + titleLines = if (showTitle) titleMaxLines else 0, + pointerInRoot = pointer, + blockInRoot = blockRoot, + ) }, onMove = dragController::move, onDrop = { dragController.finish()?.let(onDrop) }, @@ -797,7 +794,7 @@ private fun EventBlock( // After clickable, so it is the inner node and wins the main pass; // the tap still works, since a drag consumes the up. .then(dragModifier) - .padding(horizontal = BLOCK_TEXT_PADDING, vertical = 2.dp) + .padding(horizontal = BLOCK_TEXT_PADDING, vertical = BLOCK_TEXT_INSET) .semantics { contentDescription = "$title, $timeLabel" if (moveAction != null) customActions = listOf(moveAction) @@ -808,7 +805,6 @@ private fun EventBlock( BlockTitle( title = title, maxLines = titleMaxLines, - textWidth = textWidth, color = paint.titleInk, textDecoration = paint.decoration, fontWeight = paint.titleWeight, diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/week/WeekScreen.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/week/WeekScreen.kt index 740dd7f..0e5d1cd 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/week/WeekScreen.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/week/WeekScreen.kt @@ -91,10 +91,11 @@ import de.jeanlucmakiola.calendula.ui.common.IMPLEMENTED_VIEWS import de.jeanlucmakiola.calendula.ui.common.EventDimAlpha import de.jeanlucmakiola.calendula.ui.common.BLOCK_OUTER_INSET import de.jeanlucmakiola.calendula.ui.common.BLOCK_TEXT_PADDING +import de.jeanlucmakiola.calendula.ui.common.BLOCK_TEXT_INSET import de.jeanlucmakiola.calendula.ui.common.BlockTimeLabel import de.jeanlucmakiola.calendula.ui.common.BlockTitle -import de.jeanlucmakiola.calendula.ui.common.MAX_TIME_LINES import de.jeanlucmakiola.calendula.ui.common.blockTextLines +import de.jeanlucmakiola.calendula.ui.common.blockTimeLines import de.jeanlucmakiola.calendula.ui.common.animatedBlockPlacement import de.jeanlucmakiola.calendula.ui.common.ghostAlpha import de.jeanlucmakiola.calendula.ui.common.LocalEventMove @@ -129,8 +130,6 @@ import de.jeanlucmakiola.calendula.ui.common.LocalUse24HourFormat import de.jeanlucmakiola.calendula.ui.common.LocalShowHourLines import de.jeanlucmakiola.calendula.ui.common.LocalTimelineZoom import de.jeanlucmakiola.calendula.ui.common.MIN_EVENT_FRACTION -import de.jeanlucmakiola.calendula.ui.common.MIN_TITLE_WRAP_WIDTH -import de.jeanlucmakiola.calendula.ui.common.asEventTime import de.jeanlucmakiola.calendula.ui.common.hourHeight import de.jeanlucmakiola.calendula.ui.common.rememberTimelinePinchZoom import de.jeanlucmakiola.calendula.ui.common.formatMinuteOfDay @@ -883,8 +882,7 @@ private fun EventBlock( val timeLineHeight = with(density) { MaterialTheme.typography.labelSmall.lineHeight.toDp() } - // What's left for text once the 2.dp top/bottom padding is paid for. - val available = height - 4.dp + val available = height - BLOCK_TEXT_INSET * 2 // Only full-width (non-overlapping) blocks that are tall enough show the // time. On narrow overlapping columns we drop it so the title can wrap to // fill the whole block, mirroring Google Calendar — and a block that cannot @@ -906,35 +904,20 @@ private fun EventBlock( val contentHeight = available - if (showTime) timeLineHeight else 0.dp val titleBudget = (contentHeight / titleLineHeight).toInt().coerceAtLeast(1) val paint = eventPaint(block.event, dark) - val titleMaxLines = if (width < MIN_TITLE_WRAP_WIDTH) { - 1 - } else { - blockTextLines( - text = title, - // At the weight BlockTitle will set it in, or an invited block — - // drawn a weight lighter — is budgeted a line it never fills (#230). - style = MaterialTheme.typography.labelMedium.withTitleWeight(paint), - textWidth = textWidth, - max = titleBudget, - ) - } - // A week column is narrower than a "09:30–11:00" range, so the label takes a - // second line rather than lose its end — but only out of a line the title - // measured itself as not needing, never one it would have filled. + // Every line the height affords, however narrow the lane: two events side by + // side leave columns well under a word wide, and cutting the title to one + // line there lost it outright where the block had the room to wrap it. + val titleMaxLines = blockTextLines( + text = title, + // At the weight BlockTitle will set it in, or an invited block — drawn a + // weight lighter — is budgeted a line it never fills (#230). + style = MaterialTheme.typography.labelMedium.withTitleWeight(paint), + textWidth = textWidth, + max = titleBudget, + ) val spare = available - titleLineHeight * titleMaxLines - if (showTime) timeLineHeight else 0.dp - val timeMaxLines = if (showTime && spare >= timeLineHeight) { - blockTextLines( - text = timeLabel, - // The style it is drawn in, or the budget measures a line the label - // never uses (#219). - style = MaterialTheme.typography.labelSmall.asEventTime(), - textWidth = textWidth, - max = MAX_TIME_LINES, - ) - } else { - 1 - } + val timeMaxLines = if (showTime) blockTimeLines(timeLabel, textWidth, spare) else 1 val dimCutoff = LocalDimCutoff.current val dimmed = dimCutoff != null && block.event.hasEnded(dimCutoff) val zone = remember { TimeZone.currentSystemDefault() } @@ -951,7 +934,13 @@ private fun EventBlock( enabled = draggable, key = block.event.instanceId, onPickUp = { pointer, blockRoot, _ -> - dragController.begin(block, clipOffset, pointer, blockRoot) + dragController.begin( + block = block, + clipOffsetMin = clipOffset, + titleLines = if (showTitle) titleMaxLines else 0, + pointerInRoot = pointer, + blockInRoot = blockRoot, + ) }, onMove = dragController::move, onDrop = { dragController.finish()?.let(onDrop) }, @@ -968,7 +957,7 @@ private fun EventBlock( // After clickable, so it is the inner node and wins the main pass; // the tap still works, since a drag consumes the up. .then(dragModifier) - .padding(horizontal = BLOCK_TEXT_PADDING, vertical = 2.dp) + .padding(horizontal = BLOCK_TEXT_PADDING, vertical = BLOCK_TEXT_INSET) .semantics { contentDescription = "$title, $timeLabel" if (moveAction != null) customActions = listOf(moveAction) @@ -979,7 +968,6 @@ private fun EventBlock( BlockTitle( title = title, maxLines = titleMaxLines, - textWidth = textWidth, color = paint.titleInk, textDecoration = paint.decoration, fontWeight = paint.titleWeight, diff --git a/app/src/test/java/de/jeanlucmakiola/calendula/ui/common/EventTitleOverflowTest.kt b/app/src/test/java/de/jeanlucmakiola/calendula/ui/common/EventTitleOverflowTest.kt index 7fd56b7..def31a9 100644 --- a/app/src/test/java/de/jeanlucmakiola/calendula/ui/common/EventTitleOverflowTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/calendula/ui/common/EventTitleOverflowTest.kt @@ -25,16 +25,16 @@ class EventTitleOverflowTest { } @Test - fun `a multi-line block keeps the ellipsis, since wrapping needs softWrap`() { + fun `a multi-line block wraps whole words and clips, spending none on a dot`() { val result = eventTitleOverflowFor(rtl = false, singleLine = false) - assertThat(result.overflow).isEqualTo(TextOverflow.Ellipsis) + assertThat(result.overflow).isEqualTo(TextOverflow.Clip) assertThat(result.softWrap).isTrue() } @Test - fun `multi-line in RTL keeps the ellipsis too`() { + fun `multi-line in RTL clips too, since softWrap cuts the logical end`() { val result = eventTitleOverflowFor(rtl = true, singleLine = false) - assertThat(result.overflow).isEqualTo(TextOverflow.Ellipsis) + assertThat(result.overflow).isEqualTo(TextOverflow.Clip) assertThat(result.softWrap).isTrue() } }