Skip to content

Commit 30e0640

Browse files
committed
revert(drink-detail): remove the My Festival note nudge
The prompt fought the mobile keyboard through several iterations and never felt right. Note-content search (kept) already solves the real problem — a note-only drink can be found again by searching its note text — so the nudge is removed entirely rather than patched further. Kept from the same line of work: inline autosaving notes with undo, the FAB hiding while the note field is focused (it overlapped the field regardless of the nudge), and note-content search. Refs #487
1 parent 3e2957c commit 30e0640

4 files changed

Lines changed: 44 additions & 316 deletions

File tree

lib/screens/drink_detail_screen.dart

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,6 @@ class _DrinkDetailScreenState extends State<DrinkDetailScreen>
236236
provider.setUserNotes(drink, notes),
237237
onEditingChanged: (editing) =>
238238
setState(() => _isEditingNote = editing),
239-
onLogTasting: () => unawaited(_logTasting(provider, drink)),
240239
),
241240
),
242241
// Your tasting log — the record of pours.

lib/widgets/your_take_card.dart

Lines changed: 42 additions & 132 deletions
Original file line numberDiff line numberDiff line change
@@ -32,17 +32,12 @@ class YourTakeCard extends StatefulWidget {
3232
/// that would otherwise sit over the field once the keyboard is up.
3333
final ValueChanged<bool>? onEditingChanged;
3434

35-
/// Log a pour (the screen's "Drunk it!" action). When provided, the
36-
/// note-only nudge offers it alongside want-to-try.
37-
final VoidCallback? onLogTasting;
38-
3935
const YourTakeCard({
4036
required this.drink,
4137
required this.onWantToTryTap,
4238
required this.onRatingChanged,
4339
required this.onNotesChanged,
4440
this.onEditingChanged,
45-
this.onLogTasting,
4641
super.key,
4742
});
4843

@@ -287,78 +282,55 @@ class _YourTakeCardState extends State<YourTakeCard> {
287282
final notes = _lastSavedNotes;
288283
final hasNotes = notes != null && notes.isNotEmpty;
289284

290-
// A note alone doesn't say whether this is a tip ("Dave said try it") or
291-
// a memory ("loved it"), so it can't place the drink in My Festival.
292-
// Rather than guess, prompt for the explicit signal while neither exists.
293-
//
294-
// NEVER while editing: with the software keyboard up, any tap outside
295-
// the field blurs it mid-gesture and reflows the page — buttons in that
296-
// context misfire no matter where they sit (verified on the PR preview,
297-
// twice). The prompt renders only in display mode, in the slot where the
298-
// field just was, so it greets the user the instant editing ends — the
299-
// first moment a tap is actually safe. Without a log callback there
300-
// would be no action beside the question (the want-to-try pill lives in
301-
// the card header), so it also requires one.
302-
final showNudge =
303-
!_isEditing &&
304-
hasNotes &&
305-
widget.onLogTasting != null &&
306-
!widget.drink.isFavorite &&
307-
widget.drink.tastingCount == 0;
308-
309285
return Container(
310286
padding: const EdgeInsets.only(top: 12),
311287
decoration: BoxDecoration(
312288
border: Border(
313289
top: BorderSide(color: theme.colorScheme.outlineVariant),
314290
),
315291
),
316-
child: Column(
317-
crossAxisAlignment: CrossAxisAlignment.start,
318-
children: [
319-
if (showNudge)
320-
_buildMyFestivalNudge(
321-
theme,
322-
padding: const EdgeInsets.only(bottom: 10),
323-
),
324-
if (_isEditing) ...[
325-
TextField(
326-
key: const ValueKey('user-notes-field'),
327-
controller: _notesController,
328-
focusNode: _notesFocusNode,
329-
autofocus: true,
330-
minLines: 1,
331-
maxLines: null,
332-
textCapitalization: TextCapitalization.sentences,
333-
decoration: const InputDecoration(
334-
hintText: 'What did you think?',
335-
border: InputBorder.none,
336-
isDense: true,
337-
contentPadding: EdgeInsets.zero,
338-
),
339-
onChanged: _onFieldChanged,
340-
),
341-
SizedBox(
342-
height: 16,
343-
// Only mount the live region while it has something to say —
344-
// the same conditional pattern as the refresh indicator in
345-
// drinks_screen.dart. A persistent node with an empty label
346-
// would sit in the semantics tree saying nothing.
347-
child: _showSaved
348-
? Semantics(
349-
liveRegion: true,
350-
label: 'Saved',
351-
child: Text(
352-
'Saved',
353-
style: theme.textTheme.labelSmall?.copyWith(
354-
color: theme.colorScheme.primary,
355-
),
356-
),
357-
)
358-
: const SizedBox.shrink(),
359-
),
360-
] else
361-
Semantics(
292+
child: _isEditing
293+
? Column(
294+
crossAxisAlignment: CrossAxisAlignment.start,
295+
children: [
296+
TextField(
297+
key: const ValueKey('user-notes-field'),
298+
controller: _notesController,
299+
focusNode: _notesFocusNode,
300+
autofocus: true,
301+
minLines: 1,
302+
maxLines: null,
303+
textCapitalization: TextCapitalization.sentences,
304+
decoration: const InputDecoration(
305+
hintText: 'What did you think?',
306+
border: InputBorder.none,
307+
isDense: true,
308+
contentPadding: EdgeInsets.zero,
309+
),
310+
onChanged: _onFieldChanged,
311+
),
312+
SizedBox(
313+
height: 16,
314+
// Only mount the live region while it has something to say —
315+
// the same conditional pattern as the refresh indicator in
316+
// drinks_screen.dart. A persistent node with an empty label
317+
// would sit in the semantics tree saying nothing.
318+
child: _showSaved
319+
? Semantics(
320+
liveRegion: true,
321+
label: 'Saved',
322+
child: Text(
323+
'Saved',
324+
style: theme.textTheme.labelSmall?.copyWith(
325+
color: theme.colorScheme.primary,
326+
),
327+
),
328+
)
329+
: const SizedBox.shrink(),
330+
),
331+
],
332+
)
333+
: Semantics(
362334
label: hasNotes
363335
? 'Edit your notes for ${widget.drink.name}'
364336
: 'Add your notes for ${widget.drink.name}',
@@ -394,68 +366,6 @@ class _YourTakeCardState extends State<YourTakeCard> {
394366
),
395367
),
396368
),
397-
],
398-
),
399-
);
400-
}
401-
402-
/// The classification prompt for a note-only drink: a "Drunk it!" button in
403-
/// the FAB's filled style. Want-to-try is deliberately NOT repeated here —
404-
/// the card header's pill is the app's one control for that signal and is
405-
/// visible a line or two above; duplicating it read as clutter.
406-
Widget _buildMyFestivalNudge(ThemeData theme, {required EdgeInsets padding}) {
407-
return Padding(
408-
padding: padding,
409-
child: Wrap(
410-
spacing: 8,
411-
runSpacing: 8,
412-
crossAxisAlignment: WrapCrossAlignment.center,
413-
children: [
414-
Text(
415-
'Show it in My Festival?',
416-
style: theme.textTheme.bodySmall?.copyWith(
417-
color: theme.colorScheme.onSurfaceVariant,
418-
),
419-
),
420-
Semantics(
421-
label: 'Log a tasting of ${widget.drink.name}',
422-
button: true,
423-
child: InkWell(
424-
key: const ValueKey('nudge-drunk-it'),
425-
onTap: widget.onLogTasting,
426-
borderRadius: BorderRadius.circular(16),
427-
child: Container(
428-
padding: const EdgeInsets.symmetric(
429-
horizontal: 12,
430-
vertical: 6,
431-
),
432-
decoration: BoxDecoration(
433-
borderRadius: BorderRadius.circular(16),
434-
color: theme.colorScheme.primaryContainer,
435-
),
436-
child: Row(
437-
mainAxisSize: MainAxisSize.min,
438-
children: [
439-
Icon(
440-
Icons.add_circle_outline,
441-
size: 16,
442-
color: theme.colorScheme.onPrimaryContainer,
443-
),
444-
const SizedBox(width: 6),
445-
Text(
446-
'Drunk it!',
447-
style: theme.textTheme.labelMedium?.copyWith(
448-
color: theme.colorScheme.onPrimaryContainer,
449-
fontWeight: FontWeight.w600,
450-
),
451-
),
452-
],
453-
),
454-
),
455-
),
456-
),
457-
],
458-
),
459369
);
460370
}
461371
}

test/drink_detail_screen_test.dart

Lines changed: 0 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -971,52 +971,6 @@ void main() {
971971
expect(find.byKey(const ValueKey('tasted-action')), findsOneWidget);
972972
});
973973

974-
testWidgets('note-only drink shows the My Festival nudge and logging '
975-
'from it records a pour', (WidgetTester tester) async {
976-
await useTallSurface(tester);
977-
when(mockDrinkRepository.getDrinks(any)).thenAnswer(
978-
(_) async => [
979-
drink.copyWith(
980-
userState: UserDrinkState(
981-
notes: 'Dave said try this',
982-
createdAt: now,
983-
updatedAt: now,
984-
),
985-
),
986-
],
987-
);
988-
await provider.loadDrinks();
989-
990-
when(
991-
mockDrinkRepository.addTasting(any, any, now: anyNamed('now')),
992-
).thenAnswer(
993-
(_) async => UserDrinkState(
994-
notes: 'Dave said try this',
995-
tastingEvents: [now],
996-
createdAt: now,
997-
updatedAt: now,
998-
),
999-
);
1000-
1001-
await tester.pumpWidget(createTestWidget('drink1'));
1002-
await tester.pumpAndSettle();
1003-
1004-
// A note alone can't place the drink in My Festival, so the card
1005-
// prompts for an explicit signal.
1006-
expect(find.text('Show it in My Festival?'), findsOneWidget);
1007-
1008-
await tester.tap(find.byKey(const ValueKey('nudge-drunk-it')));
1009-
await tester.pumpAndSettle();
1010-
1011-
verify(
1012-
mockDrinkRepository.addTasting(any, any, now: anyNamed('now')),
1013-
).called(1);
1014-
expect(provider.getDrinkById('drink1')!.tastingCount, 1);
1015-
expect(find.text('Logged your first tasting'), findsOneWidget);
1016-
// The signal now exists, so the nudge retires.
1017-
expect(find.text('Show it in My Festival?'), findsNothing);
1018-
});
1019-
1020974
testWidgets('existing user notes are shown and prefilled for editing', (
1021975
WidgetTester tester,
1022976
) async {

0 commit comments

Comments
 (0)