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.
This commit is contained in:
@@ -465,7 +465,13 @@ class _FilaFavorito extends StatelessWidget {
|
|||||||
context,
|
context,
|
||||||
).colorScheme.onSurface.withValues(alpha: 0.45),
|
).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) {
|
onSelected: (accion) {
|
||||||
if (accion == 'assign') _asignar(context);
|
if (accion == 'assign') _asignar(context);
|
||||||
if (accion == 'remove') _eliminar(context);
|
if (accion == 'remove') _eliminar(context);
|
||||||
|
|||||||
@@ -192,7 +192,10 @@ class PluriBottomNavigation extends StatelessWidget {
|
|||||||
/// the balloon's opaque fill already covers the seam where the bar's own
|
/// the balloon's opaque fill already covers the seam where the bar's own
|
||||||
/// shadow would otherwise show through.
|
/// shadow would otherwise show through.
|
||||||
List<BoxShadow> get _shellShadows => [
|
List<BoxShadow> 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(
|
BoxShadow(
|
||||||
color: Colors.black.withValues(alpha: 0.5),
|
color: Colors.black.withValues(alpha: 0.5),
|
||||||
offset: const Offset(0, 14),
|
offset: const Offset(0, 14),
|
||||||
@@ -214,6 +217,7 @@ class _PluriNavButton extends StatelessWidget {
|
|||||||
@override
|
@override
|
||||||
Widget build(BuildContext context) {
|
Widget build(BuildContext context) {
|
||||||
final t = context.pluriTokens;
|
final t = context.pluriTokens;
|
||||||
|
final motion = context.pluriMotion;
|
||||||
return Semantics(
|
return Semantics(
|
||||||
button: true,
|
button: true,
|
||||||
selected: selected,
|
selected: selected,
|
||||||
@@ -222,6 +226,11 @@ class _PluriNavButton extends StatelessWidget {
|
|||||||
type: MaterialType.transparency,
|
type: MaterialType.transparency,
|
||||||
child: InkWell(
|
child: InkWell(
|
||||||
onTap: onTap,
|
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(
|
child: Align(
|
||||||
alignment: Alignment.bottomCenter,
|
alignment: Alignment.bottomCenter,
|
||||||
// t4/4a spec: the items row itself is 52px tall — this inner
|
// 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
|
// Flutter joins merged labels with `\n`, so screen readers
|
||||||
// would announce "Alarmas\nAlarmas" instead of "Alarmas".
|
// would announce "Alarmas\nAlarmas" instead of "Alarmas".
|
||||||
child: ExcludeSemantics(
|
child: ExcludeSemantics(
|
||||||
child: Transform.translate(
|
// t4/4a spec: active item lift `translateY(-15px)`. Every
|
||||||
// t4/4a spec: active item lift `translateY(-15px)`.
|
// property here used to change INSTANTLY while the balloon
|
||||||
offset: Offset(0, selected ? -15 : 0),
|
// behind it slid with AnimatedPositioned — the balloon
|
||||||
child: Column(
|
// glided and its contents teleported, which read as a
|
||||||
mainAxisSize: MainAxisSize.min,
|
// broken transition. All four now share the balloon's own
|
||||||
children: [
|
// duration and curve so the whole tab moves as one.
|
||||||
Opacity(
|
child: AnimatedSlide(
|
||||||
// t4/4a spec: active icon full colour; inactive
|
duration: motion.normal,
|
||||||
// `rgba(242,247,250,.46)` — .46 applied here as
|
curve: Curves.easeOutCubic,
|
||||||
// uniform opacity dims both the fallback Icon
|
// Slide is expressed in fractions of the child's size;
|
||||||
// (already `onSurface` from
|
// the icon column is ~40 tall, so -15px is about -0.375.
|
||||||
// PluriIconVariant.filled) and the real raster
|
offset: Offset(0, selected ? -0.375 : 0),
|
||||||
// badge asset identically.
|
child: AnimatedSize(
|
||||||
opacity: selected ? 1 : 0.46,
|
duration: motion.normal,
|
||||||
child: PluriIcon(
|
curve: Curves.easeOutCubic,
|
||||||
glyph: item.glyph,
|
child: Column(
|
||||||
variant: PluriIconVariant.filled,
|
mainAxisSize: MainAxisSize.min,
|
||||||
// t4/4a spec: icon `font-size:25px`/`23px`.
|
children: [
|
||||||
size: selected ? 25 : 23,
|
AnimatedOpacity(
|
||||||
color: selected ? t.electricMagenta : null,
|
duration: motion.normal,
|
||||||
// Same ARB string the outer Semantics already
|
curve: Curves.easeOutCubic,
|
||||||
// uses — passing it explicitly skips
|
// t4/4a spec: active icon full colour; inactive
|
||||||
// PluriIcon's own AppLocalizations.of lookup
|
// `rgba(242,247,250,.46)` — .46 applied here as
|
||||||
// (excluded from the tree above regardless).
|
// uniform opacity dims both the fallback Icon
|
||||||
semanticLabel: item.label,
|
// (already `onSurface` from
|
||||||
),
|
// PluriIconVariant.filled) and the real raster
|
||||||
),
|
// badge asset identically.
|
||||||
if (selected) ...[
|
opacity: selected ? 1 : 0.46,
|
||||||
const SizedBox(height: 3),
|
child: TweenAnimationBuilder<double>(
|
||||||
Text(
|
duration: motion.normal,
|
||||||
item.label,
|
curve: Curves.easeOutCubic,
|
||||||
maxLines: 1,
|
tween: Tween<double>(end: selected ? 25 : 23),
|
||||||
overflow: TextOverflow.ellipsis,
|
builder:
|
||||||
// t4/4a spec: label `font-size:11px;
|
(context, size, _) => PluriIcon(
|
||||||
// font-weight:800; line-height:1.25`, brand
|
glyph: item.glyph,
|
||||||
// colour.
|
variant: PluriIconVariant.filled,
|
||||||
style: Theme.of(
|
// t4/4a spec: `font-size:25px`/`23px`.
|
||||||
context,
|
size: size,
|
||||||
).textTheme.labelSmall?.copyWith(
|
color: selected ? t.electricMagenta : null,
|
||||||
fontSize: 11,
|
// Same ARB string the outer Semantics
|
||||||
fontWeight: FontWeight.w800,
|
// already uses — passing it explicitly
|
||||||
height: 1.25,
|
// skips PluriIcon's own
|
||||||
letterSpacing: 0,
|
// AppLocalizations.of lookup.
|
||||||
color: t.electricMagenta,
|
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,
|
||||||
|
),
|
||||||
|
),
|
||||||
|
],
|
||||||
],
|
],
|
||||||
],
|
),
|
||||||
),
|
),
|
||||||
),
|
),
|
||||||
),
|
),
|
||||||
|
|||||||
@@ -472,6 +472,24 @@ void main() {
|
|||||||
expect(find.text('Move to list'), findsOneWidget);
|
expect(find.text('Move to list'), findsOneWidget);
|
||||||
expect(find.text('Remove from favorites'), findsOneWidget);
|
expect(find.text('Remove from favorites'), findsOneWidget);
|
||||||
expect(find.byType(PopupMenuItem<String>), findsNWidgets(2));
|
expect(find.byType(PopupMenuItem<String>), 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<String>).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',
|
||||||
|
);
|
||||||
},
|
},
|
||||||
);
|
);
|
||||||
|
|
||||||
|
|||||||
@@ -96,13 +96,17 @@ void main() {
|
|||||||
);
|
);
|
||||||
expect(inactiveIcon.color, isNull);
|
expect(inactiveIcon.color, isNull);
|
||||||
|
|
||||||
final dimmed = tester.widget<Opacity>(
|
// 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<AnimatedOpacity>(
|
||||||
find.ancestor(
|
find.ancestor(
|
||||||
of: find.descendant(
|
of: find.descendant(
|
||||||
of: find.byKey(PluriBottomNavigation.itemKey(1)),
|
of: find.byKey(PluriBottomNavigation.itemKey(1)),
|
||||||
matching: find.byType(PluriIcon),
|
matching: find.byType(PluriIcon),
|
||||||
),
|
),
|
||||||
matching: find.byType(Opacity),
|
matching: find.byType(AnimatedOpacity),
|
||||||
),
|
),
|
||||||
);
|
);
|
||||||
expect(dimmed.opacity, closeTo(0.46, 0.001));
|
expect(dimmed.opacity, closeTo(0.46, 0.001));
|
||||||
|
|||||||
Reference in New Issue
Block a user