From 4be2156e58401e75a9933bbdbf5106a7dbad0c72 Mon Sep 17 00:00:00 2001 From: freetlab Date: Thu, 30 Jul 2026 18:26:25 +0200 Subject: [PATCH] fix(nav,favoritos): unclip the overflow menu and smooth the tab transition Two user-reported bugs from on-device testing. The favourites overflow menu carried `constraints: tightFor(38x42)`, which sizes the POPUP rather than the button -- every item was clipped to its first letter, so users saw "M" and "E" instead of the labels. The existing test passed throughout because find.text matches a Text widget whether or not it is visually clipped; the new guard measures the laid-out width instead. The bottom bar's ink splash had no shape, painting a hard square over the icon, and the active tab's lift, dim, icon size and label all changed instantly while the balloon slid -- the balloon glided and its contents teleported. All four now share the balloon's duration and curve. --- lib/pantallas/pantalla_favoritos.dart | 8 +- lib/widgets/pluri_bottom_navigation.dart | 122 +++++++++++------- test/pantallas/pantalla_favoritos_test.dart | 18 +++ .../widgets/pluri_bottom_navigation_test.dart | 8 +- 4 files changed, 107 insertions(+), 49 deletions(-) diff --git a/lib/pantallas/pantalla_favoritos.dart b/lib/pantallas/pantalla_favoritos.dart index 471f27b..3753a0b 100644 --- a/lib/pantallas/pantalla_favoritos.dart +++ b/lib/pantallas/pantalla_favoritos.dart @@ -465,7 +465,13 @@ class _FilaFavorito extends StatelessWidget { context, ).colorScheme.onSurface.withValues(alpha: 0.45), ), - constraints: const BoxConstraints.tightFor(width: 38, height: 42), + // NO `constraints:` here. That property sizes the POPUP MENU, not + // the button — a tightFor(38x42) clipped every menu item down to + // its first letter ("M" for "Mover a lista", "E" for "Eliminar de + // favoritos"), which is what users actually saw. Constrain the + // tap target instead. + padding: EdgeInsets.zero, + iconSize: 20, onSelected: (accion) { if (accion == 'assign') _asignar(context); if (accion == 'remove') _eliminar(context); diff --git a/lib/widgets/pluri_bottom_navigation.dart b/lib/widgets/pluri_bottom_navigation.dart index 40d1fd1..9aec675 100644 --- a/lib/widgets/pluri_bottom_navigation.dart +++ b/lib/widgets/pluri_bottom_navigation.dart @@ -192,7 +192,10 @@ class PluriBottomNavigation extends StatelessWidget { /// the balloon's opaque fill already covers the seam where the bar's own /// shadow would otherwise show through. List get _shellShadows => [ - BoxShadow(color: Colors.white.withValues(alpha: 0.17), offset: const Offset(0, -1.5)), + BoxShadow( + color: Colors.white.withValues(alpha: 0.17), + offset: const Offset(0, -1.5), + ), BoxShadow( color: Colors.black.withValues(alpha: 0.5), offset: const Offset(0, 14), @@ -214,6 +217,7 @@ class _PluriNavButton extends StatelessWidget { @override Widget build(BuildContext context) { final t = context.pluriTokens; + final motion = context.pluriMotion; return Semantics( button: true, selected: selected, @@ -222,6 +226,11 @@ class _PluriNavButton extends StatelessWidget { type: MaterialType.transparency, child: InkWell( onTap: onTap, + // Without a shape the ink splash and hover highlight paint as a + // full-bleed RECTANGLE over the cell — a hard square sitting on + // top of the icon, which is what users reported. The whole bar is + // built on 999-radius pills; the splash has to follow. + borderRadius: BorderRadius.circular(999), child: Align( alignment: Alignment.bottomCenter, // t4/4a spec: the items row itself is 52px tall — this inner @@ -238,54 +247,75 @@ class _PluriNavButton extends StatelessWidget { // Flutter joins merged labels with `\n`, so screen readers // would announce "Alarmas\nAlarmas" instead of "Alarmas". child: ExcludeSemantics( - child: Transform.translate( - // t4/4a spec: active item lift `translateY(-15px)`. - offset: Offset(0, selected ? -15 : 0), - child: Column( - mainAxisSize: MainAxisSize.min, - children: [ - Opacity( - // t4/4a spec: active icon full colour; inactive - // `rgba(242,247,250,.46)` — .46 applied here as - // uniform opacity dims both the fallback Icon - // (already `onSurface` from - // PluriIconVariant.filled) and the real raster - // badge asset identically. - opacity: selected ? 1 : 0.46, - child: PluriIcon( - glyph: item.glyph, - variant: PluriIconVariant.filled, - // t4/4a spec: icon `font-size:25px`/`23px`. - size: selected ? 25 : 23, - color: selected ? t.electricMagenta : null, - // Same ARB string the outer Semantics already - // uses — passing it explicitly skips - // PluriIcon's own AppLocalizations.of lookup - // (excluded from the tree above regardless). - semanticLabel: item.label, - ), - ), - if (selected) ...[ - const SizedBox(height: 3), - Text( - item.label, - maxLines: 1, - overflow: TextOverflow.ellipsis, - // t4/4a spec: label `font-size:11px; - // font-weight:800; line-height:1.25`, brand - // colour. - style: Theme.of( - context, - ).textTheme.labelSmall?.copyWith( - fontSize: 11, - fontWeight: FontWeight.w800, - height: 1.25, - letterSpacing: 0, - color: t.electricMagenta, + // t4/4a spec: active item lift `translateY(-15px)`. Every + // property here used to change INSTANTLY while the balloon + // behind it slid with AnimatedPositioned — the balloon + // glided and its contents teleported, which read as a + // broken transition. All four now share the balloon's own + // duration and curve so the whole tab moves as one. + child: AnimatedSlide( + duration: motion.normal, + curve: Curves.easeOutCubic, + // Slide is expressed in fractions of the child's size; + // the icon column is ~40 tall, so -15px is about -0.375. + offset: Offset(0, selected ? -0.375 : 0), + child: AnimatedSize( + duration: motion.normal, + curve: Curves.easeOutCubic, + child: Column( + mainAxisSize: MainAxisSize.min, + children: [ + AnimatedOpacity( + duration: motion.normal, + curve: Curves.easeOutCubic, + // t4/4a spec: active icon full colour; inactive + // `rgba(242,247,250,.46)` — .46 applied here as + // uniform opacity dims both the fallback Icon + // (already `onSurface` from + // PluriIconVariant.filled) and the real raster + // badge asset identically. + opacity: selected ? 1 : 0.46, + child: TweenAnimationBuilder( + duration: motion.normal, + curve: Curves.easeOutCubic, + tween: Tween(end: selected ? 25 : 23), + builder: + (context, size, _) => PluriIcon( + glyph: item.glyph, + variant: PluriIconVariant.filled, + // t4/4a spec: `font-size:25px`/`23px`. + size: size, + color: selected ? t.electricMagenta : null, + // Same ARB string the outer Semantics + // already uses — passing it explicitly + // skips PluriIcon's own + // AppLocalizations.of lookup. + semanticLabel: item.label, + ), ), ), + if (selected) ...[ + const SizedBox(height: 3), + Text( + item.label, + maxLines: 1, + overflow: TextOverflow.ellipsis, + // t4/4a spec: label `font-size:11px; + // font-weight:800; line-height:1.25`, brand + // colour. + style: Theme.of( + context, + ).textTheme.labelSmall?.copyWith( + fontSize: 11, + fontWeight: FontWeight.w800, + height: 1.25, + letterSpacing: 0, + color: t.electricMagenta, + ), + ), + ], ], - ], + ), ), ), ), diff --git a/test/pantallas/pantalla_favoritos_test.dart b/test/pantallas/pantalla_favoritos_test.dart index 0d14502..623a606 100644 --- a/test/pantallas/pantalla_favoritos_test.dart +++ b/test/pantallas/pantalla_favoritos_test.dart @@ -472,6 +472,24 @@ void main() { expect(find.text('Move to list'), findsOneWidget); expect(find.text('Remove from favorites'), findsOneWidget); expect(find.byType(PopupMenuItem), findsNWidgets(2)); + + // Regression guard for a real user-reported bug: the button carried + // `constraints: BoxConstraints.tightFor(width: 38, height: 42)`, + // which sizes the POPUP MENU rather than the button. Every item was + // clipped to its first letter — users saw "M" and "E", not the + // labels. The three assertions above all PASSED throughout, because + // find.text matches a Text widget in the tree whether or not it is + // visually clipped. Only measuring the laid-out width catches it. + final anchoItem = tester.getSize( + find.byType(PopupMenuItem).first, + ); + expect( + anchoItem.width, + greaterThan(100), + reason: + 'a menu item narrower than its label means the popup is being ' + 'constrained and the text is clipped', + ); }, ); diff --git a/test/widgets/pluri_bottom_navigation_test.dart b/test/widgets/pluri_bottom_navigation_test.dart index 29aa31e..b5a171c 100644 --- a/test/widgets/pluri_bottom_navigation_test.dart +++ b/test/widgets/pluri_bottom_navigation_test.dart @@ -96,13 +96,17 @@ void main() { ); expect(inactiveIcon.color, isNull); - final dimmed = tester.widget( + // AnimatedOpacity, not a plain Opacity: the dim now transitions with the + // balloon instead of snapping. Users reported the bar's contents + // teleporting while the balloon slid — the lift, dim, icon size and + // label all animate together now. + final dimmed = tester.widget( find.ancestor( of: find.descendant( of: find.byKey(PluriBottomNavigation.itemKey(1)), matching: find.byType(PluriIcon), ), - matching: find.byType(Opacity), + matching: find.byType(AnimatedOpacity), ), ); expect(dimmed.opacity, closeTo(0.46, 0.001));