fix(widget): address review of the agenda size scaling (#51)
Follow-up to 30fcbfa, fixing eight issues found in review:
- Cap the agenda row list at 100. SizeMode.Exact asks Glance for one
RemoteViews per host size where SizeMode.Single produced exactly one,
roughly doubling the payload; with the range reaching
AgendaRange.MAX_CUSTOM_DAYS (365) an uncapped list could push past the
binder transaction limit and the host would show "Problem loading
widget". A trailing day header stranded by the cut is dropped.
- Loosen the height cap so it only catches genuinely squashed widgets.
It previously required ~320dp of height for LARGE, so widening a widget
without also making it unusually tall — the exact resize #51 reports —
stayed REGULAR or COMPACT and the feature was near a no-op for it.
Thresholds now work on height minus header chrome.
- Lock the event stripe to the system font scale. It is a Dp beside sp
text, so at large accessibility settings the text outgrew it and it
under-ran the row it marks.
- Route the day-header and placeholder padding through the metrics table
so vertical rhythm holds at the larger tiers, and derive the text
indent from the row constants instead of a hardcoded 19dp.
- Share the bucketing as widget/WidgetScale.kt so MonthWidget (already
SizeMode.Exact) can adopt one rule rather than growing a parallel copy.
- Anchor the type ramp to Material 3 type-scale roles per CLAUDE.md, with
the two off-scale values marked and justified inline. COMPACT is
unchanged, so a default-sized widget still looks exactly as before.
- Tie the "default size unchanged" test to the provider XML's declared
3-cell band rather than one measured 222dp point.
- Hang the metrics off an ordinal-indexed table so lookup allocates
nothing per recomposition.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,89 @@
|
||||
package de.jeanlucmakiola.calendula.widget
|
||||
|
||||
import androidx.compose.ui.unit.DpSize
|
||||
import androidx.compose.ui.unit.dp
|
||||
import com.google.common.truth.Truth.assertThat
|
||||
import org.junit.jupiter.api.Test
|
||||
|
||||
class WidgetScaleTest {
|
||||
|
||||
@Test
|
||||
fun `the on-device calibration points map to their tiers`() {
|
||||
// The two sizes measured on-device: the compact widget stays COMPACT (the
|
||||
// baseline, unchanged), the full-width one steps up to LARGE — not XLARGE,
|
||||
// which read as too big on a phone (#51).
|
||||
assertThat(scaleFor(DpSize(222.dp, 270.dp))).isEqualTo(WidgetScale.COMPACT)
|
||||
assertThat(scaleFor(DpSize(378.dp, 672.dp))).isEqualTo(WidgetScale.LARGE)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a full-width widget at ordinary height still scales up`() {
|
||||
// The regression the height cap used to cause: widening the widget without
|
||||
// also making it unusually tall is *the* resize #51 reports, and it must
|
||||
// reach the tier its width earned. Three cells tall is about 270dp.
|
||||
assertThat(scaleFor(DpSize(378.dp, 270.dp))).isEqualTo(WidgetScale.LARGE)
|
||||
assertThat(scaleFor(DpSize(300.dp, 270.dp))).isEqualTo(WidgetScale.REGULAR)
|
||||
assertThat(scaleFor(DpSize(460.dp, 300.dp))).isEqualTo(WidgetScale.XLARGE)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `width buckets into the four tiers`() {
|
||||
// Tall enough that the height cap never binds, isolating the width rule.
|
||||
val h = 500.dp
|
||||
assertThat(scaleFor(DpSize(180.dp, h))).isEqualTo(WidgetScale.COMPACT)
|
||||
assertThat(scaleFor(DpSize(259.dp, h))).isEqualTo(WidgetScale.COMPACT)
|
||||
assertThat(scaleFor(DpSize(260.dp, h))).isEqualTo(WidgetScale.REGULAR)
|
||||
assertThat(scaleFor(DpSize(329.dp, h))).isEqualTo(WidgetScale.REGULAR)
|
||||
assertThat(scaleFor(DpSize(330.dp, h))).isEqualTo(WidgetScale.LARGE)
|
||||
assertThat(scaleFor(DpSize(419.dp, h))).isEqualTo(WidgetScale.LARGE)
|
||||
assertThat(scaleFor(DpSize(420.dp, h))).isEqualTo(WidgetScale.XLARGE)
|
||||
assertThat(scaleFor(DpSize(900.dp, h))).isEqualTo(WidgetScale.XLARGE)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `extra height never raises the tier`() {
|
||||
// Height decides how many rows are visible, not how big they are: a tall,
|
||||
// narrow widget wants more events, not bigger text.
|
||||
assertThat(scaleFor(DpSize(222.dp, 200.dp))).isEqualTo(WidgetScale.COMPACT)
|
||||
assertThat(scaleFor(DpSize(222.dp, 900.dp))).isEqualTo(WidgetScale.COMPACT)
|
||||
assertThat(scaleFor(DpSize(300.dp, 900.dp))).isEqualTo(WidgetScale.REGULAR)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `only a genuinely squashed widget is stepped back down`() {
|
||||
// Same (wide) width, shrinking height. The cap exists to stop oversized type
|
||||
// surviving in a one- or two-row sliver — it must not fire at normal heights.
|
||||
// Heights are gross; the cap works on height minus 60dp of chrome, so the
|
||||
// LARGE floor is 190dp (130dp usable) and the REGULAR floor 130dp (70dp).
|
||||
val wide = 378.dp
|
||||
assertThat(scaleFor(DpSize(wide, 400.dp))).isEqualTo(WidgetScale.LARGE)
|
||||
assertThat(scaleFor(DpSize(wide, 260.dp))).isEqualTo(WidgetScale.LARGE)
|
||||
assertThat(scaleFor(DpSize(wide, 190.dp))).isEqualTo(WidgetScale.LARGE)
|
||||
assertThat(scaleFor(DpSize(wide, 189.dp))).isEqualTo(WidgetScale.REGULAR)
|
||||
assertThat(scaleFor(DpSize(wide, 130.dp))).isEqualTo(WidgetScale.REGULAR)
|
||||
assertThat(scaleFor(DpSize(wide, 129.dp))).isEqualTo(WidgetScale.COMPACT)
|
||||
// The provider's declared floor (minResizeWidth/Height = 110dp) is COMPACT.
|
||||
assertThat(scaleFor(DpSize(110.dp, 110.dp))).isEqualTo(WidgetScale.COMPACT)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `the tier is monotonic in both axes`() {
|
||||
// Growing a widget must never make its type smaller. Sweeps the whole
|
||||
// plausible range rather than spot-checking, so a future threshold edit
|
||||
// can't accidentally invert a step.
|
||||
val widths = (110..900 step 7).map { it.dp }
|
||||
val heights = (110..900 step 7).map { it.dp }
|
||||
widths.forEach { w ->
|
||||
heights.zipWithNext { shorter, taller ->
|
||||
assertThat(scaleFor(DpSize(w, taller)))
|
||||
.isAtLeast(scaleFor(DpSize(w, shorter)))
|
||||
}
|
||||
}
|
||||
heights.forEach { h ->
|
||||
widths.zipWithNext { narrower, wider ->
|
||||
assertThat(scaleFor(DpSize(wider, h)))
|
||||
.isAtLeast(scaleFor(DpSize(narrower, h)))
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -4,70 +4,32 @@ import androidx.compose.ui.unit.DpSize
|
||||
import androidx.compose.ui.unit.dp
|
||||
import androidx.compose.ui.unit.sp
|
||||
import com.google.common.truth.Truth.assertThat
|
||||
import de.jeanlucmakiola.calendula.widget.WidgetScale
|
||||
import de.jeanlucmakiola.calendula.widget.scaleFor
|
||||
import org.junit.jupiter.api.Test
|
||||
|
||||
class AgendaScaleTest {
|
||||
|
||||
// --- scaleFor: bucketing -------------------------------------------------
|
||||
// --- the "default size is unchanged" regression guard ---------------------
|
||||
|
||||
@Test
|
||||
fun `the on-device calibration points map to their tiers`() {
|
||||
// The two sizes measured on-device: the compact widget stays COMPACT (the
|
||||
// baseline, unchanged), the large one steps up to LARGE — not XLARGE,
|
||||
// which read as too big on a phone (#51).
|
||||
assertThat(scaleFor(DpSize(222.dp, 270.dp))).isEqualTo(AgendaScale.COMPACT)
|
||||
assertThat(scaleFor(DpSize(378.dp, 672.dp))).isEqualTo(AgendaScale.LARGE)
|
||||
fun `every default placement width stays COMPACT`() {
|
||||
// Ties the guarantee to the provider XML's declared 3-cell default rather
|
||||
// than to one measured launcher: the whole band a default placement can
|
||||
// land in must bucket to COMPACT, or a freshly placed widget silently
|
||||
// changes appearance (#51). See AGENDA_DEFAULT_WIDTH_BAND.
|
||||
val band = AGENDA_DEFAULT_WIDTH_BAND
|
||||
var w = band.start
|
||||
while (w <= band.endInclusive) {
|
||||
assertThat(scaleFor(DpSize(w, 270.dp))).isEqualTo(WidgetScale.COMPACT)
|
||||
w += 1.dp
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `width buckets into the four tiers`() {
|
||||
// Tall enough that the height cap never binds, isolating the width rule.
|
||||
val h = 500.dp
|
||||
assertThat(scaleFor(DpSize(180.dp, h))).isEqualTo(AgendaScale.COMPACT)
|
||||
assertThat(scaleFor(DpSize(259.dp, h))).isEqualTo(AgendaScale.COMPACT)
|
||||
assertThat(scaleFor(DpSize(260.dp, h))).isEqualTo(AgendaScale.REGULAR)
|
||||
assertThat(scaleFor(DpSize(329.dp, h))).isEqualTo(AgendaScale.REGULAR)
|
||||
assertThat(scaleFor(DpSize(330.dp, h))).isEqualTo(AgendaScale.LARGE)
|
||||
assertThat(scaleFor(DpSize(419.dp, h))).isEqualTo(AgendaScale.LARGE)
|
||||
assertThat(scaleFor(DpSize(420.dp, h))).isEqualTo(AgendaScale.XLARGE)
|
||||
assertThat(scaleFor(DpSize(900.dp, h))).isEqualTo(AgendaScale.XLARGE)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `extra height never raises the tier`() {
|
||||
// Height decides how many rows are visible, not how big they are: a tall,
|
||||
// narrow widget wants more events, not bigger text.
|
||||
val short = scaleFor(DpSize(222.dp, 200.dp))
|
||||
val tall = scaleFor(DpSize(222.dp, 900.dp))
|
||||
assertThat(short).isEqualTo(AgendaScale.COMPACT)
|
||||
assertThat(tall).isEqualTo(AgendaScale.COMPACT)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a squashed widget is stepped back down`() {
|
||||
// Same (wide) width, shrinking height: the tier must walk down rather than
|
||||
// keep big type in a sliver of space.
|
||||
val wide = 378.dp
|
||||
assertThat(scaleFor(DpSize(wide, 672.dp))).isEqualTo(AgendaScale.LARGE)
|
||||
assertThat(scaleFor(DpSize(wide, 400.dp))).isEqualTo(AgendaScale.LARGE)
|
||||
assertThat(scaleFor(DpSize(wide, 300.dp))).isEqualTo(AgendaScale.REGULAR)
|
||||
assertThat(scaleFor(DpSize(wide, 200.dp))).isEqualTo(AgendaScale.COMPACT)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `the height cap never exceeds what the width earned`() {
|
||||
// A tall but narrow widget stays at its width's tier — the cap only ever
|
||||
// lowers, so a huge height can't promote a 222dp-wide widget.
|
||||
assertThat(scaleFor(DpSize(222.dp, 900.dp))).isEqualTo(AgendaScale.COMPACT)
|
||||
assertThat(scaleFor(DpSize(300.dp, 900.dp))).isEqualTo(AgendaScale.REGULAR)
|
||||
}
|
||||
|
||||
// --- metricsFor: the "default unchanged" regression guard -----------------
|
||||
|
||||
@Test
|
||||
fun `COMPACT metrics equal the widget's original constants`() {
|
||||
// If this fails, a default-sized agenda widget no longer looks as it did.
|
||||
val m = metricsFor(AgendaScale.COMPACT)
|
||||
val m = metricsFor(WidgetScale.COMPACT)
|
||||
assertThat(m.title).isEqualTo(16.sp)
|
||||
assertThat(m.dayHeader).isEqualTo(13.sp)
|
||||
assertThat(m.eventTitle).isEqualTo(14.sp)
|
||||
@@ -78,16 +40,22 @@ class AgendaScaleTest {
|
||||
assertThat(m.iconImage).isEqualTo(22.dp)
|
||||
assertThat(m.iconBox).isEqualTo(40.dp)
|
||||
assertThat(m.rowVPad).isEqualTo(4.dp)
|
||||
assertThat(m.dayHeaderTopPad).isEqualTo(10.dp)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `type sizes are non-decreasing across the tiers`() {
|
||||
val tiers = listOf(
|
||||
AgendaScale.COMPACT,
|
||||
AgendaScale.REGULAR,
|
||||
AgendaScale.LARGE,
|
||||
AgendaScale.XLARGE,
|
||||
).map(::metricsFor)
|
||||
fun `the derived text indent matches the original hardcoded inset`() {
|
||||
// TEXT_INDENT replaced a hardcoded 19dp; it must still resolve to 19dp or
|
||||
// day headers and the placeholder line stop aligning with event titles.
|
||||
assertThat(TEXT_INDENT).isEqualTo(19.dp)
|
||||
assertThat(TEXT_INDENT).isEqualTo(ROW_H_PAD + STRIPE_WIDTH + STRIPE_GAP)
|
||||
}
|
||||
|
||||
// --- the ramp ------------------------------------------------------------
|
||||
|
||||
@Test
|
||||
fun `sizes are non-decreasing across the tiers`() {
|
||||
val tiers = WidgetScale.entries.map(::metricsFor)
|
||||
|
||||
tiers.zipWithNext { small, big ->
|
||||
assertThat(big.title.value).isAtLeast(small.title.value)
|
||||
@@ -100,6 +68,54 @@ class AgendaScaleTest {
|
||||
assertThat(big.iconImage.value).isAtLeast(small.iconImage.value)
|
||||
assertThat(big.iconBox.value).isAtLeast(small.iconBox.value)
|
||||
assertThat(big.rowVPad.value).isAtLeast(small.rowVPad.value)
|
||||
assertThat(big.dayHeaderTopPad.value).isAtLeast(small.dayHeaderTopPad.value)
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `the event title keeps its lead over the time line`() {
|
||||
// The secondary line steps more slowly on purpose; if it ever caught up the
|
||||
// row would lose its hierarchy.
|
||||
WidgetScale.entries.map(::metricsFor).forEach { m ->
|
||||
assertThat(m.eventTitle.value).isGreaterThan(m.eventTime.value)
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `no tier grows type more than half again over the baseline`() {
|
||||
// Guards against a future edit turning "more readable" into "absurd".
|
||||
val base = metricsFor(WidgetScale.COMPACT)
|
||||
val top = metricsFor(WidgetScale.XLARGE)
|
||||
assertThat(top.title.value / base.title.value).isLessThan(1.5f)
|
||||
assertThat(top.eventTitle.value / base.eventTitle.value).isLessThan(1.5f)
|
||||
}
|
||||
|
||||
// --- font scale ----------------------------------------------------------
|
||||
|
||||
@Test
|
||||
fun `the stripe tracks the system font scale`() {
|
||||
// The stripe is Dp, the text beside it is sp: without this the two diverge
|
||||
// at large accessibility font settings and the stripe under-runs the row.
|
||||
val m = metricsFor(WidgetScale.COMPACT)
|
||||
assertThat(m.scaledForFont(1f).stripeH).isEqualTo(36.dp)
|
||||
assertThat(m.scaledForFont(1.3f).stripeH.value).isWithin(0.01f).of(46.8f)
|
||||
assertThat(m.scaledForFont(0.85f).stripeH.value).isWithin(0.01f).of(30.6f)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `scaling for the default font scale changes nothing`() {
|
||||
val m = metricsFor(WidgetScale.LARGE)
|
||||
assertThat(m.scaledForFont(1f)).isSameInstanceAs(m)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `font scaling leaves the sp sizes alone`() {
|
||||
// Glance already applies the font scale to sp; scaling them here too would
|
||||
// double-count it.
|
||||
val m = metricsFor(WidgetScale.REGULAR)
|
||||
val scaled = m.scaledForFont(1.3f)
|
||||
assertThat(scaled.title).isEqualTo(m.title)
|
||||
assertThat(scaled.eventTitle).isEqualTo(m.eventTitle)
|
||||
assertThat(scaled.rowVPad).isEqualTo(m.rowVPad)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user