Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions packages/stac/lib/src/framework/framework.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@ export 'stac.dart';
export 'stac_app.dart';
export 'stac_registry.dart';
export 'stac_service.dart';
export 'stac_app_theme.dart';
111 changes: 104 additions & 7 deletions packages/stac/lib/src/framework/stac_app.dart
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import 'dart:async';

import 'package:flutter/material.dart';
import 'package:stac/src/parsers/theme/themes.dart';

Expand Down Expand Up @@ -108,8 +110,8 @@ class StacApp extends StatelessWidget {
final TransitionBuilder? builder;
final String title;
final GenerateAppTitle? onGenerateTitle;
final StacTheme? theme;
final StacTheme? darkTheme;
final FutureOr<StacTheme?>? theme;
final FutureOr<StacTheme?>? darkTheme;
final ThemeData? highContrastTheme;
final ThemeData? highContrastDarkTheme;
final ThemeMode? themeMode;
Expand Down Expand Up @@ -142,6 +144,22 @@ class StacApp extends StatelessWidget {
}

Widget _materialApp(BuildContext context) {
return _withResolvedThemes(
context,
(resolvedContext, resolved) =>
_buildMaterialApp(resolvedContext, resolved),
);
}

Widget _materialRouterApp(BuildContext context) {
return _withResolvedThemes(
context,
(resolvedContext, resolved) =>
_buildMaterialAppRouter(resolvedContext, resolved),
);
}

Widget _buildMaterialApp(BuildContext context, _ResolvedStacThemes themes) {
return MaterialApp(
navigatorKey: navigatorKey,
scaffoldMessengerKey: scaffoldMessengerKey,
Expand All @@ -162,8 +180,8 @@ class StacApp extends StatelessWidget {
builder: builder,
title: title,
onGenerateTitle: onGenerateTitle,
theme: theme?.parse(context),
darkTheme: darkTheme?.parse(context),
theme: themes.theme?.parse(context),
darkTheme: themes.darkTheme?.parse(context),
highContrastTheme: highContrastTheme,
highContrastDarkTheme: highContrastDarkTheme,
themeMode: themeMode,
Expand All @@ -188,7 +206,10 @@ class StacApp extends StatelessWidget {
);
}

Widget _materialRouterApp(BuildContext context) {
Widget _buildMaterialAppRouter(
BuildContext context,
_ResolvedStacThemes themes,
) {
return MaterialApp.router(
scaffoldMessengerKey: scaffoldMessengerKey,
routeInformationProvider: routeInformationProvider,
Expand All @@ -200,8 +221,8 @@ class StacApp extends StatelessWidget {
title: title,
onGenerateTitle: onGenerateTitle,
color: color,
theme: theme?.parse(context),
darkTheme: darkTheme?.parse(context),
theme: themes.theme?.parse(context),
darkTheme: themes.darkTheme?.parse(context),
highContrastTheme: highContrastTheme,
highContrastDarkTheme: highContrastDarkTheme,
themeMode: themeMode,
Expand All @@ -224,4 +245,80 @@ class StacApp extends StatelessWidget {
scrollBehavior: scrollBehavior,
);
}

FutureOr<_ResolvedStacThemes> _resolveThemes() {
final themeInput = theme;
final darkThemeInput = darkTheme;

final Future<StacTheme?>? themeFuture = themeInput is Future<StacTheme?>
? themeInput
: null;
final Future<StacTheme?>? darkThemeFuture =
darkThemeInput is Future<StacTheme?> ? darkThemeInput : null;

final StacTheme? themeValue = themeFuture == null
? themeInput as StacTheme?
: null;
final StacTheme? darkThemeValue = darkThemeFuture == null
? darkThemeInput as StacTheme?
: null;

if (themeFuture == null && darkThemeFuture == null) {
return _ResolvedStacThemes(theme: themeValue, darkTheme: darkThemeValue);
}

return Future<_ResolvedStacThemes>(() async {
final resolvedTheme =
await (themeFuture ?? Future<StacTheme?>.value(themeValue));
final resolvedDarkTheme =
await (darkThemeFuture ?? Future<StacTheme?>.value(darkThemeValue));

return _ResolvedStacThemes(
theme: resolvedTheme,
darkTheme: resolvedDarkTheme,
);
});
}

Widget _withResolvedThemes(
BuildContext context,
Widget Function(BuildContext, _ResolvedStacThemes) builder,
) {
final resolved = _resolveThemes();
if (resolved is Future<_ResolvedStacThemes>) {
return FutureBuilder<_ResolvedStacThemes>(
future: resolved,
builder: (futureContext, snapshot) {
if (snapshot.connectionState == ConnectionState.waiting) {
return const _ThemeFutureLoading();
}
if (snapshot.hasError) {
return const _ThemeFutureLoading();
}
final themes = snapshot.data;
if (themes == null) {
return const _ThemeFutureLoading();
Comment on lines +295 to +300
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Error state is silently swallowed with no feedback.

When theme loading fails, the same loading indicator is shown indefinitely. Consider logging the error and/or falling back to a default theme rather than showing a perpetual loading state.

           if (snapshot.hasError) {
+            // Log error for debugging
+            debugPrint('Theme loading failed: ${snapshot.error}');
-            return const _ThemeFutureLoading();
+            // Fall back to no theme rather than infinite loading
+            return builder(futureContext, const _ResolvedStacThemes(theme: null, darkTheme: null));
           }

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In packages/stac/lib/src/framework/stac_app.dart around lines 295-300 the
snapshot.hasError branch currently returns the same loading widget and swallows
the error; change this to log the snapshot.error (using debugPrint or the app
logger) and return a sensible fallback (for example a default ThemeData-based
widget or an error/fallback _Theme widget) instead of _ThemeFutureLoading so the
app doesn't show a perpetual spinner; ensure snapshot.error and
snapshot.stackTrace are included in the log and that the fallback provides safe
default colors/fonts so UI remains usable.

}
return builder(futureContext, themes);
},
);
}
return builder(context, resolved);
}
Comment on lines +283 to +307
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Future is recreated on every widget rebuild.

_resolveThemes() is called inside _withResolvedThemes during build(). When theme or darkTheme is a Future, a new Future object is created on each rebuild. FutureBuilder uses reference equality, so it will restart the async operation and show the loading state again whenever StacApp rebuilds.

Consider caching the resolved future in a stateful widget or using AsyncSnapshot's connectionState more carefully, or lift the resolution to an ancestor widget/state.

-class StacApp extends StatelessWidget {
+class StacApp extends StatefulWidget {
   const StacApp({
     ...
   });
+
+  @override
+  State<StacApp> createState() => _StacAppState();
+}
+
+class _StacAppState extends State<StacApp> {
+  late final FutureOr<_ResolvedStacThemes> _resolvedThemes;
+
+  @override
+  void initState() {
+    super.initState();
+    _resolvedThemes = _resolveThemes();
+  }
+
+  // Move build logic here, using _resolvedThemes instead of calling _resolveThemes()

Committable suggestion skipped: line range outside the PR's diff.

}

class _ResolvedStacThemes {
const _ResolvedStacThemes({required this.theme, required this.darkTheme});

final StacTheme? theme;
final StacTheme? darkTheme;
}

class _ThemeFutureLoading extends StatelessWidget {
const _ThemeFutureLoading();

@override
Widget build(BuildContext context) {
return const Material(child: Center(child: CircularProgressIndicator()));
}
}
81 changes: 81 additions & 0 deletions packages/stac/lib/src/framework/stac_app_theme.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
import 'dart:convert';

import 'package:flutter/widgets.dart';
import 'package:stac/src/services/stac_cloud.dart';
import 'package:stac/src/services/stac_network_service.dart';
import 'package:stac_core/actions/network_request/stac_network_request.dart';
import 'package:stac_core/foundation/theme/stac_theme/stac_theme.dart';

/// Provides helpers to load [StacTheme] definitions for [StacApp].
class StacAppTheme {
const StacAppTheme._();

/// Fetches a theme from the `/themes` endpoint by [themeName].
///
/// Returns `null` if the network call fails or the payload is malformed.
static Future<StacTheme?> fromCloud({required String themeName}) async {
final response = await StacCloud.fetchTheme(themeName: themeName);
if (response == null) {
return null;
}

final rawData = response.data;
if (rawData is! Map<String, dynamic>) {
return null;
}

final themePayload = _normalizeThemeJson(rawData['stacJson']);
if (themePayload == null) {
return null;
}

return StacTheme.fromJson(themePayload);
}

/// Fetches a theme over HTTP using a [StacNetworkRequest].
///
/// Mirrors [Stac.fromNetwork], allowing callers to reuse existing request
/// builders and middleware.
static Future<StacTheme?> fromNetwork({
required BuildContext context,
required StacNetworkRequest request,
}) async {
final response = await StacNetworkService.request(context, request);
if (response == null) {
return null;
}

return fromJson(response.data);
}

/// Creates a [StacTheme] from raw JSON payloads.
///
/// Accepts either a `Map<String, dynamic>` or a JSON `String`. Returns `null`
/// when the payload cannot be parsed into a valid [StacTheme].
static StacTheme? fromJson(dynamic payload) {
final themePayload = _normalizeThemeJson(payload);
if (themePayload == null) {
return null;
}
return StacTheme.fromJson(themePayload);
}

static Map<String, dynamic>? _normalizeThemeJson(dynamic payload) {
if (payload == null) {
return null;
}
if (payload is Map<String, dynamic> && payload['stacJson'] != null) {
return _normalizeThemeJson(payload['stacJson']);
}
if (payload is Map<String, dynamic>) {
return payload;
}
if (payload is String) {
final decoded = jsonDecode(payload);
if (decoded is Map<String, dynamic>) {
return decoded;
}
}
Comment on lines +73 to +78
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

jsonDecode can throw FormatException on invalid JSON.

If payload is a malformed JSON string, jsonDecode throws an exception that isn't caught, propagating up to the caller unexpectedly.

     if (payload is String) {
-      final decoded = jsonDecode(payload);
-      if (decoded is Map<String, dynamic>) {
-        return decoded;
+      try {
+        final decoded = jsonDecode(payload);
+        if (decoded is Map<String, dynamic>) {
+          return decoded;
+        }
+      } on FormatException {
+        return null;
       }
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (payload is String) {
final decoded = jsonDecode(payload);
if (decoded is Map<String, dynamic>) {
return decoded;
}
}
if (payload is String) {
try {
final decoded = jsonDecode(payload);
if (decoded is Map<String, dynamic>) {
return decoded;
}
} on FormatException {
return null;
}
}
🤖 Prompt for AI Agents
In packages/stac/lib/src/framework/stac_app_theme.dart around lines 73 to 78,
jsonDecode(payload) can throw a FormatException for malformed JSON; wrap the
jsonDecode call in a try/catch that catches FormatException (and optionally any
other parsing errors), handle the error by returning null (or the function's
appropriate fallback) and/or log the parse failure so the exception doesn't
propagate; ensure existing behavior still returns the decoded Map when parsing
succeeds.

return null;
}
}
8 changes: 8 additions & 0 deletions packages/stac/lib/src/models/stac_artifact_type.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
/// Type of artifact that can be fetched from Stac Cloud.
enum StacArtifactType {
/// A screen artifact.
screen,

/// A theme artifact.
theme,
}
Loading