From d4102c0489ba6553448dd74bd0958b0f3a508c2f Mon Sep 17 00:00:00 2001 From: shenlong <139912620+shenlong-tanwen@users.noreply.github.com> Date: Tue, 23 Jun 2026 01:54:52 +0530 Subject: [PATCH] refactor: ui icon buttons implicit loading (#29263) * refactor: icon buttons implicit loading * chore: cleanup * feat: ui color override (#29264) Co-authored-by: shenlong-tanwen <139912620+shalong-tanwen@users.noreply.github.com> --------- Co-authored-by: shenlong-tanwen <139912620+shalong-tanwen@users.noreply.github.com> --- .../packages/ui/lib/src/color_override.dart | 13 +++ .../ui/lib/src/components/close_button.dart | 11 +-- .../packages/ui/lib/src/components/form.dart | 2 +- .../ui/lib/src/components/formatted_text.dart | 25 ++--- .../ui/lib/src/components/icon_button.dart | 86 ++++++++++------ .../ui/lib/src/components/password_input.dart | 1 - .../ui/lib/src/components/text_button.dart | 97 ++++++++----------- mobile/packages/ui/lib/src/internal.dart | 5 + .../ui/lib/src/previews/text_button.dart | 43 +------- .../packages/ui/test/color_override_test.dart | 53 ++++++++++ 10 files changed, 189 insertions(+), 147 deletions(-) create mode 100644 mobile/packages/ui/lib/src/color_override.dart create mode 100644 mobile/packages/ui/test/color_override_test.dart diff --git a/mobile/packages/ui/lib/src/color_override.dart b/mobile/packages/ui/lib/src/color_override.dart new file mode 100644 index 0000000000..c0b8358bb5 --- /dev/null +++ b/mobile/packages/ui/lib/src/color_override.dart @@ -0,0 +1,13 @@ +import 'package:flutter/widgets.dart'; + +class ImmichColorOverride extends InheritedWidget { + const ImmichColorOverride({super.key, required this.color, required super.child}); + + final Color color; + + static Color? maybeOf(BuildContext context) => + context.dependOnInheritedWidgetOfExactType()?.color; + + @override + bool updateShouldNotify(ImmichColorOverride oldWidget) => color != oldWidget.color; +} diff --git a/mobile/packages/ui/lib/src/components/close_button.dart b/mobile/packages/ui/lib/src/components/close_button.dart index d23c309c91..031f3992aa 100644 --- a/mobile/packages/ui/lib/src/components/close_button.dart +++ b/mobile/packages/ui/lib/src/components/close_button.dart @@ -16,10 +16,9 @@ class ImmichCloseButton extends StatelessWidget { @override Widget build(BuildContext context) => ImmichIconButton( - key: key, - icon: Icons.close, - color: color, - variant: variant, - onPressed: onPressed ?? () => Navigator.of(context).pop(), - ); + icon: Icons.close, + color: color, + variant: variant, + onPressed: onPressed ?? () => Navigator.of(context).pop(), + ); } diff --git a/mobile/packages/ui/lib/src/components/form.dart b/mobile/packages/ui/lib/src/components/form.dart index 31e9d01980..4b6e3af085 100644 --- a/mobile/packages/ui/lib/src/components/form.dart +++ b/mobile/packages/ui/lib/src/components/form.dart @@ -88,7 +88,7 @@ class _ImmichFormState extends State { builder: (context, _) => ImmichTextButton( labelText: submitText, icon: widget.submitIcon, - variant: ImmichVariant.filled, + variant: .filled, loading: _controller.isLoading, onPressed: _controller.submit, disabled: _controller.onSubmit == null, diff --git a/mobile/packages/ui/lib/src/components/formatted_text.dart b/mobile/packages/ui/lib/src/components/formatted_text.dart index 95e42d834d..1cb75c8e0a 100644 --- a/mobile/packages/ui/lib/src/components/formatted_text.dart +++ b/mobile/packages/ui/lib/src/components/formatted_text.dart @@ -94,12 +94,12 @@ class _ImmichFormattedTextState extends State { final tag = match.group(1)!.toLowerCase(); final content = match.group(2)!; - final formattedSpan = (widget.spanBuilder ?? _defaultSpanBuilder)(tag); - final style = formattedSpan.style ?? _defaultTextStyle(tag); + final span = widget.spanBuilder?.call(tag); + final style = span?.style ?? _defaultTextStyle(tag); GestureRecognizer? recognizer; - if (formattedSpan.onTap != null) { - recognizer = TapGestureRecognizer()..onTap = formattedSpan.onTap; + if (span?.onTap != null) { + recognizer = TapGestureRecognizer()..onTap = span!.onTap; _recognizers.add(recognizer); } spans.add(TextSpan(text: content, style: style, recognizer: recognizer)); @@ -114,19 +114,12 @@ class _ImmichFormattedTextState extends State { return spans; } - FormattedSpan _defaultSpanBuilder(String tag) => switch (tag) { - 'b' => const FormattedSpan(style: TextStyle(fontWeight: FontWeight.bold)), - 'link' => const FormattedSpan(style: TextStyle(decoration: TextDecoration.underline)), - _ when tag.endsWith('-link') => const FormattedSpan(style: TextStyle(decoration: TextDecoration.underline)), - _ => const FormattedSpan(), - }; - TextStyle? _defaultTextStyle(String tag) => switch (tag) { - 'b' => const TextStyle(fontWeight: FontWeight.bold), - 'link' => const TextStyle(decoration: TextDecoration.underline), - _ when tag.endsWith('-link') => const TextStyle(decoration: TextDecoration.underline), - _ => null, - }; + 'b' => const TextStyle(fontWeight: FontWeight.bold), + 'link' => const TextStyle(decoration: TextDecoration.underline), + _ when tag.endsWith('-link') => const TextStyle(decoration: TextDecoration.underline), + _ => null, + }; @override Widget build(BuildContext context) { diff --git a/mobile/packages/ui/lib/src/components/icon_button.dart b/mobile/packages/ui/lib/src/components/icon_button.dart index dc140b71f9..b9bdd3a406 100644 --- a/mobile/packages/ui/lib/src/components/icon_button.dart +++ b/mobile/packages/ui/lib/src/components/icon_button.dart @@ -1,54 +1,80 @@ -import 'package:flutter/material.dart'; -import 'package:immich_ui/src/types.dart'; +import 'dart:async'; -class ImmichIconButton extends StatelessWidget { +import 'package:flutter/material.dart'; +import 'package:immich_ui/immich_ui.dart'; +import 'package:immich_ui/src/internal.dart'; + +class ImmichIconButton extends StatefulWidget { final IconData icon; - final VoidCallback onPressed; + final FutureOr Function() onPressed; final ImmichVariant variant; final ImmichColor color; final bool disabled; + final bool? loading; const ImmichIconButton({ super.key, required this.icon, required this.onPressed, - this.color = ImmichColor.primary, - this.variant = ImmichVariant.filled, + this.color = .primary, + this.variant = .filled, this.disabled = false, + this.loading, }); + @override + State createState() => _ImmichIconButtonState(); +} + +class _ImmichIconButtonState extends State { + bool _loading = false; + bool get _isLoading => widget.loading ?? _loading; + + Future _onPressed() async { + setState(() => _loading = true); + try { + await widget.onPressed(); + } finally { + if (mounted) { + setState(() => _loading = false); + } + } + } + @override Widget build(BuildContext context) { final colorScheme = Theme.of(context).colorScheme; - final background = switch (variant) { - ImmichVariant.filled => switch (color) { - ImmichColor.primary => colorScheme.primary, - ImmichColor.secondary => colorScheme.secondary, - }, - ImmichVariant.ghost => Colors.transparent, + final background = switch (widget.variant) { + .filled => switch (widget.color) { + .primary => colorScheme.primary, + .secondary => colorScheme.secondary, + }, + .ghost => Colors.transparent, }; - final foreground = switch (variant) { - ImmichVariant.filled => switch (color) { - ImmichColor.primary => colorScheme.onPrimary, - ImmichColor.secondary => colorScheme.onSecondary, - }, - ImmichVariant.ghost => switch (color) { - ImmichColor.primary => colorScheme.primary, - ImmichColor.secondary => colorScheme.secondary, - }, - }; - - final effectiveOnPressed = disabled ? null : onPressed; + final foreground = + context.colorOverride ?? + switch (widget.variant) { + .filled => switch (widget.color) { + .primary => colorScheme.onPrimary, + .secondary => colorScheme.onSecondary, + }, + .ghost => switch (widget.color) { + .primary => colorScheme.primary, + .secondary => colorScheme.secondary, + }, + }; return IconButton( - icon: Icon(icon), - onPressed: effectiveOnPressed, - style: IconButton.styleFrom( - backgroundColor: background, - foregroundColor: foreground, - ), + icon: _isLoading + ? const SizedBox.square( + dimension: ImmichIconSize.sm, + child: CircularProgressIndicator(strokeWidth: ImmichBorderWidth.md), + ) + : Icon(widget.icon), + onPressed: widget.disabled || _isLoading ? null : _onPressed, + style: IconButton.styleFrom(backgroundColor: background, foregroundColor: foreground), ); } } diff --git a/mobile/packages/ui/lib/src/components/password_input.dart b/mobile/packages/ui/lib/src/components/password_input.dart index 70808472df..e99e9730a8 100644 --- a/mobile/packages/ui/lib/src/components/password_input.dart +++ b/mobile/packages/ui/lib/src/components/password_input.dart @@ -52,7 +52,6 @@ class _ImmichPasswordInputState extends State { icon: Icon(_visible ? Icons.visibility_off_rounded : Icons.visibility_rounded), ), autofillHints: [AutofillHints.password], - keyboardType: TextInputType.text, ); } } diff --git a/mobile/packages/ui/lib/src/components/text_button.dart b/mobile/packages/ui/lib/src/components/text_button.dart index 6dc677aee2..fb3459897c 100644 --- a/mobile/packages/ui/lib/src/components/text_button.dart +++ b/mobile/packages/ui/lib/src/components/text_button.dart @@ -1,85 +1,72 @@ import 'dart:async'; import 'package:flutter/material.dart'; -import 'package:immich_ui/src/constants.dart'; -import 'package:immich_ui/src/types.dart'; +import 'package:immich_ui/immich_ui.dart'; -class ImmichTextButton extends StatelessWidget { +class ImmichTextButton extends StatefulWidget { final String labelText; final IconData? icon; final FutureOr Function() onPressed; final ImmichVariant variant; - final ImmichColor color; final bool expanded; - final bool loading; final bool disabled; + final bool? loading; const ImmichTextButton({ super.key, required this.labelText, this.icon, required this.onPressed, - this.variant = ImmichVariant.filled, - this.color = ImmichColor.primary, + this.variant = .filled, this.expanded = true, - this.loading = false, + this.disabled = false, + this.loading, }); - Widget _buildButton(ImmichVariant variant) { - final Widget? effectiveIcon = loading - ? const SizedBox.square( - dimension: ImmichIconSize.md, - child: CircularProgressIndicator(strokeWidth: ImmichBorderWidth.lg), - ) - : icon != null - ? Icon(icon, fontWeight: FontWeight.w600) - : null; - final hasIcon = effectiveIcon != null; + @override + State createState() => _ImmichTextButtonState(); +} - final label = Text(labelText, style: const TextStyle(fontSize: ImmichTextSize.body, fontWeight: FontWeight.bold)); - final style = ElevatedButton.styleFrom(padding: const EdgeInsets.symmetric(vertical: ImmichSpacing.md)); +class _ImmichTextButtonState extends State { + bool _loading = false; + bool get _isLoading => widget.loading ?? _loading; - final effectiveOnPressed = disabled || loading ? null : onPressed; - - switch (variant) { - case ImmichVariant.filled: - if (hasIcon) { - return ElevatedButton.icon( - style: style, - onPressed: effectiveOnPressed, - icon: effectiveIcon, - label: label, - ); - } - - return ElevatedButton( - style: style, - onPressed: effectiveOnPressed, - child: label, - ); - case ImmichVariant.ghost: - if (hasIcon) { - return TextButton.icon( - style: style, - onPressed: effectiveOnPressed, - icon: effectiveIcon, - label: label, - ); - } - - return TextButton( - style: style, - onPressed: effectiveOnPressed, - child: label, - ); + Future _onPressed() async { + setState(() => _loading = true); + try { + await widget.onPressed(); + } finally { + if (mounted) { + setState(() => _loading = false); + } } } @override Widget build(BuildContext context) { - final button = _buildButton(variant); - if (expanded) { + final Widget? icon = _isLoading + ? const SizedBox.square( + dimension: ImmichIconSize.md, + child: CircularProgressIndicator(strokeWidth: ImmichBorderWidth.lg), + ) + : widget.icon != null + ? Icon(widget.icon, fontWeight: .w600) + : null; + + final label = Text( + widget.labelText, + style: const .new(fontSize: ImmichTextSize.body, fontWeight: .bold), + ); + final style = ElevatedButton.styleFrom(padding: const .symmetric(vertical: ImmichSpacing.md)); + final onPressed = widget.disabled || _isLoading ? null : _onPressed; + + final button = switch (widget.variant) { + ImmichVariant.filled => ElevatedButton.icon(style: style, onPressed: onPressed, icon: icon, label: label), + ImmichVariant.ghost => TextButton.icon(style: style, onPressed: onPressed, icon: icon, label: label), + }; + + if (widget.expanded) { return SizedBox(width: double.infinity, child: button); } return button; diff --git a/mobile/packages/ui/lib/src/internal.dart b/mobile/packages/ui/lib/src/internal.dart index 7f503927ff..4ca1f4b073 100644 --- a/mobile/packages/ui/lib/src/internal.dart +++ b/mobile/packages/ui/lib/src/internal.dart @@ -1,6 +1,11 @@ import 'package:flutter/material.dart'; +import 'package:immich_ui/src/color_override.dart'; import 'package:immich_ui/src/translation.dart'; extension TranslationHelper on BuildContext { ImmichTranslations get translations => ImmichTranslationProvider.of(this); } + +extension ColorHelper on BuildContext { + Color? get colorOverride => ImmichColorOverride.maybeOf(this); +} diff --git a/mobile/packages/ui/lib/src/previews/text_button.dart b/mobile/packages/ui/lib/src/previews/text_button.dart index 46c689bbae..fc1af0be3e 100644 --- a/mobile/packages/ui/lib/src/previews/text_button.dart +++ b/mobile/packages/ui/lib/src/previews/text_button.dart @@ -15,16 +15,6 @@ Widget previewTextButtonVariants() => const Wrap( ], ); -@ImmichPreview(group: 'TextButton', name: 'Colors') -Widget previewTextButtonColors() => const Wrap( - spacing: 12, - runSpacing: 12, - children: [ - ImmichTextButton(onPressed: _previewNoop, labelText: 'Primary', expanded: false), - ImmichTextButton(onPressed: _previewNoop, labelText: 'Secondary', color: ImmichColor.secondary, expanded: false), - ], -); - @ImmichPreview(group: 'TextButton', name: 'With Icons') Widget previewTextButtonWithIcons() => const Wrap( spacing: 12, @@ -42,7 +32,11 @@ Widget previewTextButtonWithIcons() => const Wrap( ); @ImmichPreview(group: 'TextButton', name: 'Loading') -Widget previewTextButtonLoading() => const _PreviewLoadingDemo(); +Widget previewTextButtonLoading() => ImmichTextButton( + onPressed: () => Future.delayed(const Duration(seconds: 2)), + labelText: 'Click me', + expanded: false, +); @ImmichPreview(group: 'TextButton', name: 'Disabled') Widget previewTextButtonDisabled() => const Wrap( @@ -59,30 +53,3 @@ Widget previewTextButtonDisabled() => const Wrap( ), ], ); - -class _PreviewLoadingDemo extends StatefulWidget { - const _PreviewLoadingDemo(); - - @override - State<_PreviewLoadingDemo> createState() => _PreviewLoadingDemoState(); -} - -class _PreviewLoadingDemoState extends State<_PreviewLoadingDemo> { - bool _isLoading = false; - - @override - Widget build(BuildContext context) { - return ImmichTextButton( - onPressed: () async { - setState(() => _isLoading = true); - await Future.delayed(const Duration(seconds: 2)); - if (mounted) { - setState(() => _isLoading = false); - } - }, - labelText: _isLoading ? 'Loading...' : 'Click Me', - loading: _isLoading, - expanded: false, - ); - } -} diff --git a/mobile/packages/ui/test/color_override_test.dart b/mobile/packages/ui/test/color_override_test.dart new file mode 100644 index 0000000000..ebd3b19c9d --- /dev/null +++ b/mobile/packages/ui/test/color_override_test.dart @@ -0,0 +1,53 @@ +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:immich_ui/src/color_override.dart'; +import 'package:immich_ui/src/components/icon_button.dart'; + +import 'test_utils.dart'; + +void main() { + group('ImmichColorOverride', () { + testWidgets('exposes the override color to descendants', (tester) async { + Color? captured; + await tester.pumpTestWidget( + ImmichColorOverride( + color: Colors.green, + child: Builder( + builder: (context) { + captured = ImmichColorOverride.maybeOf(context); + return const SizedBox.shrink(); + }, + ), + ), + ); + + expect(captured, Colors.green); + }); + + testWidgets('maybeOf returns null when there is no override', (tester) async { + Color? captured = Colors.black; + await tester.pumpTestWidget( + Builder( + builder: (context) { + captured = ImmichColorOverride.maybeOf(context); + return const SizedBox.shrink(); + }, + ), + ); + + expect(captured, isNull); + }); + + testWidgets('a descendant component adopts the override as its foreground', (tester) async { + await tester.pumpTestWidget( + ImmichColorOverride( + color: Colors.green, + child: ImmichIconButton(icon: Icons.add, onPressed: () {}), + ), + ); + + final button = tester.widget(find.byType(IconButton)); + expect(button.style?.foregroundColor?.resolve({}), Colors.green); + }); + }); +}