Skip to content

Fix popup UI; Added scroll pickers #17

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
gonfff wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix popup UI; Added scroll pickers #17

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 0 additions & 21 deletions ios/Runner/Info.plist
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,27 +32,6 @@
<dict>
<key>NSAllowsArbitraryLoads</key>
<false/>
<key>NSExceptionDomains</key>
<dict>
<key>fc.yahoo.com</key>
<dict>
<key>NSExceptionAllowsInsecureHTTPLoads</key>
<false/>
<key>NSExceptionRequiresForwardSecrecy</key>
<false/>
<key>NSIncludesSubdomains</key>
<true/>
</dict>
<key>query1.finance.yahoo.com</key>
<dict>
<key>NSExceptionAllowsInsecureHTTPLoads</key>
<false/>
<key>NSExceptionRequiresForwardSecrecy</key>
<false/>
<key>NSIncludesSubdomains</key>
<true/>
</dict>
</dict>
</dict>
<key>NSUserNotificationUsageDescription</key>
<string>We use notifications to remind you about upcoming subscription payments.</string>
Expand Down
236 changes: 172 additions & 64 deletions lib/presentation/widgets/add_subscription_sheet.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@ import 'package:subctrl/domain/entities/tag.dart';
import 'package:subctrl/presentation/formatters/date_formatter.dart';
import 'package:subctrl/presentation/l10n/app_localizations.dart';
import 'package:subctrl/presentation/mappers/billing_cycle_labels.dart';
import 'package:subctrl/presentation/widgets/currency_picker.dart';
import 'package:subctrl/presentation/widgets/tag_picker.dart';
import 'package:subctrl/presentation/utils/color_utils.dart';

class AddSubscriptionSheet extends StatefulWidget {
const AddSubscriptionSheet({
Expand Down Expand Up @@ -120,42 +119,99 @@ class _AddSubscriptionSheetState extends State<AddSubscriptionSheet> {
}

Future<void> _pickCurrency(FormFieldState<String> state) async {
final selected = await showCurrencyPicker(
if (widget.currencies.isEmpty) return;
final localizations = AppLocalizations.of(context);
var tempIndex = widget.currencies.indexWhere(
(currency) => currency.code.toUpperCase() == _currencyCode.toUpperCase(),
);
if (tempIndex < 0) tempIndex = 0;
final controller = FixedExtentScrollController(initialItem: tempIndex);
Copy link

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The FixedExtentScrollController is created but never disposed. This could lead to a memory leak. Consider disposing the controller after the modal is dismissed, for example by calling controller.dispose() after the await statement.

Copilot uses AI. Check for mistakes.

await showCupertinoModalPopup<void>(
context: context,
currencies: widget.currencies,
selectedCode: _currencyCode,
builder: (context) {
return CupertinoActionSheet(
title: Text(localizations.currencyLabel),
message: SizedBox(
height: 200,
child: CupertinoPicker(
itemExtent: 32,
scrollController: controller,
onSelectedItemChanged: (index) => tempIndex = index,
children: widget.currencies
.map(
(currency) => Center(
child: Row(
mainAxisSize: MainAxisSize.min,
children: [
if ((currency.symbol ?? '').trim().isNotEmpty)
Padding(
padding: const EdgeInsets.only(right: 8),
child: Text(currency.symbol!.trim()),
),
Text(currency.code.toUpperCase()),
],
),
),
)
.toList(growable: false),
),
),
cancelButton: CupertinoActionSheetAction(
onPressed: () => Navigator.of(context).pop(),
child: Text(localizations.done),
),
);
},
);
if (selected != null) {
setState(() => _currencyCode = selected.toUpperCase());
state.didChange(_currencyCode);
}

if (!mounted) return;
final selected = widget.currencies[tempIndex];
setState(() => _currencyCode = selected.code.toUpperCase());
state.didChange(_currencyCode);
Comment on lines +168 to +171
Copy link

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The picker always applies the selected value even when the user presses the cancel button. The old implementation only updated the value when a selection was explicitly confirmed. This changes the user experience - now scrolling in the picker and then pressing cancel will still apply the scrolled-to value. Consider tracking whether the user confirmed or cancelled the selection, and only applying the change on confirmation.

Copilot uses AI. Check for mistakes.
}

Future<void> _pickCycle(FormFieldState<BillingCycle> state) async {
final localizations = AppLocalizations.of(context);
final cycle = await showCupertinoModalPopup<BillingCycle>(
final initialIndex = _orderedCycles.indexOf(_cycle);
var tempIndex = initialIndex < 0 ? 0 : initialIndex;
final controller = FixedExtentScrollController(initialItem: tempIndex);
Copy link

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The FixedExtentScrollController is created but never disposed. This could lead to a memory leak. Consider disposing the controller after the modal is dismissed, for example by calling controller.dispose() after the await statement.

Copilot uses AI. Check for mistakes.
await showCupertinoModalPopup<void>(
context: context,
builder: (context) {
return CupertinoActionSheet(
title: Text(localizations.periodLabel),
actions: [
for (final option in _orderedCycles)
CupertinoActionSheetAction(
onPressed: () => Navigator.of(context).pop(option),
child: Text(billingCycleLongLabel(option, localizations)),
),
],
message: SizedBox(
height: 200,
child: CupertinoPicker(
itemExtent: 40,
scrollController: controller,
onSelectedItemChanged: (index) => tempIndex = index,
children: _orderedCycles
.map(
(option) => Center(
child: Text(
billingCycleLongLabel(option, localizations),
style: CupertinoTheme.of(
context,
).textTheme.textStyle.copyWith(fontSize: 19),
),
),
)
.toList(growable: false),
),
),
cancelButton: CupertinoActionSheetAction(
onPressed: () => Navigator.of(context).pop(),
child: Text(localizations.settingsClose),
child: Text(localizations.done),
),
);
},
);
if (cycle != null) {
setState(() => _cycle = cycle);
state.didChange(cycle);
}
if (!mounted) return;
final selected = _orderedCycles[tempIndex];
setState(() => _cycle = selected);
state.didChange(selected);
Comment on lines +211 to +214
Copy link

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The picker always applies the selected value even when the user presses the cancel button. The old implementation only updated the value when a selection was explicitly confirmed. This changes the user experience - now scrolling in the picker and then pressing cancel will still apply the scrolled-to value. Consider tracking whether the user confirmed or cancelled the selection, and only applying the change on confirmation.

Copilot uses AI. Check for mistakes.
}

Future<void> _pickPurchaseDate(FormFieldState<DateTime?> state) async {
Expand All @@ -164,37 +220,21 @@ class _AddSubscriptionSheetState extends State<AddSubscriptionSheet> {
context: context,
builder: (context) {
final localizations = AppLocalizations.of(context);
final background = CupertinoColors.systemBackground.resolveFrom(
context,
);
return Container(
color: background,
height: 320,
child: Column(
children: [
SizedBox(
height: 44,
child: Row(
mainAxisAlignment: MainAxisAlignment.end,
children: [
CupertinoButton(
padding: const EdgeInsets.symmetric(horizontal: 16),
onPressed: () => Navigator.of(context).pop(),
child: Text(localizations.done),
),
],
),
),
Expanded(
child: CupertinoDatePicker(
mode: CupertinoDatePickerMode.date,
initialDateTime: tempDate,
minimumDate: DateTime(DateTime.now().year - 10),
maximumDate: DateTime(DateTime.now().year + 5),
onDateTimeChanged: (value) => tempDate = value,
),
),
],
return CupertinoActionSheet(
title: Text(localizations.purchaseDateLabel),
message: SizedBox(
height: 200,
child: CupertinoDatePicker(
mode: CupertinoDatePickerMode.date,
initialDateTime: tempDate,
minimumDate: DateTime(DateTime.now().year - 10),
maximumDate: DateTime(DateTime.now().year + 5),
onDateTimeChanged: (value) => tempDate = value,
),
),
cancelButton: CupertinoActionSheetAction(
onPressed: () => Navigator.of(context).pop(),
child: Text(localizations.done),
Comment on lines +235 to +237
Copy link

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The date picker always applies the selected value even when the user presses the cancel button. The old implementation had a "Done" button to confirm the selection. This changes the user experience - now scrolling the date picker and then pressing cancel will still apply the scrolled-to date. Consider tracking whether the user confirmed or cancelled the selection, and only applying the change on confirmation.

Copilot uses AI. Check for mistakes.
),
);
},
Expand All @@ -206,20 +246,73 @@ class _AddSubscriptionSheetState extends State<AddSubscriptionSheet> {

Future<void> _pickTag(FormFieldState<int?> state) async {
if (widget.tags.isEmpty) return;
final result = await showTagPicker(
final localizations = AppLocalizations.of(context);
final options = [
_TagOption.none(localizations.subscriptionTagNone),
...widget.tags.map((tag) => _TagOption.tag(tag)),
];
var initialIndex = options.indexWhere(
(option) => option.matches(_selectedTagId),
);
if (initialIndex < 0) initialIndex = 0;
var tempIndex = initialIndex;
final controller = FixedExtentScrollController(initialItem: tempIndex);
Copy link

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The FixedExtentScrollController is created but never disposed. This could lead to a memory leak. Consider disposing the controller after the modal is dismissed, for example by calling controller.dispose() after the await statement.

Copilot uses AI. Check for mistakes.

await showCupertinoModalPopup<void>(
context: context,
tags: widget.tags,
selectedTagId: _selectedTagId,
builder: (context) {
return CupertinoActionSheet(
title: Text(localizations.subscriptionTagLabel),
message: SizedBox(
height: 200,
child: CupertinoPicker(
itemExtent: 40,
scrollController: controller,
onSelectedItemChanged: (index) => tempIndex = index,
children: options
.map(
(option) => Center(
child: Row(
mainAxisSize: MainAxisSize.min,
children: [
if (option.colorHex != null)
Container(
width: 12,
height: 12,
decoration: BoxDecoration(
shape: BoxShape.circle,
color: colorFromHex(
option.colorHex!,
fallbackColor: const Color(0xFF000000),
),
),
),
if (option.colorHex != null) const SizedBox(width: 8),
Text(
option.label,
style: CupertinoTheme.of(
context,
).textTheme.textStyle.copyWith(fontSize: 19),
),
],
),
),
)
.toList(growable: false),
),
),
cancelButton: CupertinoActionSheetAction(
onPressed: () => Navigator.of(context).pop(),
child: Text(localizations.done),
),
);
},
);
if (result == null) return;

if (!mounted) return;
if (result == -1) {
setState(() => _selectedTagId = null);
state.didChange(null);
} else {
setState(() => _selectedTagId = result);
state.didChange(result);
}
final selected = options[tempIndex];
setState(() => _selectedTagId = selected.tagId);
state.didChange(selected.tagId);
Comment on lines 215 to +315
Copy link

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The picker always applies the selected value even when the user presses the cancel button. The old implementation (using showTagPicker) only updated the value when a selection was explicitly confirmed. This changes the user experience - now scrolling in the picker and then pressing cancel will still apply the scrolled-to value. Consider tracking whether the user confirmed or cancelled the selection, and only applying the change on confirmation.

Copilot uses AI. Check for mistakes.
}

void _handleSubmit() {
Expand Down Expand Up @@ -657,3 +750,18 @@ class _AddSubscriptionSheetState extends State<AddSubscriptionSheet> {
);
}
}

class _TagOption {
const _TagOption._(this.tagId, this.label, this.colorHex);

factory _TagOption.none(String label) => _TagOption._(null, label, null);

factory _TagOption.tag(Tag tag) =>
_TagOption._(tag.id, tag.name, tag.colorHex);

final int? tagId;
final String label;
final String? colorHex;

bool matches(int? selectedTagId) => tagId == selectedTagId;
}
Loading
Loading