Next #12
Next #12
Conversation
WalkthroughThis pull request implements extensive refactoring and new functionality for a slide-based presentation system. Major updates include enhanced dependency management and build configuration using FVM, Melos, and updated GitHub workflows; platform-specific plugin re-registration for Linux, macOS, and Windows; and significant improvements to UI components, slide rendering, markdown display, and PDF export via robust slide capture. Numerous legacy files, obsolete chat components, and deprecated widgets have been removed, while new providers, mapping utilities, and helper classes have been introduced to streamline deck options and slide configurations. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant D as PdfExportDialogScreen
participant PC as PdfController
participant SCS as SlideCaptureService
participant TC as ThumbnailController
participant FS as FileSaver/System
U->>D: Initiate PDF export
D->>PC: export() method called
PC->>PC: prepare() slides for export
PC->>SCS: capture() slide images (with retry logic)
SCS-->>TC: Utilize ThumbnailController for image capture
TC-->>PC: Returns image data for each slide
PC->>PC: Build PDF document in isolate
PC->>FS: Save PDF file to storage
FS-->>PC: Confirm file saved
PC-->>D: Update export status (complete)
D->>U: Display export result dialog
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (55)
packages/superdeck/example/macos/Runner/Release.entitlements (2)
5-6: Verify sandbox compatibility with existing features.Enabling app sandboxing is a good security practice, but it may restrict access to system resources. Please ensure all existing features work correctly under sandbox constraints.
Consider documenting any sandbox-related limitations or requirements in the README to help users and developers understand the security model.
5-12: Review security implications of combined entitlements.The combination of entitlements grants significant capabilities:
- App sandboxing (security boundary)
- Read/write access to user-selected files
- Both client and server network capabilities
While these permissions align with a presentation app's needs, they increase the attack surface.
Consider implementing additional security measures:
- File access validation before read/write operations
- Network request validation and rate limiting
- Secure storage for any cached user data
packages/superdeck/example/pubspec_overrides.yaml (1)
3-8: MIX Package Dependency Overrides:The previously commented-out dependencies for
mix_generator,mix_lint, andmixhave been reactivated with updated relative paths pointing four directories up. Please verify that these paths accurately reflect the new monorepo structure. For enhanced maintainability, consider documenting (via inline comments) the rationale behind using such relative paths.packages/superdeck/example/lib/src/style.dart (4)
6-6:_accentis unused.
Currently,_accentis defined but used only in commented-out code. Consider removing it or reintroducing it where needed.
8-44: Large block of commented-out code may reduce clarity.
If these style configurations are no longer needed, consider removing them to improve readability. Otherwise, if they're placeholders for future development, add a short inline comment clarifying the intent.
69-96: Consider unifying color usage with_accent.
AnnouncementStylesets a distinct color for headings. If the intent is to maintain a brand color, you might unify usage with_accent.
98-117: Unify border color for better brand consistency.
QuoteStyleuses a hardcoded red border. If brand consistency is desired, consider using a shared accent color.packages/superdeck/example/.superdeck/superdeck.json (8)
3-38: Slide 'cS8UY7ii' – Structure and Placeholder Content:
This slide is well organized with an appropriate hierarchy: it contains akey, emptyoptions, asectionsarray whose single section holds multipleblocks. Note the placeholder markdown in the first block (line 10) and the Twitter widget configuration (lines 15–22). Please verify that these placeholder texts and widget details (e.g.,"username": "faafasdf") will be replaced or are intended for demonstration.
39-70: Slide 'nPPBLQ6k' – Handling Empty and Minimal Content Blocks:
This slide includes blocks with empty content (lines 45, 47, and 58) alongside a heading block (line 52). Confirm whether the empty blocks are intentional placeholders for spacing or if they could be removed for a cleaner configuration.
204-231: Slide 'H2GzZVSx' – Personal Information Display:
The slide presents personal and professional details using two blocks with different alignments ("center_right"and"center_left"). Double-check that the formatting (including"leoafarias{.here}") correctly represents the intended display without unintended artifacts.
265-297: Slide 'ybLDY8oi' – Generative UI Headers:
This slide introduces the concept of Generative UI using multiple heading levels. While the headings are clear, verify that the visual hierarchy remains consistent across devices and consider if the repetition could be streamlined.
318-337: Slide '9mHDFwa9' – Emphasized Title with Emoji:
The title combines text with an emoji (LLM ❤️ GUI{.animate}). It would be wise to check emoji rendering consistency across different platforms to ensure the intended visual impact is maintained.
338-370: Slide '9BmK4SPw' – Managing Empty Blocks and Descriptive Text:
This slide contains an empty block (lines 344–349) alongside a descriptive text block (lines 350–356). Consider removing or merging the empty blocks if they do not serve a specific spacing or layout purpose, thus simplifying the configuration.
391-417: Slide 'oglBIjM0' – Promotion of Flutter for Generative UI:
The content promotes Flutter’s suitability for generative UIs and includes HTML<br>elements for line breaks. Verify that these HTML tags integrate seamlessly with your markdown rendering process to produce the intended layout.
555-556: Configuration Section:
The"config"object is currently empty. Verify that this is intentional; if further customization is anticipated, consider populating this section or removing it for clarity.packages/superdeck/lib/src/components/molecules/slide_screen.dart (1)
20-20: Consider theming for background color.
Using a hardcoded ARGB value is fine for a fixed design, but relying on a theme-based color can improve maintainability and customization.- color: const Color.fromARGB(255, 29, 29, 29), + color: Theme.of(context).colorScheme.background,packages/superdeck/lib/src/components/molecules/block_widget.dart (3)
47-107: Stack-based block layout.
The use of flex-based offsets, aStackwidget, and switch expressions to render different block types is elegant and maintainable. Consider logging or handling unknown block types if your data could be malformed.- _ => const SizedBox.shrink(), + _ => Container( + color: Colors.red, + child: const Text('Unknown block type'), + ),
214-236:_ImageBlockWidget: Potential fallback handling.
Implementation is correct for typical use, but consider adding error fallback for invalid or unreachable URIs if that’s a common case in your app.
292-322:_DartPadBlockWidget: URL construction.
Embedding a DartPad instance is a neat feature. Ifid,theme, orembedcan be user-generated, consider validating or sanitizing them before building the URL to avoid potential issues.packages/superdeck/example/lib/src/parts/footer.dart (1)
3-23: Consider making the footer text configurable.The footer text is currently hardcoded. Consider making it configurable through a constructor parameter to improve reusability and flexibility.
Apply this diff to make the text configurable:
class FooterPart extends StatelessWidget implements PreferredSizeWidget { const FooterPart({ super.key, + this.text = 'SUPERDECK', }); + final String text; + @override Size get preferredSize => const Size.fromHeight(50); @override Widget build(context) { - return const Padding( - padding: EdgeInsets.symmetric(horizontal: 16.0), + return Padding( + padding: const EdgeInsets.symmetric(horizontal: 16.0), child: Row( mainAxisAlignment: MainAxisAlignment.start, children: [ - Text('SUPERDECK'), + Text(text), ], ), ); } }packages/superdeck/example/lib/src/parts/header.dart (1)
4-29: Consider making the fallback title configurable.The fallback title is currently hardcoded. Consider making it configurable through a constructor parameter to improve reusability and flexibility.
Apply this diff to make the fallback title configurable:
class HeaderPart extends StatelessWidget implements PreferredSizeWidget { const HeaderPart({ super.key, + this.fallbackTitle = 'Generative UI with Flutter', }); + final String fallbackTitle; + @override Size get preferredSize => const Size.fromHeight(50); @override Widget build(context) { final slide = SlideConfiguration.of(context); final index = slide.slideIndex; return Padding( padding: const EdgeInsets.symmetric(horizontal: 16.0), child: Row( mainAxisAlignment: MainAxisAlignment.end, children: [ - Text(slide.options?.title ?? 'Generative UI with Flutter'), + Text(slide.options?.title ?? fallbackTitle), const SizedBox(width: 20), Text('${index + 1}'), ], ), ); } }packages/superdeck/lib/src/components/organisms/comments_panel.dart (1)
4-33: Consider making the style configurable.The style is currently hardcoded. Consider making it configurable through a constructor parameter to improve reusability and flexibility.
Apply this diff to make the style configurable:
class CommentsPanel extends StatelessWidget { const CommentsPanel({ super.key, required this.comments, + Style? style, + }) : style = style ?? + Style( + $box.margin(10, 10, 0, 10), + $box.color(const Color.fromARGB(255, 35, 35, 35)), + $box.padding(10), + $box.borderRadius(10), + $flex.crossAxisAlignment.stretch(), + ); final List<String> comments; + final Style style; @override Widget build(BuildContext context) { - final style = Style( - $box.margin(10, 10, 0, 10), - $box.color(const Color.fromARGB(255, 35, 35, 35)), - $box.padding(10), - $box.borderRadius(10), - $flex.crossAxisAlignment.stretch(), - ); return VBox( style: style, children: [ SingleChildScrollView( child: Column( crossAxisAlignment: CrossAxisAlignment.start, children: comments.map(Text.new).toList(), ), ), ], ); } }packages/superdeck/lib/src/components/atoms/async_snapshot_widget.dart (1)
24-35: Consider enhancing error and loading state handling.While the implementation is functional, consider these improvements:
- Use
snapshot.data!instead ofrequireDatato make the null-check more explicit.- Add error details for better debugging.
- Consider making the loading indicator customizable.
Widget _buildSnapshot<T>( AsyncSnapshot<T> snapshot, Widget Function(T data) builder, ) { if (snapshot.hasData) { - return builder(snapshot.requireData); + return builder(snapshot.data!); } else if (snapshot.hasError) { - return Center(child: Text('Error: ${snapshot.error}')); + return Center( + child: Text( + 'Error: ${snapshot.error}\n${snapshot.stackTrace}', + textAlign: TextAlign.center, + ), + ); } - return const Center(child: CircularProgressIndicator()); + return const Center( + child: CircularProgressIndicator.adaptive(), + ); }packages/superdeck/lib/src/components/atoms/linear_progresss_indicator_widget.dart (1)
3-14: Fix typo in filename.The filename contains a typo: "linear_progresss_indicator_widget.dart" has an extra 's'. Rename it to "linear_progress_indicator_widget.dart".
packages/superdeck/lib/src/modules/common/helpers/provider.dart (2)
23-30: Enhance error message inofmethod.The error message could be more descriptive to help developers understand the context of the failure.
- throw FlutterError('Provider of type $T not found in the widget tree'); + throw FlutterError( + 'Could not find an InheritedData<$T> widget above the current context.\n' + 'Make sure there is an InheritedData<$T> widget in the widget tree above the current context.' + );
46-52: Enhance error message inofmethod.The error message could be more descriptive to help developers understand the context of the failure.
- throw FlutterError('Provider of type $T not found in the widget tree'); + throw FlutterError( + 'Could not find an InheritedNotifierData<$T> widget above the current context.\n' + 'Make sure there is an InheritedNotifierData<$T> widget in the widget tree above the current context.' + );packages/superdeck/lib/src/components/molecules/block_provider.dart (1)
41-46: Make_tryAnyBlockDatamore extensible.The method currently hardcodes block types, which could make it difficult to add new block types in the future.
Consider using a more extensible approach:
- static BlockData? _tryAnyBlockData(BuildContext context) { - return inheritedData<ColumnBlock>(context) ?? - inheritedData<WidgetBlock>(context) ?? - inheritedData<ImageBlock>(context) ?? - inheritedData<DartPadBlock>(context); - } + static final List<Type> _blockTypes = [ + ColumnBlock, + WidgetBlock, + ImageBlock, + DartPadBlock, + ]; + + static BlockData? _tryAnyBlockData(BuildContext context) { + for (final type in _blockTypes) { + final data = context.dependOnInheritedWidgetOfExactType<InheritedData<BlockData>>()?.data; + if (data != null && data.block.runtimeType == type) { + return data; + } + } + return null; + }packages/superdeck/lib/src/components/atoms/cache_image_widget.dart (1)
58-65: Enhance error widget.The error widget could provide more detailed information and better visual feedback.
errorBuilder: (context, error, stackTrace) { return Container( - color: Colors.red, + decoration: BoxDecoration( + color: Colors.red.withOpacity(0.1), + border: Border.all(color: Colors.red), + borderRadius: BorderRadius.circular(8), + ), + padding: const EdgeInsets.all(16), child: Center( - child: Text('Error loading image: $uri '), + child: Column( + mainAxisSize: MainAxisSize.min, + children: [ + const Icon(Icons.error_outline, color: Colors.red), + const SizedBox(height: 8), + Text( + 'Failed to load image', + style: Theme.of(context).textTheme.titleMedium?.copyWith( + color: Colors.red, + ), + ), + const SizedBox(height: 4), + Text( + uri.toString(), + style: Theme.of(context).textTheme.bodySmall, + ), + if (error != null) ...[ + const SizedBox(height: 4), + Text( + error.toString(), + style: Theme.of(context).textTheme.bodySmall, + ), + ], + ], + ), ), ); },packages/superdeck/lib/src/components/atoms/markdown_viewer.dart (1)
25-45: Enhance null-safety in the state implementation.The current implementation has potential null-safety issues:
_styleTween!.evaluate()uses force unwrap- No null check in
forEachTweenConsider this safer implementation:
@override Widget build(BuildContext context) { - final spec = _styleTween!.evaluate(animation) ?? const SlideSpec(); + final tween = _styleTween; + final spec = tween?.evaluate(animation) ?? const SlideSpec(); return _MarkdownBuilder( content: widget.content, spec: spec, ); }packages/superdeck/lib/src/components/organisms/keyboard_shortcuts.dart (2)
10-19: Consider adding error handling and return value documentation.The action implementation could be more robust:
- No error handling for navigation failures
- Return value purpose is not documented
Consider this enhanced implementation:
class NextSlideAction extends Action<NextSlideIntent> { final NavigationController navigation; NextSlideAction({required this.navigation}); @override + /// Returns true if navigation was successful, false otherwise Object? invoke(covariant NextSlideIntent intent) { + try { navigation.nextSlide(); - return null; + return true; + } catch (e) { + return false; + } } }
32-67: Consider platform-specific keyboard shortcuts.The current implementation uses the meta key (Command/Control) for all shortcuts, which might not be ideal for all platforms. Consider:
- Adding platform-specific key mappings
- Supporting alternative keys (e.g., Alt for Windows/Linux)
Example implementation:
@override Widget build(BuildContext context) { final navigation = NavigationController.of(context); + final platform = Theme.of(context).platform; + final modifierKey = platform == TargetPlatform.macOS + ? LogicalKeyboardKey.meta + : LogicalKeyboardKey.control; return Shortcuts( shortcuts: <LogicalKeySet, Intent>{ - LogicalKeySet(LogicalKeyboardKey.arrowRight, LogicalKeyboardKey.meta): + LogicalKeySet(LogicalKeyboardKey.arrowRight, modifierKey): NextSlideIntent(), // Update other shortcuts similarly }, // ... rest of the implementation ); }packages/superdeck/lib/src/components/molecules/scaled_app.dart (1)
6-64: Optimize scaling and positioning logic.The current implementation has a few areas for improvement:
- Complex positioning calculations could be simplified
- MediaQuery rebuild optimization
Consider these improvements:
@override Widget build(BuildContext context) { + // Cache MediaQuery to avoid rebuilds + final mediaQuery = MediaQuery.of(context); return LayoutBuilder( builder: (context, constraints) { - var width = constraints.biggest.width; - var height = width / kAspectRatio; + final size = _calculateFittedSize( + constraints: constraints, + targetAspectRatio: kAspectRatio, + ); + final scale = Size( + size.width / targetSize.width, + size.height / targetSize.height, + ); - if (height > constraints.biggest.height) { - height = constraints.biggest.height; - width = height * kAspectRatio; - } - final scaleWidth = width / targetSize.width; - final scaleHeight = height / targetSize.height; // ... rest of the implementation with simplified positioning }, ); } +Size _calculateFittedSize({ + required BoxConstraints constraints, + required double targetAspectRatio, +}) { + var width = constraints.biggest.width; + var height = width / targetAspectRatio; + if (height > constraints.biggest.height) { + height = constraints.biggest.height; + width = height * targetAspectRatio; + } + return Size(width, height); +}packages/superdeck/lib/src/components/molecules/bottom_bar.dart (2)
7-60: Enhance accessibility and state handling.Consider the following improvements:
- Add tooltip and semantics labels to IconButtons
- Handle edge cases in navigation state
Example improvements:
IconButton( onPressed: navigation.toggleNotes, + tooltip: navigation.isNotesOpen ? 'Hide Notes' : 'Show Notes', icon: navigation.isNotesOpen ? const Icon(Icons.comment) : const Icon(Icons.comments_disabled), ), // ... IconButton( icon: const Icon(Icons.arrow_back), - onPressed: navigation.previousSlide, + onPressed: currentPage > 1 ? navigation.previousSlide : null, + tooltip: 'Previous Slide', ),
62-85: Make styling theme-aware.The container uses hard-coded colors which could be derived from the theme for better customization support.
Consider this theme-aware implementation:
@override Widget build(BuildContext context) { + final theme = Theme.of(context); final style = Style( $box.height(60), $box.borderRadius(16), - $box.color(const Color.fromARGB(255, 17, 17, 17)), + $box.color(theme.colorScheme.surface), $box.padding(10, 20), $box.margin(12), ); // ... rest of the implementation }packages/superdeck/lib/src/modules/common/helpers/converters.dart (1)
34-66: Consider breaking down the complex alignment logic for better maintainability.The
toFlexAlignmentmethod contains complex logic for determining alignments. Consider extracting the tuple initialization logic into separate methods for better readability and maintainability.static (MainAxisAlignment mainAxis, CrossAxisAlignment crossAxis) toFlexAlignment(Axis axis, ContentAlignment alignment) { + final (mainStart, mainCenter, mainEnd) = _getMainAxisAlignments(axis); + final (crossStart, crossCenter, crossEnd) = _getCrossAxisAlignments(); - final isHorizontal = axis == Axis.horizontal; - final (mainStart, mainCenter, mainEnd) = isHorizontal - ? ( - MainAxisAlignment.start, - MainAxisAlignment.center, - MainAxisAlignment.end - ) - : ( - MainAxisAlignment.end, - MainAxisAlignment.center, - MainAxisAlignment.start - ); - - final (crossStart, crossCenter, crossEnd) = ( - CrossAxisAlignment.start, - CrossAxisAlignment.center, - CrossAxisAlignment.end - ); return switch (alignment) { ContentAlignment.topLeft => (mainStart, crossStart), // ... rest of the switch cases }; } + static (MainAxisAlignment, MainAxisAlignment, MainAxisAlignment) + _getMainAxisAlignments(Axis axis) { + return axis == Axis.horizontal + ? ( + MainAxisAlignment.start, + MainAxisAlignment.center, + MainAxisAlignment.end + ) + : ( + MainAxisAlignment.end, + MainAxisAlignment.center, + MainAxisAlignment.start + ); + } + + static (CrossAxisAlignment, CrossAxisAlignment, CrossAxisAlignment) + _getCrossAxisAlignments() { + return ( + CrossAxisAlignment.start, + CrossAxisAlignment.center, + CrossAxisAlignment.end + ); + }packages/superdeck/lib/src/components/organisms/webview_wrapper.dart (1)
75-99: Enhance UI feedback for better user experience.Consider adding loading indicators and error states to provide better feedback to users.
@override Widget build(BuildContext context) { return Stack( children: [ WebViewWidget(key: ValueKey(_hide), controller: _controller), + if (_isLoading) + const Center( + child: CircularProgressIndicator(), + ), Row( children: [ IconButton( onPressed: () { setState(() { _hide = !_hide; }); }, icon: const Icon(Icons.refresh), + tooltip: 'Refresh page', ), IconButton( onPressed: clearDartPadEditor, icon: const Icon(Icons.clear), + tooltip: 'Clear editor', ), ], ) ], ); }packages/superdeck/lib/src/components/organisms/thumbnail_panel.dart (2)
62-94: Document the complex scroll alignment logic for better maintainability.The
_scrollToActiveSlidemethod contains complex alignment calculations that would benefit from detailed documentation explaining the logic and edge cases.+ /// Scrolls to the active slide with smart alignment calculation. + /// + /// The alignment is calculated based on the following rules: + /// - If the target item is not visible: + /// - For items before the visible range: Align to start (0) + /// - For items after the visible range: Align to 70% (0.7) + /// - If the target item is visible: + /// - If trailing edge is beyond view: Adjust to fit + /// - If leading edge is before view: Align to start + /// - Otherwise: Maintain current leading edge alignment void _scrollToActiveSlide(int index) { final visibleItems = _visibleItems; double alignment;
104-127: Optimize list building for better performance.The list builder could be optimized to reduce unnecessary rebuilds and improve scrolling performance.
@override Widget build(BuildContext context) { + // Memoize the item builder to prevent unnecessary rebuilds + final itemBuilder = useMemoized( + () => (BuildContext context, int index) { + return Padding( + padding: const EdgeInsets.symmetric( + vertical: 8, + horizontal: 10, + ), + child: GestureDetector( + onTap: () => widget.onItemTap(index), + child: widget.itemBuilder(index, index == widget.activeIndex), + ), + ); + }, + [widget.itemBuilder, widget.activeIndex], + ); return Container( color: Colors.black, child: ScrollablePositionedList.builder( scrollDirection: context.isSmall ? Axis.horizontal : Axis.vertical, itemCount: widget.itemCount, itemPositionsListener: _itemPositionsListener, itemScrollController: _itemScrollController, padding: const EdgeInsets.all(20), - itemBuilder: (context, index) { - return Padding( - padding: const EdgeInsets.symmetric( - vertical: 8, - horizontal: 10, - ), - child: GestureDetector( - onTap: () => widget.onItemTap(index), - child: widget.itemBuilder(index, index == widget.activeIndex), - ), - ); - }), + itemBuilder: itemBuilder, + ), ); }packages/superdeck/lib/src/components/atoms/slide_view.dart (2)
24-41: Consider extracting debug styling to a separate constant.The debug information styling could be reused elsewhere in the application.
+const _debugTextStyle = TextStyle( + color: Colors.black, + fontSize: 12, +); Positioned _renderDebugInfo(SectionBlock section, Size slideSize) { final label = ''' @section | blocks: ${section.blocks.length} | ${slideSize.width.toStringAsFixed(2)} x ${slideSize.height.toStringAsFixed(2)} | align: ${section.align} | flex: ${section.flex}'''; - const textStyle = TextStyle( - color: Colors.black, - fontSize: 12, - ); return Positioned( bottom: 0, left: 0, child: Container( color: Colors.cyan, padding: const EdgeInsets.all(8), - child: Text(label, style: textStyle), + child: Text(label, style: _debugTextStyle), ), ); }
43-96: Consider breaking down the complex section rendering logic.The
_renderSectionsmethod is handling multiple responsibilities: calculating sizes, managing offsets, and rendering widgets. Consider splitting it into smaller, focused methods for better maintainability.+ Map<SectionBlock, Size> _calculateSectionSizes(List<SectionBlock> sections, Size slideSize) { + final totalSectionsFlex = sections.fold(0, (previous, section) => previous + section.flex); + return { + for (var section in sections) + section: Size( + slideSize.width, + slideSize.height * (section.flex / totalSectionsFlex), + ) + }; + } + + Map<SectionBlock, Offset> _calculateSectionOffsets(Map<SectionBlock, Size> sectionSizes) { + final offsets = <SectionBlock, Offset>{}; + var currentOffset = Offset.zero; + for (var entry in sectionSizes.entries) { + offsets[entry.key] = Offset(0, currentOffset.dy); + currentOffset = Offset(currentOffset.dx, currentOffset.dy + entry.value.height); + } + return offsets; + } Widget _renderSections(SlideConfiguration configuration, Size slideSize) { final sections = configuration.sections; if (sections.isEmpty) { return const SizedBox.shrink(); } - final totalSectionsFlex = - sections.fold(0, (previous, section) => previous + section.flex); - - final sectionSizes = <SectionBlock, Size>{}; - for (var section in sections) { - final heightPercentage = section.flex / totalSectionsFlex; - final sectionSize = Size( - slideSize.width, - slideSize.height * heightPercentage, - ); - sectionSizes[section] = sectionSize; - } - - Offset currentOffset = Offset.zero; - Map<SectionBlock, Offset> sectionOffsets = {}; - for (var section in sectionSizes.entries) { - final sectionOffset = Offset(0, currentOffset.dy); - sectionOffsets[section.key] = sectionOffset; - currentOffset = Offset( - currentOffset.dx, - currentOffset.dy + section.value.height, - ); - } + final sectionSizes = _calculateSectionSizes(sections, slideSize); + final sectionOffsets = _calculateSectionOffsets(sectionSizes);packages/superdeck/lib/src/components/organisms/app_shell.dart (2)
13-67: Consider extracting the side panel layout to a separate widget.The Column widget in the side panel contains complex layout logic that could be extracted to improve readability and maintainability.
+class _SidePanel extends StatelessWidget { + final bool isNotesOpen; + final void Function(int) onItemTap; + final int activeIndex; + final Widget Function(int, bool) itemBuilder; + final int itemCount; + final List<String> comments; + + const _SidePanel({ + required this.isNotesOpen, + required this.onItemTap, + required this.activeIndex, + required this.itemBuilder, + required this.itemCount, + required this.comments, + }); + + @override + Widget build(BuildContext context) { + return Column( + children: [ + Expanded( + flex: 3, + child: ThumbnailPanel( + onItemTap: onItemTap, + activeIndex: activeIndex, + itemBuilder: itemBuilder, + itemCount: itemCount, + ), + ), + isNotesOpen + ? Expanded( + flex: 1, + child: CommentsPanel( + comments: comments, + ), + ) + : const SizedBox(), + ], + ); + } +}Then use it in the AppShell:
- sideWidget: Column( - children: [ - Expanded( - flex: 3, - child: ThumbnailPanel( - onItemTap: navigation.goToSlide, - activeIndex: currentSlide.slideIndex, - itemBuilder: (index, selected) { - return SlideThumbnail( - selected: selected, - slideConfig: deckController.slides[index], - ); - }, - itemCount: deckController.slides.length, - ), - ), - isNotesOpen - ? Expanded( - flex: 1, - child: CommentsPanel( - comments: currentSlide.comments, - ), - ) - : const SizedBox(), - ], - ), + sideWidget: _SidePanel( + isNotesOpen: isNotesOpen, + onItemTap: navigation.goToSlide, + activeIndex: currentSlide.slideIndex, + itemBuilder: (index, selected) { + return SlideThumbnail( + selected: selected, + slideConfig: deckController.slides[index], + ); + }, + itemCount: deckController.slides.length, + comments: currentSlide.comments, + ),
86-164: Consider using AnimatedPositioned for smoother transitions.The current implementation uses SizeTransition for animations. Consider using AnimatedPositioned for smoother layout transitions, especially for the side panel.
- SizeTransition( - axis: Axis.horizontal, - sizeFactor: animation, - child: SizedBox( - width: 300, - child: widget.sideWidget, - ), - ), + AnimatedPositioned( + duration: _duration, + curve: Curves.easeInOut, + left: widget.isOpen ? 0 : -300, + top: 0, + bottom: 0, + width: 300, + child: widget.sideWidget, + ),packages/superdeck/lib/src/components/atoms/slide_thumbnail.dart (2)
70-80: Remove or implement the commented-out code.The commented-out
_handleActionmethod should either be implemented or removed to maintain code cleanliness.
152-184: Consider adding keyboard shortcuts for menu actions.The overlay menu could benefit from keyboard shortcuts for better accessibility.
showMenu( context: context, menuPadding: EdgeInsets.zero, items: [ - menuItem(_PopMenuAction.refreshThumbnail), + PopupMenuItem( + value: _PopMenuAction.refreshThumbnail, + onTap: () => handleAction(_PopMenuAction.refreshThumbnail), + mouseCursor: SystemMouseCursors.click, + child: Row( + mainAxisAlignment: MainAxisAlignment.center, + children: [ + Icon(_PopMenuAction.refreshThumbnail.icon), + const SizedBox(width: 8), + Text('${_PopMenuAction.refreshThumbnail.label} (Ctrl+R)'), + ], + ), + ), ],packages/superdeck/lib/src/components/atoms/loading_indicator.dart (1)
41-55: Consider using FutureBuilder for state management.The current implementation manually manages state updates. Consider using FutureBuilder for cleaner state management.
- Future<void> _triggerChange(bool isLoading) async { - if (isLoading) { - _animationController.reverse().then((value) { - if (mounted) { - setState(() {}); - } - }); - } else { - _animationController.forward().then((value) { - if (mounted) { - setState(() {}); - } - }); - } - } + Future<void> _triggerChange(bool isLoading) { + return isLoading + ? _animationController.reverse() + : _animationController.forward(); + } + @override + Widget build(BuildContext context) { + return FutureBuilder<void>( + future: _triggerChange(widget.isLoading), + builder: (context, snapshot) { + if (widget.isLoading || _animationController.isAnimating) { + return FadeTransition( + opacity: _opacityAnimation, + child: Container( + color: Colors.black87, + child: const Center( + child: IsometricLoading(), + ), + ), + ); + } + return const SizedBox.shrink(); + }, + ); + }packages/superdeck/assets/iframe_template.html (1)
30-58: Function: executeInIframe
This function wraps JavaScript evaluation inside a Promise and first checks that the iframe exists. It accounts for both cases—immediate access if the iframe’s document is ready and delayed execution via theonloadevent.
Security Note: The use ofevalcan be a potential security risk if untrusted code is ever passed to this function. Please ensure that inputs are controlled or sanitized in production.packages/superdeck/example/slides.md (4)
5-6: Section Heading with Class Annotation
The line with the heading (which repeats "Another image" multiple times and includes the{.here}class) is acceptable if it is intended for design/mock purposes. If the repetition is unintentional, consider reducing redundancy.
36-37: Duplicate @section Directives
There are two consecutive@sectiondirectives (one plain, followed by one with configuration{flex: 2}). If both are needed for layered configuration, please document the rationale; otherwise, remove the duplicate to avoid confusion.
266-272: Slide: Color Palette Generator Introduction
The slide introduces the concept of generating a color palette based on text and lists key attributes. There is a minor static analysis note regarding possible duplication or repetition in the description. Verify that the bullet list is intentional and concise.
331-336: @colorpalette Directive Configuration
The new@colorPalettedirective is configured with aschemaflag set to true and a list of prompts. Consider quoting each prompt value (e.g.,"tropical","vibrant", etc.) to avoid potential parsing issues, unless the DSL intentionally accepts unquoted identifiers.packages/superdeck/example/lib/src/parts/background.dart (5)
18-33: Make mesh dimensions and colors configurable.Consider making the mesh dimensions and colors configurable to improve reusability and flexibility.
Apply this diff to make the function more configurable:
-OMeshRect _meshBuilder(List<Color> colors) { +OMeshRect _meshBuilder( + List<Color> colors, { + int width = 3, + int height = 3, + Color fallbackColor = const Color(0xff0e0e0e), + Color backgroundColor = const Color(0x00d6d6d6), +}) { return OMeshRect( - width: 3, - height: 3, - fallbackColor: const Color(0xff0e0e0e), - backgroundColor: const Color(0x00d6d6d6), + width: width, + height: height, + fallbackColor: fallbackColor, + backgroundColor: backgroundColor, vertices: [ (0.0, 0.0).v, (0.5, 0.0).v, (1.0, 0.0).v, // Row 1 (0.0, 0.5).v, (0.5, 0.5).v, (1.0, 0.5).v, // Row 2 (0.0, 1.0).v, (0.5, 1.0).v, (1.0, 1.0).v, // Row 3 ], colors: colors, ); }
50-50: Fix typo in comment.The comment has a typo: "bwett" should be "between".
Apply this diff to fix the typo:
-// animate bwett colors and previous colors in duration +// animate between colors and previous colors in duration
62-92: Make animation duration configurable.Consider making the animation duration configurable through the widget constructor to allow for customization.
Apply this diff to make the duration configurable:
class _AnimatedSwitcherOMesh extends StatefulWidget { final SlideConfiguration slide; + final Duration duration; const _AnimatedSwitcherOMesh({ required this.slide, + this.duration = const Duration(milliseconds: 1000), }); @override _AnimatedSwitcherOMeshState createState() => _AnimatedSwitcherOMeshState(); } class _AnimatedSwitcherOMeshState extends State<_AnimatedSwitcherOMesh> with SingleTickerProviderStateMixin { late List<Color> _colors; - final _duration = const Duration(milliseconds: 1000); - @override void initState() { super.initState(); _colors = _determiniscOrderBasedOnIndex(widget.slide.slideIndex); } @override Widget build(BuildContext context) { return AnimatedOMeshGradient( mesh: _meshBuilder(_colors), - duration: _duration, + duration: widget.duration, ); } }
94-104: Consider making colors configurable and adding more contrast.The current color palette has very similar dark colors which might not provide enough visual contrast. Consider:
- Making the color palette configurable through the widget constructor.
- Adding more contrasting colors to improve visual appeal.
105-108: Fix typo in function name.The function name has a typo: "determiniscOrderBasedOnIndex" should be "deterministicOrderBasedOnIndex".
Apply this diff to fix the typo:
-List<Color> _determiniscOrderBasedOnIndex(int index) { +List<Color> _deterministicOrderBasedOnIndex(int index) { return _buildColors.sublist(index % _buildColors.length) ..addAll(_buildColors.sublist(0, index % _buildColors.length)); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (42)
packages/superdeck/example/.superdeck/assets/mermaid_srHRIuii.pngis excluded by!**/*.pngpackages/superdeck/example/.superdeck/assets/thumbnail_0zqy1l5c.pngis excluded by!**/*.pngpackages/superdeck/example/.superdeck/assets/thumbnail_14RbmSW5.pngis excluded by!**/*.pngpackages/superdeck/example/.superdeck/assets/thumbnail_3sLdrfsM.pngis excluded by!**/*.pngpackages/superdeck/example/.superdeck/assets/thumbnail_9BmK4SPw.pngis excluded by!**/*.pngpackages/superdeck/example/.superdeck/assets/thumbnail_9mHDFwa9.pngis excluded by!**/*.pngpackages/superdeck/example/.superdeck/assets/thumbnail_9y5hBeTm.pngis excluded by!**/*.pngpackages/superdeck/example/.superdeck/assets/thumbnail_CwxHOCpO.pngis excluded by!**/*.pngpackages/superdeck/example/.superdeck/assets/thumbnail_F2fTbXOG.pngis excluded by!**/*.pngpackages/superdeck/example/.superdeck/assets/thumbnail_H2GzZVSx.pngis excluded by!**/*.pngpackages/superdeck/example/.superdeck/assets/thumbnail_RiDZbaFZ.pngis excluded by!**/*.pngpackages/superdeck/example/.superdeck/assets/thumbnail_SJncL4H2.pngis excluded by!**/*.pngpackages/superdeck/example/.superdeck/assets/thumbnail_WyJ8NjmF.pngis excluded by!**/*.pngpackages/superdeck/example/.superdeck/assets/thumbnail_XeDZiCNk.pngis excluded by!**/*.pngpackages/superdeck/example/.superdeck/assets/thumbnail_Z40wIUYP.pngis excluded by!**/*.pngpackages/superdeck/example/.superdeck/assets/thumbnail_aTAXFyQ7.pngis excluded by!**/*.pngpackages/superdeck/example/.superdeck/assets/thumbnail_cS8UY7ii.pngis excluded by!**/*.pngpackages/superdeck/example/.superdeck/assets/thumbnail_nPPBLQ6k.pngis excluded by!**/*.pngpackages/superdeck/example/.superdeck/assets/thumbnail_oglBIjM0.pngis excluded by!**/*.pngpackages/superdeck/example/.superdeck/assets/thumbnail_pukXIjvK.pngis excluded by!**/*.pngpackages/superdeck/example/.superdeck/assets/thumbnail_ybLDY8oi.pngis excluded by!**/*.pngpackages/superdeck/example/.superdeck/assets/thumbnail_z34aal1W.pngis excluded by!**/*.pngpackages/superdeck/example/.superdeck/generated/image_caching_c5tnibJL.gifis excluded by!**/*.gif,!**/generated/**packages/superdeck/example/.superdeck/generated/image_caching_woxVKegn.gifis excluded by!**/*.gif,!**/generated/**packages/superdeck/example/.superdeck/generated/mermaid_s3Iic43G.pngis excluded by!**/*.png,!**/generated/**packages/superdeck/example/.superdeck/generated/thumbnail_9aLrluQc.pngis excluded by!**/*.png,!**/generated/**packages/superdeck/example/.superdeck/generated/thumbnail_AbUkCAD1.pngis excluded by!**/*.png,!**/generated/**packages/superdeck/example/.superdeck/generated/thumbnail_HR8xUsF0.pngis excluded by!**/*.png,!**/generated/**packages/superdeck/example/.superdeck/generated/thumbnail_RiJb4GGG.pngis excluded by!**/*.png,!**/generated/**packages/superdeck/example/.superdeck/generated/thumbnail_bztq1AdA.pngis excluded by!**/*.png,!**/generated/**packages/superdeck/example/.superdeck/generated/thumbnail_hhnMdIWj.pngis excluded by!**/*.png,!**/generated/**packages/superdeck/example/.superdeck/generated/thumbnail_kXnyZsjq.pngis excluded by!**/*.png,!**/generated/**packages/superdeck/example/.superdeck/generated/thumbnail_nuiio8i0.pngis excluded by!**/*.png,!**/generated/**packages/superdeck/example/.superdeck/generated/thumbnail_q6arHVVz.pngis excluded by!**/*.png,!**/generated/**packages/superdeck/example/.superdeck/generated/thumbnail_shPxXVHO.pngis excluded by!**/*.png,!**/generated/**packages/superdeck/example/.superdeck/generated/thumbnail_xRHNCnNN.pngis excluded by!**/*.png,!**/generated/**packages/superdeck/example/assets/llm_interaction.pngis excluded by!**/*.pngpackages/superdeck/example/assets/llm_tools.pngis excluded by!**/*.pngpackages/superdeck/example/assets/structured_output.pngis excluded by!**/*.pngpackages/superdeck/example/assets/widget_response.pngis excluded by!**/*.pngpackages/superdeck/example/macos/Podfile.lockis excluded by!**/*.lockpubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (107)
.gitignore(1 hunks)melos.yaml(0 hunks)packages/superdeck/.cursorignore(1 hunks)packages/superdeck/.gitignore(1 hunks)packages/superdeck/analysis_options.yaml(1 hunks)packages/superdeck/assets/iframe_template.html(1 hunks)packages/superdeck/build.yaml(1 hunks)packages/superdeck/devtools_options.yaml(1 hunks)packages/superdeck/example/.metadata(2 hunks)packages/superdeck/example/.superdeck/generated_assets.json(1 hunks)packages/superdeck/example/.superdeck/slides.json(0 hunks)packages/superdeck/example/.superdeck/superdeck.json(1 hunks)packages/superdeck/example/lib/main.dart(2 hunks)packages/superdeck/example/lib/src/parts/background.dart(1 hunks)packages/superdeck/example/lib/src/parts/footer.dart(1 hunks)packages/superdeck/example/lib/src/parts/header.dart(1 hunks)packages/superdeck/example/lib/src/style.dart(1 hunks)packages/superdeck/example/lib/src/widget/mix_demo.dart(0 hunks)packages/superdeck/example/linux/flutter/generated_plugin_registrant.cc(2 hunks)packages/superdeck/example/linux/flutter/generated_plugins.cmake(1 hunks)packages/superdeck/example/macos/Flutter/GeneratedPluginRegistrant.swift(1 hunks)packages/superdeck/example/macos/Runner.xcodeproj/project.pbxproj(15 hunks)packages/superdeck/example/macos/Runner/Release.entitlements(1 hunks)packages/superdeck/example/pubspec.yaml(1 hunks)packages/superdeck/example/pubspec_overrides.yaml(1 hunks)packages/superdeck/example/slides.md(1 hunks)packages/superdeck/example/superdeck.yaml(0 hunks)packages/superdeck/example/windows/flutter/generated_plugin_registrant.cc(2 hunks)packages/superdeck/example/windows/flutter/generated_plugins.cmake(1 hunks)packages/superdeck/lib/chat/chat_theme.dart(0 hunks)packages/superdeck/lib/chat/components/typing_indicator.dart(0 hunks)packages/superdeck/lib/chat/prompt.dart(0 hunks)packages/superdeck/lib/components/atoms/cache_image_widget.dart(0 hunks)packages/superdeck/lib/components/atoms/linear_progresss_indicator_widget.dart(0 hunks)packages/superdeck/lib/components/atoms/markdown_viewer.dart(0 hunks)packages/superdeck/lib/components/atoms/sized_transition.dart(0 hunks)packages/superdeck/lib/components/atoms/slide_thumbnail.dart(0 hunks)packages/superdeck/lib/components/atoms/slide_view.dart(0 hunks)packages/superdeck/lib/components/atoms/transition_widget.dart(0 hunks)packages/superdeck/lib/components/molecules/code_preview.dart(0 hunks)packages/superdeck/lib/components/molecules/exception_widget.dart(0 hunks)packages/superdeck/lib/components/molecules/navigation_rail.dart(0 hunks)packages/superdeck/lib/components/molecules/slide_content.dart(0 hunks)packages/superdeck/lib/components/molecules/split_view.dart(0 hunks)packages/superdeck/lib/components/organisms/app_shell.dart(0 hunks)packages/superdeck/lib/components/organisms/chat_panel.dart(0 hunks)packages/superdeck/lib/components/organisms/drawer.dart(0 hunks)packages/superdeck/lib/components/organisms/presentation_side_panel.dart(0 hunks)packages/superdeck/lib/components/remix/button.dart(0 hunks)packages/superdeck/lib/components/superdeck_app.dart(0 hunks)packages/superdeck/lib/helpers/cache_memory_image.dart(0 hunks)packages/superdeck/lib/helpers/dialog_page.dart(0 hunks)packages/superdeck/lib/helpers/extensions.dart(0 hunks)packages/superdeck/lib/helpers/hooks.dart(0 hunks)packages/superdeck/lib/helpers/measure_size.dart(0 hunks)packages/superdeck/lib/helpers/routes.dart(0 hunks)packages/superdeck/lib/helpers/section_tag.dart(0 hunks)packages/superdeck/lib/helpers/universal.dart(0 hunks)packages/superdeck/lib/helpers/value_notifiers.dart(0 hunks)packages/superdeck/lib/helpers/watcher.dart(0 hunks)packages/superdeck/lib/models/asset_model.dart(0 hunks)packages/superdeck/lib/models/asset_model.mapper.dart(0 hunks)packages/superdeck/lib/models/config_model.dart(0 hunks)packages/superdeck/lib/models/config_model.mapper.dart(0 hunks)packages/superdeck/lib/models/options_model.dart(0 hunks)packages/superdeck/lib/models/options_model.mapper.dart(0 hunks)packages/superdeck/lib/models/reference_model.dart(0 hunks)packages/superdeck/lib/models/reference_model.mapper.dart(0 hunks)packages/superdeck/lib/models/slide_model.dart(0 hunks)packages/superdeck/lib/models/slide_model.mapper.dart(0 hunks)packages/superdeck/lib/providers/assets_provider.dart(0 hunks)packages/superdeck/lib/providers/controller.dart(0 hunks)packages/superdeck/lib/providers/examples_provider.dart(0 hunks)packages/superdeck/lib/providers/slide_provider.dart(0 hunks)packages/superdeck/lib/providers/snapshot_provider.dart(0 hunks)packages/superdeck/lib/providers/style_provider.dart(0 hunks)packages/superdeck/lib/schema/schema_model.dart(0 hunks)packages/superdeck/lib/schema/schema_validation.dart(0 hunks)packages/superdeck/lib/schema/schema_validation.mapper.dart(0 hunks)packages/superdeck/lib/schema/schema_values.dart(0 hunks)packages/superdeck/lib/screens/export_screen.dart(0 hunks)packages/superdeck/lib/screens/presentation_screen.dart(0 hunks)packages/superdeck/lib/services/export_service.dart(0 hunks)packages/superdeck/lib/services/reference_service.dart(0 hunks)packages/superdeck/lib/src/components/atoms/async_snapshot_widget.dart(1 hunks)packages/superdeck/lib/src/components/atoms/cache_image_widget.dart(1 hunks)packages/superdeck/lib/src/components/atoms/linear_progresss_indicator_widget.dart(1 hunks)packages/superdeck/lib/src/components/atoms/loading_indicator.dart(3 hunks)packages/superdeck/lib/src/components/atoms/markdown_viewer.dart(1 hunks)packages/superdeck/lib/src/components/atoms/slide_thumbnail.dart(1 hunks)packages/superdeck/lib/src/components/atoms/slide_view.dart(1 hunks)packages/superdeck/lib/src/components/molecules/block_provider.dart(1 hunks)packages/superdeck/lib/src/components/molecules/block_widget.dart(1 hunks)packages/superdeck/lib/src/components/molecules/bottom_bar.dart(1 hunks)packages/superdeck/lib/src/components/molecules/scaled_app.dart(3 hunks)packages/superdeck/lib/src/components/molecules/slide_screen.dart(2 hunks)packages/superdeck/lib/src/components/organisms/app_shell.dart(1 hunks)packages/superdeck/lib/src/components/organisms/comments_panel.dart(1 hunks)packages/superdeck/lib/src/components/organisms/keyboard_shortcuts.dart(1 hunks)packages/superdeck/lib/src/components/organisms/thumbnail_panel.dart(1 hunks)packages/superdeck/lib/src/components/organisms/webview_wrapper.dart(1 hunks)packages/superdeck/lib/src/components/superdeck_app.dart(1 hunks)packages/superdeck/lib/src/modules/common/helpers/converters.dart(1 hunks)packages/superdeck/lib/src/modules/common/helpers/extensions.dart(1 hunks)packages/superdeck/lib/src/modules/common/helpers/measure_size.dart(1 hunks)packages/superdeck/lib/src/modules/common/helpers/provider.dart(1 hunks)packages/superdeck/lib/src/modules/common/helpers/root_bundle_data_store.dart(1 hunks)
⛔ Files not processed due to max files limit (28)
- packages/superdeck/lib/src/modules/common/helpers/slide_parts.dart
- packages/superdeck/lib/src/modules/common/helpers/syntax_highlighter.dart
- packages/superdeck/lib/src/modules/common/helpers/utils.dart
- packages/superdeck/lib/src/modules/common/initializer_provider.dart
- packages/superdeck/lib/src/modules/common/markdown/builders/alert_element_builder.dart
- packages/superdeck/lib/src/modules/common/markdown/builders/code_element_builder.dart
- packages/superdeck/lib/src/modules/common/markdown/builders/element_data_provider.dart
- packages/superdeck/lib/src/modules/common/markdown/builders/image_element_builder.dart
- packages/superdeck/lib/src/modules/common/markdown/builders/text_element_builder.dart
- packages/superdeck/lib/src/modules/common/markdown/builders/zero_padding_builder.dart
- packages/superdeck/lib/src/modules/common/markdown/markdown_element_builders.dart
- packages/superdeck/lib/src/modules/common/markdown/markdown_helpers.dart
- packages/superdeck/lib/src/modules/common/styles/style.dart
- packages/superdeck/lib/src/modules/common/styles/style_spec.dart
- packages/superdeck/lib/src/modules/common/styles/style_spec.g.dart
- packages/superdeck/lib/src/modules/deck/deck_controller.dart
- packages/superdeck/lib/src/modules/deck/deck_options.dart
- packages/superdeck/lib/src/modules/deck/deck_options.mapper.dart
- packages/superdeck/lib/src/modules/deck/deck_provider.dart
- packages/superdeck/lib/src/modules/deck/slide_configuration.dart
- packages/superdeck/lib/src/modules/deck/slide_configuration.mapper.dart
- packages/superdeck/lib/src/modules/export/export_controller.dart
- packages/superdeck/lib/src/modules/export/slide_capture_service.dart
- packages/superdeck/lib/src/modules/navigation/navigation_controller.dart
- packages/superdeck/lib/src/modules/navigation/navigation_provider.dart
- packages/superdeck/lib/src/modules/navigation/routes.dart
- packages/superdeck/lib/src/screens/export_screen.dart
- packages/superdeck/lib/styles/style_spec.dart
💤 Files with no reviewable changes (59)
- packages/superdeck/example/superdeck.yaml
- packages/superdeck/example/.superdeck/slides.json
- packages/superdeck/lib/components/molecules/exception_widget.dart
- packages/superdeck/lib/helpers/cache_memory_image.dart
- packages/superdeck/lib/providers/style_provider.dart
- packages/superdeck/lib/providers/assets_provider.dart
- packages/superdeck/lib/providers/examples_provider.dart
- melos.yaml
- packages/superdeck/lib/models/reference_model.dart
- packages/superdeck/lib/components/atoms/sized_transition.dart
- packages/superdeck/lib/helpers/dialog_page.dart
- packages/superdeck/lib/components/molecules/slide_content.dart
- packages/superdeck/lib/components/atoms/linear_progresss_indicator_widget.dart
- packages/superdeck/lib/models/config_model.dart
- packages/superdeck/lib/chat/components/typing_indicator.dart
- packages/superdeck/lib/providers/snapshot_provider.dart
- packages/superdeck/lib/components/atoms/slide_thumbnail.dart
- packages/superdeck/lib/models/asset_model.dart
- packages/superdeck/lib/components/atoms/slide_view.dart
- packages/superdeck/lib/components/superdeck_app.dart
- packages/superdeck/lib/helpers/value_notifiers.dart
- packages/superdeck/lib/helpers/extensions.dart
- packages/superdeck/lib/schema/schema_validation.dart
- packages/superdeck/lib/services/export_service.dart
- packages/superdeck/example/lib/src/widget/mix_demo.dart
- packages/superdeck/lib/components/atoms/transition_widget.dart
- packages/superdeck/lib/models/config_model.mapper.dart
- packages/superdeck/lib/models/asset_model.mapper.dart
- packages/superdeck/lib/schema/schema_values.dart
- packages/superdeck/lib/schema/schema_validation.mapper.dart
- packages/superdeck/lib/models/reference_model.mapper.dart
- packages/superdeck/lib/components/organisms/presentation_side_panel.dart
- packages/superdeck/lib/components/remix/button.dart
- packages/superdeck/lib/components/organisms/drawer.dart
- packages/superdeck/lib/components/atoms/markdown_viewer.dart
- packages/superdeck/lib/chat/chat_theme.dart
- packages/superdeck/lib/helpers/watcher.dart
- packages/superdeck/lib/models/slide_model.dart
- packages/superdeck/lib/services/reference_service.dart
- packages/superdeck/lib/screens/presentation_screen.dart
- packages/superdeck/lib/components/organisms/chat_panel.dart
- packages/superdeck/lib/components/molecules/code_preview.dart
- packages/superdeck/lib/helpers/section_tag.dart
- packages/superdeck/lib/screens/export_screen.dart
- packages/superdeck/lib/providers/controller.dart
- packages/superdeck/lib/components/organisms/app_shell.dart
- packages/superdeck/lib/helpers/routes.dart
- packages/superdeck/lib/helpers/universal.dart
- packages/superdeck/lib/chat/prompt.dart
- packages/superdeck/lib/components/molecules/split_view.dart
- packages/superdeck/lib/models/options_model.dart
- packages/superdeck/lib/components/molecules/navigation_rail.dart
- packages/superdeck/lib/components/atoms/cache_image_widget.dart
- packages/superdeck/lib/schema/schema_model.dart
- packages/superdeck/lib/providers/slide_provider.dart
- packages/superdeck/lib/helpers/measure_size.dart
- packages/superdeck/lib/helpers/hooks.dart
- packages/superdeck/lib/models/slide_model.mapper.dart
- packages/superdeck/lib/models/options_model.mapper.dart
✅ Files skipped from review due to trivial changes (7)
- .gitignore
- packages/superdeck/example/.metadata
- packages/superdeck/devtools_options.yaml
- packages/superdeck/example/.superdeck/generated_assets.json
- packages/superdeck/analysis_options.yaml
- packages/superdeck/.cursorignore
- packages/superdeck/.gitignore
🧰 Additional context used
🪛 LanguageTool
packages/superdeck/example/slides.md
[duplication] ~35-~35: Possible typo: you repeated a word.
Context: ... @column @column # Hi @column --- @section @section {flex: 2} @column { align: center f...
(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~273-~273: Possible typo: you repeated a word.
Context: ...ame of the palette - Font family - Font color - Color for each corner of the palette --- @...
(ENGLISH_WORD_REPEAT_RULE)
🔇 Additional comments (122)
packages/superdeck/example/pubspec_overrides.yaml (2)
1-1: Managed Dependency Overrides Update:The override comment now explicitly includes
superdeck_corealongsidesuperdeckandsuperdeck_cli. This is consistent with the refactoring and clearly communicates the additional managed dependency.
13-14: Addition of superdeck_core Override:A new dependency override for
superdeck_corehas been introduced with the path../../superdeck_core. Ensure that this relative path correctly resolves the intended package location, and that related configurations (e.g., in the workspace ormelos.yaml) are adjusted accordingly.packages/superdeck/example/linux/flutter/generated_plugins.cmake (1)
8-8: LGTM! Plugin name updated to follow platform-specific convention.The change from
screen_retrievertoscreen_retriever_linuxaligns with the platform-specific naming convention used by other plugins likefile_selector_linuxandurl_launcher_linux.packages/superdeck/example/windows/flutter/generated_plugins.cmake (1)
8-8: LGTM! Plugin name updated to follow platform-specific convention.The change from
screen_retrievertoscreen_retriever_windowsaligns with the platform-specific naming convention used by other plugins likefile_selector_windowsandurl_launcher_windows.packages/superdeck/example/windows/flutter/generated_plugin_registrant.cc (1)
11-11: LGTM! Header and registration updated for platform-specific implementation.The changes correctly update both the header path and registration function to use the Windows-specific implementation with C API.
Also applies to: 20-21
packages/superdeck/example/macos/Flutter/GeneratedPluginRegistrant.swift (1)
11-11: LGTM! Platform-specific updates and new webview plugin added.The changes include:
- Updated screen_retriever to use macOS-specific implementation
- Added webview_flutter_wkwebview plugin for macOS web view support
Also applies to: 14-14, 21-21, 24-24
packages/superdeck/example/linux/flutter/generated_plugin_registrant.cc (2)
11-11: LGTM! Header update aligns with platform-specific implementation.The update to use
screen_retriever_linux/screen_retriever_linux_plugin.hcorrectly reflects the move to platform-specific implementations.
22-24: LGTM! Plugin registration updated consistently.The registration code has been properly updated to use the Linux-specific plugin name and registration function, maintaining consistency with the platform-specific implementation pattern.
packages/superdeck/example/pubspec.yaml (8)
1-1: Consistent Quoting for Package NameThe package name is now enclosed in quotes. This improves YAML parsing consistency.
3-3: Unquoted Publish FieldThe
publish_tofield is now unquoted (set asnone), which is appropriate if you intend to disable publication. Please confirm this behavior aligns with your distribution strategy.
4-4: Quoted Version FieldThe version field is now quoted (
'1.0.0+1'), ensuring it’s treated as a string. This maintains consistency with best practices.
6-7: Updated Environment ConstraintsThe
environmentsection now specifiessdk: '^3.5.0'and addsflutter: '>=3.24.0'. These updates help enforce newer SDK features and Flutter compatibility.
11-13: Dependency Updates and Additions
- The
google_fontsdependency is updated to'^6.2.1', which should provide minor updates/bug fixes.- The new dependencies
mesh: '^0.4.1'andmix: '^1.5.0'have been added.Please verify that these new versions and dependencies are compatible with your codebase and that any required migration steps have been handled.
19-19: Updated Flutter Lints VersionThe
flutter_lintsversion is updated to'^4.0.0'. Ensure that your project's lint rules and analysis settings are adjusted accordingly if there are any breaking changes in the lint configurations.
21-21: Consistent Dependency Path FormattingThe path for
superdeck_clihas been reformatted (now set as'../../superdeck_cli'). Confirm that this relative path correctly resolves to the intended directory in your workspace.
25-25: Refined Assets Section
- The assets section now explicitly includes the directory
assets/and updates the previously listed.superdeck/generated/(implicitly removed) to.superdeck/assets/.- Retaining the
.superdeck/entry alongside these changes supports your asset organization strategy.Please verify that these asset paths correctly reflect your project’s file structure.
Also applies to: 27-27
packages/superdeck/example/lib/src/style.dart (2)
3-3: Import usage looks good.
No concerns with addingmixfor style composition.
46-67: Implementation ofCoverStyleappears consistent.
The approach of merging withsuper.build()is clear, and gradient usage is well-defined.packages/superdeck/example/.superdeck/superdeck.json (14)
1-2: Overall Structure and JSON Validity:
The JSON file appears to be well-structured and adheres to the expected format for slide configurations. The use of"slides"and"config"keys is consistent and clear.
71-103: Slide 'WyJ8NjmF' – Code Snippet and Image Integration:
The slide efficiently separates a Dart code snippet (line 84) and an image block (line 91) into two distinct blocks. Ensure that the formatting of the code snippet is rendered as expected by your markdown processor and that the image URL is correct.
104-129: Slide 'F2fTbXOG' – Image and Markdown Content:
The first block (line 111) includes markdown with an embedded image and a header, utilizing custom annotations such as{.code}and{.here}. Please verify that these annotations are correctly interpreted by the presentation renderer.
130-149: Slide '0zqy1l5c' – Dart Code Snippet Block:
A single block in this slide presents a Dart code snippet (lines 136–141). The snippet appears formatted correctly; however, please ensure it accurately reflects the intended widget usage in your application.
150-169: Slide 'RiDZbaFZ' – Unusual Dart Snippet Structure:
The Dart code snippet (line 157) uses.toList()on aColumnwidget, which is atypical. Please confirm whether this snippet is meant for demonstration purposes or if it requires correction to better reflect valid Dart/Flutter patterns.
170-203: Slide 'z34aal1W' – Mixed Media and DartPad Widget Integration:
This slide features a combination of an image block, a DartPad widget (with an"id": "idhere"at line 183), and markdown content. Verify that the DartPad widget configuration meets your requirements and that the layout balances the media and textual content appropriately.
232-264: Slide 'SJncL4H2' – Warning Message Formatting:
The warning message block (lines 245–250) uses markdown alert syntax ([!WARNING]), which seems well-formed. Ensure that your markdown renderer supports this syntax for proper visual emphasis on critical warnings.
298-317: Slide 'aTAXFyQ7' – Single-Block Highlight:
A single block communicates "What is Generative UI?" with a subtle animation marker ({.animate}). The simplicity here is effective; just ensure the animation marker is supported in your final environment.
371-390: Slide '3sLdrfsM' – Benefits Highlight:
The slide clearly outlines the benefits of UI over chat using markdown and a list structure. The content is well formatted and direct.
438-463: Slide '9y5hBeTm' – Structured Output Code Snippet:
The slide features a code snippet that outlines a schema definition. The snippet is well formatted; ensure that the schema details align with your overall documentation and expected functionality.
464-483: Slide 'CwxHOCpO' – Asynchronous Model Generation Example:
The Dart code snippet demonstrates model initialization and asynchronous content generation. Confirm that API key handling is secure and that appropriate error handling (if needed) is implemented in the actual application code.
484-503: Slide 'pukXIjvK' – Color Palette Generator Description:
The description clearly introduces the Color Palette Generator feature with detailed bullet points. The content is informative and aligns with its intended purpose.
504-523: Slide '14RbmSW5' – Schema Definition for Color Palette:
The Dart code snippet provided for schema definition is comprehensive and well-detailed. Consider reviewing the formatting to ensure readability on all devices and confirming that all property descriptions meet your project’s requirements.
524-553: Slide 'XeDZiCNk' – Widget Configuration for Color Palette:
This slide configures a widget for the color palette feature with a schema flag and a list of sample prompts. Verify that the widget correctly interprets the"schema": truesetting and that the prompts reflect intended use cases.packages/superdeck/example/macos/Runner.xcodeproj/project.pbxproj (19)
30-31: Add references to updated Pods frameworks.Everything looks consistent with the new framework references.
83-84: Add references to new release/profile config files.The references to
Pods-Runner.release.xcconfigandPods-RunnerTests.profile.xcconfigappear correct for CocoaPods integration.
85-86: Add references to new Pods_Runner.framework.No issues identified; the file reference seems in line with typical pod usage.
88-88: Add references to Pods-RunnerTests.release.xcconfig.Looks consistent with the multi-target setup for tests.
90-90: Add references to Pods-RunnerTests.debug.xcconfig.No issues identified; consistent with the newly added test config.
98-98: Add references to Pods_RunnerTests.framework in Frameworks.The new build file entry aligns with the renamed or updated test target framework.
106-106: Add references to Pods_Runner.framework in Frameworks.No concerns; framework references look properly updated.
140-140: Add new Pods group reference.The extra group for CocoaPods references aligns with standard Xcode project structure.
188-210: Introduce references for additional frameworks and xcconfig files.This large block establishes new Pods and Frameworks groups/configs. Everything seems to follow a typical CocoaPods project integration pattern.
218-218: Add new [CP] Check Pods Manifest.lock entry.Part of the standard CocoaPods build script for ensuring Podfile.lock sync. No issues found.
326-326: Add new [CP] Check Pods Manifest.lock shell script phase.Matches the usual pods verification script. Looks good.
341-341: Add output path for [CP] Check Pods Manifest.lock script.Ensures the check result is properly captured. No concerns.
386-386: Add [CP] Embed Pods Frameworks build phase.Universal step for embedding pods frameworks into the build. No issues identified.
403-403: Add another [CP] Check Pods Manifest.lock reference (test target).Ensures the test target also verifies the pods state. Looks consistent.
418-418: Add new output path for [CP] Check Pods Manifest.lock in test target.No concerns; consistent with the parallel pods check for RunnerTests.
476-476: Configure RunnerTests debug base configuration.The reference to
Pods-RunnerTests.debug.xcconfigis coherent with the newly introduced test config.
491-491: Link RunnerTests release base configuration to new xcconfig.No issues identified; the reference is properly updated for test release mode.
506-506: Link RunnerTests profile base configuration to new xcconfig.Configuration reference appears consistent and valid for the test profile build.
65-66: Confirm presence of new debug config file.Please ensure that the file
Pods-Runner.debug.xcconfigactually exists in the path “Target Support Files/Pods-Runner/”.❌ Verification inconclusive
🏁 Script executed:
#!/bin/bash # Checking for the newly introduced xcconfig file in the repository fd "Pods-Runner.debug.xcconfig"Length of output: 31
I'm not seeing any output from the initial search. To further verify whether the file exists anywhere in the repository under the expected name, please run the following script:
Once we have the result from this script, we can determine if the debug config file is actually present or if further actions are needed.
🏁 Script executed:
[run_scripts] #!/bin/bash # Searching for Pods-Runner.debug.xcconfig using the find command for a broader search scope find . -type f -name "Pods-Runner.debug.xcconfig"Length of output: 171
Missing Debug Config File in the Expected Location
It appears that the file
Pods-Runner.debug.xcconfigcannot be found in the repository under the expected path (i.e., "Target Support Files/Pods-Runner/"). Please verify that this file is present in version control and included in the project. Otherwise, the reference in the project.pbxproj will be invalid.
- Confirm that
Pods-Runner.debug.xcconfigis committed and located at the expected path.- If the file is missing, add it or update the reference accordingly.
packages/superdeck/lib/src/components/molecules/slide_screen.dart (4)
3-4: Imports look good.
No immediate issues with these new imports. They align with the recently introducedprovider.dartandslide_configuration.dartmodules.
7-9: Constructor refactoring appears consistent.
Renaming fromSlidePreviewtoSlideScreenand accepting aSlideConfigurationparameter directly is a clean approach.
13-13: Straightforward field addition.
Thefinal SlideConfiguration configuration;field clearly communicates the intention to store configuration data.
29-31: InheritedData usage is appropriate.
WrappingSlideViewwithInheritedDataensures that child or descendant widgets can easily access the configuration. This is a solid approach for data sharing in the subtree.packages/superdeck/lib/src/components/molecules/block_widget.dart (8)
1-8: Import statements look fine.
These imports correctly reference collection utilities, Flutter widgets, and custom modules.
9-11: Additional imports for helpers and styles.
No immediate issues with the new imports. They maintain consistency with the existing architecture.
17-26: Constructor is straightforward.
Accepting aSectionBlockandSizeis a clear API for rendering sections of blocks with a known layout size.
27-45: Debug info overlay.
The_renderDebugInfohelper method is a neat solution to visualize internal details. It's effectively gated in the parent build method, so it won't clutter production builds.
110-125: Abstract_BlockWidgetdesign.
Deriving fromStatefulWidgetwhile enforcing abuildcontract is well-structured. If no genuine local state is required, you could consider aStatelessWidgetapproach—but this design choice might be intentional for extension later.
127-195: Managing block data & style.
The_BlockWidgetStateapproach is thorough, applying styles, scrollers, and alignment. Good use ofInheritedDatafor block data propagation.
197-212:ColumnBlockWidget: Markdown rendering.
Simple usage ofMarkdownViewerensures consistent styling. Looks good.
238-290:_WidgetBlockWidget: Graceful failure.
The logic for a missing widget builder or runtime errors is handled nicely. Returning an error container helps with debugging.packages/superdeck/lib/src/modules/common/helpers/extensions.dart (1)
3-7: Theme-data extension is concise and effective.
Exposingtheme,textTheme, andcolorSchemefromBuildContextsimplifies theming calls. No concerns here.packages/superdeck/lib/src/modules/common/helpers/root_bundle_data_store.dart (1)
4-9: LGTM!The implementation is correct and follows best practices for handling asset bundle data in Flutter. The class properly extends
LocalDataStoreand thefileReadermethod is correctly implemented to handle asynchronous file reading from the asset bundle.packages/superdeck/lib/src/components/atoms/async_snapshot_widget.dart (1)
3-22: LGTM! Well-structured generic widget for handling async streams.The implementation follows Flutter's best practices for widget composition and provides a clean abstraction for handling asynchronous data streams.
packages/superdeck/lib/src/modules/common/helpers/measure_size.dart (1)
5-17: LGTM! Well-structured widget for size measurement.The implementation follows Flutter's best practices for widget composition and provides a clean API for size measurement.
packages/superdeck/example/lib/main.dart (1)
4-6: LGTM!The imports and initialization of
SuperDeckAppwith the new options structure look good. The code is well-organized and follows a modular approach.Also applies to: 17-37
packages/superdeck/lib/src/components/molecules/block_provider.dart (1)
49-67: LGTM!The
SectionDataclass is well-implemented with proper equality and hashCode implementations.packages/superdeck/lib/src/components/atoms/markdown_viewer.dart (2)
8-23: LGTM! Well-structured animated widget implementation.The class follows Flutter's widget patterns and properly extends ImplicitlyAnimatedWidget with appropriate default values for animation duration and curve.
47-71: LGTM! Comprehensive Markdown rendering implementation.The class effectively utilizes all available Markdown customization options from the flutter_markdown package, providing a robust rendering solution.
packages/superdeck/lib/src/components/organisms/keyboard_shortcuts.dart (1)
21-30: Apply the same improvements as NextSlideAction.The same concerns about error handling and return value documentation apply to this class.
packages/superdeck/lib/src/components/molecules/scaled_app.dart (1)
66-80: LGTM! Clean InheritedWidget implementation.The implementation follows Flutter's best practices for providing data to descendants.
packages/superdeck/lib/src/modules/common/helpers/converters.dart (1)
68-76: LGTM!The wrapper methods
toRowAlignmentandtoColumnAlignmentprovide a clean and intuitive API for common use cases.packages/superdeck/lib/src/components/atoms/slide_view.dart (2)
15-22: LGTM! Clean implementation of_renderPreferredSize.The method handles null cases gracefully and uses appropriate Flutter widgets for size management.
98-144: LGTM! Well-structured build method with clear layout management.The build method effectively manages the layout of different slide components using Positioned widgets within a Stack.
packages/superdeck/lib/src/components/atoms/loading_indicator.dart (2)
27-30: LGTM! Improved animation curve implementation.The change to use
CurvedAnimationwith separate curves for forward and reverse animations provides better control over the animation behavior.
171-176: LGTM! Good use of constants for magic numbers.The introduction of constants for
minY,maxY, andpathHeightimproves code readability and maintainability.packages/superdeck/build.yaml (2)
1-18: Review of Existing Builders Configuration
The existing builders (e.g.,dart_mappable_builder,mix_generator|spec,mix_generator|dto,mix_generator|enum_utility, andmix_generator|class_utility) remain unchanged and are configured correctly with their respectivegenerate_forrules.
19-21: New Builder: mix_generator|tokens Added
The new builder configuration formix_generator|tokensis added appropriately to target all Dart files in thelibdirectory. Make sure that its functionality is properly integrated with the rest of the system and that any references to the now-removedglobal_optionsare updated accordingly.packages/superdeck/assets/iframe_template.html (7)
1-6: HTML Structure & Meta Configuration
The document begins with a proper DOCTYPE declaration, language specification, and meta tags for character set and viewport settings. The title is succinct, and the overall HTML structure is clear.
7-20: Embedded CSS for Full-Viewport Layout
The CSS styles ensure that both thehtmlandbodyelements have no margin or padding and fill the viewport, while the iframe is set to occupy the full width and height without borders. The placeholder comment (/* CSS_PLACEHOLDER */) clearly indicates where future CSS customizations might go.
21-26: Function: loadURL
TheloadURL(url)function correctly sets thesrcattribute of the iframe with idcontent-frame. The implementation is straightforward and meets the expected use case.
61-71: Function: clearDartPadEditor
The implementation ofclearDartPadEditorleveragesexecuteInIframeto locate the DartPad editor (using optional chaining) and clear its content. This approach is clear and should work as expected.
73-85: Function: addToDartPadEditor
TheaddToDartPadEditorfunction correctly retrieves the current value of the DartPad editor and appends the new content with a line break. The usage ofJSON.stringifyensures that the provided content is safely embedded in the code executed in the iframe.
87-97: Function: setDartPadEditorContent
This function is well implemented: it replaces the editor’s content with the provided value, resets the cursor, and logs the action. The use ofJSON.stringifyagain provides safe encapsulation of the inserted content.
100-104: HTML Body and Iframe Integration
The<iframe>element with IDcontent-frameis set to load "https://dartpad.dev" by default, which aligns with the intended live demo/testing of DartPad functionality.packages/superdeck/example/slides.md (34)
1-2: Markdown Document Separator
The leading document separator (---) is correctly used to delineate the beginning of the slides content.
3-4: New Layout Directive: @column
The introduction of the@columndirective helps in establishing a modular layout. Ensure that the rendering logic tied to these directives in the application is updated and documented.
7-10: New Directive: @twitter Block
The
12-13: Additional @column Block
An additional@columndirective is present—ensure this is intentional and that multiple consecutive column declarations produce the desired layout.
14-20: Mermaid Diagram Block
The mermaid code block for a flowchart is well formatted and tagged with{.code}. Confirm that the render engine for Mermaid diagrams is correctly integrated so that these diagrams display as intended.
22-25: HTML Comment & Separator
The HTML comment (<!-- Test of notes -->) and subsequent separator are clear and help organize content.
26-31: Repeated @column Directives and Heading
There are several consecutive@columndirectives surrounding the heading "# Hi". Although multiple columns can be used to structure content side by side, please verify that this repetition is intentional. If not, consider consolidating redundant directives for clarity.
34-35: Document Separator Usage
The use of a separator (---) following the column block is appropriate for segmenting slide sections.🧰 Tools
38-41: Configured @column Block with Styling
This block uses@column { align: center, flex: 2 }to style the column. The configuration appears correct. Just ensure that the layout engine supports these style properties as specified.
44-56: Dart Code Block for Column Widget
The embedded Dart code (rendered in a code block tagged with{.code}) demonstrates a Flutter layout using aColumnandRichTextwidget. The code is clean and well formatted.
58-60: Image Rendering Directive with @column
The@columndirective followed by an image markdown (with class{.cover}) is consistent with the new layout structure. Confirm that the image URL and class styling work as expected on the target presentation renderer.
62-63: Section Separator
Another document separator is correctly used to mark a new slide section.
65-73: Mermaid Diagram Reuse
The subsequent mermaid diagram code block is almost identical to the earlier one. If this duplication is intentional to illustrate multiple slides or views of similar content, it is acceptable. Otherwise, consider reducing duplication.
75-80: Heading and Image Block
The heading "## Another image{.here}" along with a following@columndirective and an image markdown are clear. Verify that the class annotations (e.g.,{.here}and{.cover}) are handled correctly by the Markdown parser.
81-96: Embedded Dart Code Block (Complex Layout)
This Dart code block is well formatted and provides a concrete example of building a FlutterColumnwidget. The use of map and rich text rendering is clear. Ensure that the in-slide code examples remain in sync with any future API changes.
97-104: Additional Dart Code Example
Another Dart code block that creates a Flutter layout is provided. The syntax and structure are correct.
107-114: @dartpad Directive Integration
The new@dartpaddirective block (lines 111-114) defines an id and an empty code string. Verify that this directive triggers the intended DartPad integration and that providing an empty code string is handled gracefully by the application.
116-120: Heading for Differentiated Image Slide
The heading "## Yes different image{.here}" with an accompanying@columndirective is clear. This slide appears to target a variation in image presentation.
122-128: @column Block with Alignment for Personal Profile
The block that sets a column with{ align: center_right }followed by a heading and a username (with proper annotations) is structured well. Confirm that the custom alignment values (e.g.,center_right) are supported by your layout engine.
130-133: Additional @column with Opposite Alignment
The subsequent@column { align: center_left }directive appears to complement the previous layout by balancing content.
134-138: Personal Credentials List
The bullet list displaying credentials (Founder/CEO/CTO, Open Source Contributor, etc.) is formatted correctly.
141-149: Warning Slide with Dual Column Layout
This section uses two@columndirectives (one with{ align: center_left, flex: 2 }) followed by a blockquote that issues a warning. The layout is clear, but please ensure the blockquote formatting (using[!WARNING]) is rendered as intended.
150-152: Isolated @column and Slide Separator
The isolated@columndirective and subsequent document separator help delineate slide sections.
156-165: Slide Section on Generative UI
This block mixes column directives with headings such as "### Generative UI", "## VS", and "### AI Assisted Code Generation." The structure is creative, though confirm that the intended visual hierarchy is achieved with the mixed use of heading levels and column configurations.
171-177: Slide: What is Generative UI?
The slide introduces the concept with an animated heading and is followed by a list of descriptive bullet points. The content is clear and the@columndirective usage is consistent.
180-182: Slide: LLM ❤️ GUI
A standalone animated heading is used for impact. The minimalist slide appears to be used as a transition or thematic slide.
185-190: Slide with Combined Column Configurations and Descriptive Text
This section uses multiple column directives (one plain and one with{ flex: 2, align: center }) to center and emphasize a descriptive sentence about dynamic, context-aware UIs. The use of the.animateclass further adds visual dynamism.
196-201: Slide: Benefits of UI over Chat
The heading and associated bullet list succinctly communicate the advantages of UI interactions over plain chat interfaces.
206-213: Slide: Flutter and Generative UI
The block that describes Flutter’s suitability for generative UI uses a column directive with configuration (flex: 3, align: center) along with a heading. The HTML<br>tags for line breaks in the heading are acceptable.
216-221: Slide: How can LLMs Understand Your UI?
This slide mixes a heading with an image (referenced by a relative path). Verify that the asset path (assets/structured_output.png) is correct relative to the repository root and that the image displays at the intended resolution.
225-243: Slide: Structured Output Schema (Dart Code Block)
A Dart code block is provided that defines a schema for structured output. The schema declaration usingSchema.arrayand its nestedSchema.objectis clear. Ensure that this example correctly demonstrates the intended usage of the schema API.
247-263: Slide: Generative Model Example
This Dart code block shows the instantiation of a generative model and its configuration. The use of asynchronous operations (e.g.,await model.generateContent(...)) is correct. Confirm that the API keys and other sensitive details are managed securely in a real-world setting.
278-324: Dart Code Block: Color Palette Schema
This code block defines a schema for a color palette, including properties such asname,font,fontColor, and corner colors. The configuration is detailed and appears robust. Double-check that any enums (e.g., forfont) are defined with the expected values elsewhere in the codebase.
327-330: Slide Metadata: Style Definition
The linestyle: demofollowed by a document separator is used to set metadata for the slide. This is acceptable as long as the rendering engine recognizes this metadata block.packages/superdeck/example/lib/src/parts/background.dart (1)
35-48: LGTM!The widget implementation is clean, follows Flutter best practices, and correctly uses the configuration from context.
| { | ||
| "key": "Z40wIUYP", | ||
| "options": {}, | ||
| "sections": [ | ||
| { | ||
| "blocks": [ | ||
| { | ||
| "content": "## How can LLMs Understtand Your UI?\n\n", | ||
| "flex": 1, | ||
| "scrollable": false, | ||
| "type": "column" | ||
| } | ||
| ], | ||
| "flex": 1, | ||
| "scrollable": false, | ||
| "type": "section" | ||
| } | ||
| ], | ||
| "comments": [] | ||
| }, |
There was a problem hiding this comment.
Slide 'Z40wIUYP' – Typo Correction Needed:
There is a noticeable typo in the header (line 425): "Understtand" should be corrected to "Understand". Please apply the following change:
- "content": "## How can LLMs Understtand Your UI?\n\n",
+ "content": "## How can LLMs Understand Your UI?\n\n",📝 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.
| { | |
| "key": "Z40wIUYP", | |
| "options": {}, | |
| "sections": [ | |
| { | |
| "blocks": [ | |
| { | |
| "content": "## How can LLMs Understtand Your UI?\n\n", | |
| "flex": 1, | |
| "scrollable": false, | |
| "type": "column" | |
| } | |
| ], | |
| "flex": 1, | |
| "scrollable": false, | |
| "type": "section" | |
| } | |
| ], | |
| "comments": [] | |
| }, | |
| { | |
| "key": "Z40wIUYP", | |
| "options": {}, | |
| "sections": [ | |
| { | |
| "blocks": [ | |
| { | |
| "content": "## How can LLMs Understand Your UI?\n\n", | |
| "flex": 1, | |
| "scrollable": false, | |
| "type": "column" | |
| } | |
| ], | |
| "flex": 1, | |
| "scrollable": false, | |
| "type": "section" | |
| } | |
| ], | |
| "comments": [] | |
| }, |
| static Future<void> initialize() async { | ||
| await initializeDependencies(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling to initialization.
The initialize method should handle potential errors during dependency initialization to prevent app crashes.
static Future<void> initialize() async {
+ try {
await initializeDependencies();
+ } catch (e, stackTrace) {
+ debugPrint('Failed to initialize dependencies: $e\n$stackTrace');
+ rethrow;
+ }
}📝 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.
| static Future<void> initialize() async { | |
| await initializeDependencies(); | |
| } | |
| static Future<void> initialize() async { | |
| try { | |
| await initializeDependencies(); | |
| } catch (e, stackTrace) { | |
| debugPrint('Failed to initialize dependencies: $e\n$stackTrace'); | |
| rethrow; | |
| } | |
| } |
| class _MeasureSizeState extends State<MeasureSize> { | ||
| late final GlobalKey _widgetKey; | ||
| Size? _size; | ||
|
|
||
| @override | ||
| void initState() { | ||
| super.initState(); | ||
| _widgetKey = GlobalKey(debugLabel: 'MeasureSize'); | ||
| } | ||
|
|
||
| @override | ||
| Widget build(BuildContext context) { | ||
| WidgetsBinding.instance.addPostFrameCallback(_afterLayout); | ||
| return Container( | ||
| key: _widgetKey, | ||
| child: widget.child, | ||
| ); | ||
| } | ||
|
|
||
| void _afterLayout(Duration timeStamp) { | ||
| final context = _widgetKey.currentContext; | ||
| if (context == null) return; | ||
| final renderBox = context.findRenderObject() as RenderBox?; | ||
| if (_size == renderBox?.size) return; | ||
| if (renderBox != null && renderBox.hasSize) { | ||
| _size = renderBox.size; | ||
| widget.onChange(renderBox.size); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Optimize performance by reducing unnecessary post-frame callbacks.
The current implementation schedules a new post-frame callback on every build, which could impact performance. Consider optimizing by:
- Only scheduling callbacks when necessary
- Adding
didUpdateWidgetto handle size changes
@override
Widget build(BuildContext context) {
- WidgetsBinding.instance.addPostFrameCallback(_afterLayout);
+ // Schedule post-frame callback only once during initState
return Container(
key: _widgetKey,
child: widget.child,
);
}
+@override
+void initState() {
+ super.initState();
+ _widgetKey = GlobalKey(debugLabel: 'MeasureSize');
+ // Schedule initial measurement
+ WidgetsBinding.instance.addPostFrameCallback(_afterLayout);
+}
+
+@override
+void didUpdateWidget(MeasureSize oldWidget) {
+ super.didUpdateWidget(oldWidget);
+ // Schedule measurement when child widget changes
+ if (widget.child != oldWidget.child) {
+ WidgetsBinding.instance.addPostFrameCallback(_afterLayout);
+ }
+}📝 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.
| class _MeasureSizeState extends State<MeasureSize> { | |
| late final GlobalKey _widgetKey; | |
| Size? _size; | |
| @override | |
| void initState() { | |
| super.initState(); | |
| _widgetKey = GlobalKey(debugLabel: 'MeasureSize'); | |
| } | |
| @override | |
| Widget build(BuildContext context) { | |
| WidgetsBinding.instance.addPostFrameCallback(_afterLayout); | |
| return Container( | |
| key: _widgetKey, | |
| child: widget.child, | |
| ); | |
| } | |
| void _afterLayout(Duration timeStamp) { | |
| final context = _widgetKey.currentContext; | |
| if (context == null) return; | |
| final renderBox = context.findRenderObject() as RenderBox?; | |
| if (_size == renderBox?.size) return; | |
| if (renderBox != null && renderBox.hasSize) { | |
| _size = renderBox.size; | |
| widget.onChange(renderBox.size); | |
| } | |
| } | |
| } | |
| class _MeasureSizeState extends State<MeasureSize> { | |
| late final GlobalKey _widgetKey; | |
| Size? _size; | |
| @override | |
| void initState() { | |
| super.initState(); | |
| _widgetKey = GlobalKey(debugLabel: 'MeasureSize'); | |
| // Schedule initial measurement | |
| WidgetsBinding.instance.addPostFrameCallback(_afterLayout); | |
| } | |
| @override | |
| Widget build(BuildContext context) { | |
| // Schedule post-frame callback only once during initState | |
| return Container( | |
| key: _widgetKey, | |
| child: widget.child, | |
| ); | |
| } | |
| @override | |
| void didUpdateWidget(MeasureSize oldWidget) { | |
| super.didUpdateWidget(oldWidget); | |
| // Schedule measurement when child widget changes | |
| if (widget.child != oldWidget.child) { | |
| WidgetsBinding.instance.addPostFrameCallback(_afterLayout); | |
| } | |
| } | |
| void _afterLayout(Duration timeStamp) { | |
| final context = _widgetKey.currentContext; | |
| if (context == null) return; | |
| final renderBox = context.findRenderObject() as RenderBox?; | |
| if (_size == renderBox?.size) return; | |
| if (renderBox != null && renderBox.hasSize) { | |
| _size = renderBox.size; | |
| widget.onChange(renderBox.size); | |
| } | |
| } | |
| } |
| class _AnimatedLinearProgressIndicatorState | ||
| extends State<AnimatedLinearProgressIndicator> | ||
| with SingleTickerProviderStateMixin { | ||
| late AnimationController _animationController; | ||
| late Animation<double> _animation; | ||
|
|
||
| @override | ||
| void initState() { | ||
| super.initState(); | ||
| _animationController = AnimationController( | ||
| vsync: this, | ||
| duration: const Duration(milliseconds: 100), | ||
| ); | ||
| _animation = Tween<double>(begin: 0.0, end: widget.progress) | ||
| .animate(_animationController); | ||
| _animationController.forward(); | ||
| } | ||
|
|
||
| @override | ||
| void dispose() { | ||
| _animationController.dispose(); | ||
| super.dispose(); | ||
| } | ||
|
|
||
| @override | ||
| Widget build(BuildContext context) { | ||
| return AnimatedBuilder( | ||
| animation: _animation, | ||
| builder: (context, child) { | ||
| return LinearProgressIndicator( | ||
| minHeight: 10, | ||
| borderRadius: BorderRadius.circular(10), | ||
| value: _animation.value, | ||
| ); | ||
| }, | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add support for progress updates.
The widget doesn't animate when the progress value changes. Add didUpdateWidget to handle progress updates:
@override
void initState() {
super.initState();
_animationController = AnimationController(
vsync: this,
duration: const Duration(milliseconds: 100),
);
_animation = Tween<double>(begin: 0.0, end: widget.progress)
.animate(_animationController);
_animationController.forward();
}
+@override
+void didUpdateWidget(AnimatedLinearProgressIndicator oldWidget) {
+ super.didUpdateWidget(oldWidget);
+ if (widget.progress != oldWidget.progress) {
+ _animation = Tween<double>(
+ begin: oldWidget.progress,
+ end: widget.progress,
+ ).animate(_animationController);
+ _animationController
+ ..reset()
+ ..forward();
+ }
+}📝 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.
| class _AnimatedLinearProgressIndicatorState | |
| extends State<AnimatedLinearProgressIndicator> | |
| with SingleTickerProviderStateMixin { | |
| late AnimationController _animationController; | |
| late Animation<double> _animation; | |
| @override | |
| void initState() { | |
| super.initState(); | |
| _animationController = AnimationController( | |
| vsync: this, | |
| duration: const Duration(milliseconds: 100), | |
| ); | |
| _animation = Tween<double>(begin: 0.0, end: widget.progress) | |
| .animate(_animationController); | |
| _animationController.forward(); | |
| } | |
| @override | |
| void dispose() { | |
| _animationController.dispose(); | |
| super.dispose(); | |
| } | |
| @override | |
| Widget build(BuildContext context) { | |
| return AnimatedBuilder( | |
| animation: _animation, | |
| builder: (context, child) { | |
| return LinearProgressIndicator( | |
| minHeight: 10, | |
| borderRadius: BorderRadius.circular(10), | |
| value: _animation.value, | |
| ); | |
| }, | |
| ); | |
| } | |
| } | |
| class _AnimatedLinearProgressIndicatorState | |
| extends State<AnimatedLinearProgressIndicator> | |
| with SingleTickerProviderStateMixin { | |
| late AnimationController _animationController; | |
| late Animation<double> _animation; | |
| @override | |
| void initState() { | |
| super.initState(); | |
| _animationController = AnimationController( | |
| vsync: this, | |
| duration: const Duration(milliseconds: 100), | |
| ); | |
| _animation = Tween<double>(begin: 0.0, end: widget.progress) | |
| .animate(_animationController); | |
| _animationController.forward(); | |
| } | |
| @override | |
| void didUpdateWidget(AnimatedLinearProgressIndicator oldWidget) { | |
| super.didUpdateWidget(oldWidget); | |
| if (widget.progress != oldWidget.progress) { | |
| _animation = Tween<double>( | |
| begin: oldWidget.progress, | |
| end: widget.progress, | |
| ).animate(_animationController); | |
| _animationController | |
| ..reset() | |
| ..forward(); | |
| } | |
| } | |
| @override | |
| void dispose() { | |
| _animationController.dispose(); | |
| super.dispose(); | |
| } | |
| @override | |
| Widget build(BuildContext context) { | |
| return AnimatedBuilder( | |
| animation: _animation, | |
| builder: (context, child) { | |
| return LinearProgressIndicator( | |
| minHeight: 10, | |
| borderRadius: BorderRadius.circular(10), | |
| value: _animation.value, | |
| ); | |
| }, | |
| ); | |
| } | |
| } |
| class TwitterWidget extends StatelessWidget { | ||
| final String username; | ||
| final String tweetId; | ||
|
|
||
| const TwitterWidget( | ||
| {super.key, required this.username, required this.tweetId}); | ||
|
|
||
| @override | ||
| Widget build(BuildContext context) { | ||
| return Row( | ||
| children: [ | ||
| Container( | ||
| color: Colors.purple, | ||
| child: Text('Twitter: $username'), | ||
| ), | ||
| ], | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance TwitterWidget implementation.
The current implementation appears to be a placeholder:
- The
tweetIdparameter is not utilized. - The UI is basic and doesn't match typical Twitter embed styles.
Consider implementing a more complete Twitter embed widget:
class TwitterWidget extends StatelessWidget {
final String username;
final String tweetId;
const TwitterWidget({super.key, required this.username, required this.tweetId});
@override
Widget build(BuildContext context) {
- return Row(
- children: [
- Container(
- color: Colors.purple,
- child: Text('Twitter: $username'),
- ),
- ],
- );
+ return Card(
+ child: Padding(
+ padding: const EdgeInsets.all(16.0),
+ child: Column(
+ crossAxisAlignment: CrossAxisAlignment.start,
+ children: [
+ Row(
+ children: [
+ CircleAvatar(
+ child: Text(username[0].toUpperCase()),
+ ),
+ const SizedBox(width: 8),
+ Text(
+ '@$username',
+ style: Theme.of(context).textTheme.titleMedium,
+ ),
+ ],
+ ),
+ const SizedBox(height: 8),
+ // TODO: Implement tweet content using tweetId
+ Text('Tweet ID: $tweetId'),
+ ],
+ ),
+ ),
+ );📝 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.
| class TwitterWidget extends StatelessWidget { | |
| final String username; | |
| final String tweetId; | |
| const TwitterWidget( | |
| {super.key, required this.username, required this.tweetId}); | |
| @override | |
| Widget build(BuildContext context) { | |
| return Row( | |
| children: [ | |
| Container( | |
| color: Colors.purple, | |
| child: Text('Twitter: $username'), | |
| ), | |
| ], | |
| ); | |
| } | |
| } | |
| class TwitterWidget extends StatelessWidget { | |
| final String username; | |
| final String tweetId; | |
| const TwitterWidget( | |
| {super.key, required this.username, required this.tweetId}); | |
| @override | |
| Widget build(BuildContext context) { | |
| return Card( | |
| child: Padding( | |
| padding: const EdgeInsets.all(16.0), | |
| child: Column( | |
| crossAxisAlignment: CrossAxisAlignment.start, | |
| children: [ | |
| Row( | |
| children: [ | |
| CircleAvatar( | |
| child: Text(username[0].toUpperCase()), | |
| ), | |
| const SizedBox(width: 8), | |
| Text( | |
| '@$username', | |
| style: Theme.of(context).textTheme.titleMedium, | |
| ), | |
| ], | |
| ), | |
| const SizedBox(height: 8), | |
| // TODO: Implement tweet content using tweetId | |
| Text('Tweet ID: $tweetId'), | |
| ], | |
| ), | |
| ), | |
| ); | |
| } | |
| } |
| ImageProvider getImageProvider(Uri uri) { | ||
| switch (uri.scheme) { | ||
| case 'http': | ||
| case 'https': | ||
| return CachedNetworkImageProvider(uri.toString()); | ||
| default: | ||
| if (kCanRunProcess) { | ||
| return FileImage(File.fromUri(uri)); | ||
| } else { | ||
| return AssetImage(uri.path); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation for URIs and file paths.
The function should handle invalid URIs and validate file paths to prevent runtime errors.
ImageProvider getImageProvider(Uri uri) {
+ if (uri.toString().isEmpty) {
+ throw ArgumentError('URI cannot be empty');
+ }
+
switch (uri.scheme) {
case 'http':
case 'https':
+ if (!uri.hasAuthority) {
+ throw ArgumentError('Invalid HTTP(S) URI: ${uri.toString()}');
+ }
return CachedNetworkImageProvider(uri.toString());
default:
if (kCanRunProcess) {
+ final file = File.fromUri(uri);
+ if (!file.existsSync()) {
+ throw ArgumentError('File not found: ${uri.toString()}');
+ }
return FileImage(File.fromUri(uri));
} else {
+ if (uri.path.isEmpty) {
+ throw ArgumentError('Asset path cannot be empty');
+ }
return AssetImage(uri.path);
}
}
}📝 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.
| ImageProvider getImageProvider(Uri uri) { | |
| switch (uri.scheme) { | |
| case 'http': | |
| case 'https': | |
| return CachedNetworkImageProvider(uri.toString()); | |
| default: | |
| if (kCanRunProcess) { | |
| return FileImage(File.fromUri(uri)); | |
| } else { | |
| return AssetImage(uri.path); | |
| } | |
| } | |
| } | |
| ImageProvider getImageProvider(Uri uri) { | |
| if (uri.toString().isEmpty) { | |
| throw ArgumentError('URI cannot be empty'); | |
| } | |
| switch (uri.scheme) { | |
| case 'http': | |
| case 'https': | |
| if (!uri.hasAuthority) { | |
| throw ArgumentError('Invalid HTTP(S) URI: ${uri.toString()}'); | |
| } | |
| return CachedNetworkImageProvider(uri.toString()); | |
| default: | |
| if (kCanRunProcess) { | |
| final file = File.fromUri(uri); | |
| if (!file.existsSync()) { | |
| throw ArgumentError('File not found: ${uri.toString()}'); | |
| } | |
| return FileImage(File.fromUri(uri)); | |
| } else { | |
| if (uri.path.isEmpty) { | |
| throw ArgumentError('Asset path cannot be empty'); | |
| } | |
| return AssetImage(uri.path); | |
| } | |
| } | |
| } |
| (widget._uniqueKey).currentState?.dispose(); | ||
| _controller = WebViewController() | ||
| ..setJavaScriptMode(JavaScriptMode.unrestricted) | ||
| ..setNavigationDelegate( | ||
| NavigationDelegate( | ||
| onProgress: (int progress) {}, | ||
| onPageStarted: (String url) {}, | ||
| onPageFinished: (String url) {}, | ||
| onHttpError: (HttpResponseError error) {}, | ||
| onWebResourceError: (WebResourceError error) {}, | ||
| onNavigationRequest: (NavigationRequest request) { | ||
| return NavigationDecision.navigate; | ||
| }, | ||
| ), | ||
| ); | ||
|
|
||
| _controller.loadRequest(Uri.parse(widget.url)); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Implement error handlers for better error management and user feedback.
The navigation delegate has empty handlers for progress, errors, and page events. Consider implementing these handlers to provide better error management and user feedback.
..setNavigationDelegate(
NavigationDelegate(
- onProgress: (int progress) {},
+ onProgress: (int progress) {
+ // Update loading progress indicator
+ setState(() => _loadingProgress = progress);
+ },
- onPageStarted: (String url) {},
+ onPageStarted: (String url) {
+ setState(() => _isLoading = true);
+ },
- onPageFinished: (String url) {},
+ onPageFinished: (String url) {
+ setState(() => _isLoading = false);
+ },
- onHttpError: (HttpResponseError error) {},
+ onHttpError: (HttpResponseError error) {
+ _showError('HTTP Error: ${error.statusCode}');
+ },
- onWebResourceError: (WebResourceError error) {},
+ onWebResourceError: (WebResourceError error) {
+ _showError('Failed to load: ${error.description}');
+ },📝 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.
| (widget._uniqueKey).currentState?.dispose(); | |
| _controller = WebViewController() | |
| ..setJavaScriptMode(JavaScriptMode.unrestricted) | |
| ..setNavigationDelegate( | |
| NavigationDelegate( | |
| onProgress: (int progress) {}, | |
| onPageStarted: (String url) {}, | |
| onPageFinished: (String url) {}, | |
| onHttpError: (HttpResponseError error) {}, | |
| onWebResourceError: (WebResourceError error) {}, | |
| onNavigationRequest: (NavigationRequest request) { | |
| return NavigationDecision.navigate; | |
| }, | |
| ), | |
| ); | |
| _controller.loadRequest(Uri.parse(widget.url)); | |
| (widget._uniqueKey).currentState?.dispose(); | |
| _controller = WebViewController() | |
| ..setJavaScriptMode(JavaScriptMode.unrestricted) | |
| ..setNavigationDelegate( | |
| NavigationDelegate( | |
| onProgress: (int progress) { | |
| // Update loading progress indicator | |
| setState(() => _loadingProgress = progress); | |
| }, | |
| onPageStarted: (String url) { | |
| setState(() => _isLoading = true); | |
| }, | |
| onPageFinished: (String url) { | |
| setState(() => _isLoading = false); | |
| }, | |
| onHttpError: (HttpResponseError error) { | |
| _showError('HTTP Error: ${error.statusCode}'); | |
| }, | |
| onWebResourceError: (WebResourceError error) { | |
| _showError('Failed to load: ${error.description}'); | |
| }, | |
| onNavigationRequest: (NavigationRequest request) { | |
| return NavigationDecision.navigate; | |
| }, | |
| ), | |
| ); | |
| _controller.loadRequest(Uri.parse(widget.url)); |
| Future<void> clearDartPadEditor() { | ||
| _controller.reload(); | ||
| return executeInIframe(''' | ||
| var editor = document.querySelector('.CodeMirror')?.CodeMirror; | ||
| if (editor) { | ||
| editor.setValue(''); | ||
| editor.setCursor({line: 0, ch: 0}); | ||
| editor.focus(); | ||
| console.log('DartPad editor cleared!'); | ||
| } | ||
| '''); | ||
| } | ||
|
|
||
| // Function to set content in the DartPad editor | ||
| Future<void> setDartPadEditorContent(String content) { | ||
| return executeInIframe(''' | ||
| var editor = document.querySelector('.CodeMirror')?.CodeMirror; | ||
| if(editor){ | ||
| editor.setValue($content); | ||
| editor.setCursor(editor.lineCount(), 0); | ||
| editor.focus(); | ||
| console.log('DartPad editor content set!'); | ||
| } | ||
| '''); | ||
| } |
There was a problem hiding this comment.
Secure the JavaScript execution to prevent XSS attacks.
The methods clearDartPadEditor and setDartPadEditorContent directly inject content into JavaScript execution, which could be vulnerable to XSS attacks. Consider sanitizing the input and using template literals properly.
Future<void> setDartPadEditorContent(String content) {
+ // Sanitize content to prevent XSS
+ final sanitizedContent = jsonEncode(content);
return executeInIframe('''
var editor = document.querySelector('.CodeMirror')?.CodeMirror;
if(editor){
- editor.setValue($content);
+ editor.setValue($sanitizedContent);
editor.setCursor(editor.lineCount(), 0);
editor.focus();
console.log('DartPad editor content set!');
}
''');
}📝 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.
| Future<void> clearDartPadEditor() { | |
| _controller.reload(); | |
| return executeInIframe(''' | |
| var editor = document.querySelector('.CodeMirror')?.CodeMirror; | |
| if (editor) { | |
| editor.setValue(''); | |
| editor.setCursor({line: 0, ch: 0}); | |
| editor.focus(); | |
| console.log('DartPad editor cleared!'); | |
| } | |
| '''); | |
| } | |
| // Function to set content in the DartPad editor | |
| Future<void> setDartPadEditorContent(String content) { | |
| return executeInIframe(''' | |
| var editor = document.querySelector('.CodeMirror')?.CodeMirror; | |
| if(editor){ | |
| editor.setValue($content); | |
| editor.setCursor(editor.lineCount(), 0); | |
| editor.focus(); | |
| console.log('DartPad editor content set!'); | |
| } | |
| '''); | |
| } | |
| Future<void> clearDartPadEditor() { | |
| _controller.reload(); | |
| return executeInIframe(''' | |
| var editor = document.querySelector('.CodeMirror')?.CodeMirror; | |
| if (editor) { | |
| editor.setValue(''); | |
| editor.setCursor({line: 0, ch: 0}); | |
| editor.focus(); | |
| console.log('DartPad editor cleared!'); | |
| } | |
| '''); | |
| } | |
| // Function to set content in the DartPad editor | |
| Future<void> setDartPadEditorContent(String content) { | |
| // Sanitize content to prevent XSS | |
| final sanitizedContent = jsonEncode(content); | |
| return executeInIframe(''' | |
| var editor = document.querySelector('.CodeMirror')?.CodeMirror; | |
| if(editor){ | |
| editor.setValue($sanitizedContent); | |
| editor.setCursor(editor.lineCount(), 0); | |
| editor.focus(); | |
| console.log('DartPad editor content set!'); | |
| } | |
| '''); | |
| } |
| Future<File> _generateThumbnail({ | ||
| required SlideConfiguration slide, | ||
| required DeckController controller, | ||
| bool force = false, | ||
| }) async { | ||
| final thumbnailFile = controller.getSlideThumbnailFile(slide); | ||
|
|
||
| final isValid = | ||
| await thumbnailFile.exists() && (await thumbnailFile.length()) > 0; | ||
|
|
||
| if (isValid && !force) { | ||
| return thumbnailFile; | ||
| } | ||
|
|
||
| final imageData = await _slideCaptureService.capture( | ||
| slide: slide, | ||
| globalKey: _thumbnailKey, | ||
| ); | ||
|
|
||
| await thumbnailFile.writeAsBytes(imageData, flush: false); | ||
|
|
||
| return thumbnailFile; | ||
| } | ||
|
|
||
| // Future<void> _handleAction(_PopMenuAction action) async { | ||
| // switch (action) { | ||
| // case _PopMenuAction.refreshThumbnail: | ||
| // final thumbnailFile = _getThumbnailFile(widget.slide); | ||
|
|
||
| // if (await thumbnailFile.exists()) { | ||
| // await thumbnailFile.delete(); | ||
| // } | ||
| // await _thumbnailGeneration.refresh(); | ||
| // } | ||
| // } | ||
|
|
||
| Future<File> _load(BuildContext context) async { | ||
| final controller = DeckController.of(context); | ||
| return kCanRunProcess | ||
| ? await _generateThumbnail( | ||
| slide: widget.slideConfig, | ||
| controller: controller, | ||
| ) | ||
| : controller.getSlideThumbnailFile(widget.slideConfig); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider implementing error handling for thumbnail generation.
The _generateThumbnail method should handle potential errors during thumbnail generation and file operations.
Future<File> _generateThumbnail({
required SlideConfiguration slide,
required DeckController controller,
bool force = false,
}) async {
+ try {
final thumbnailFile = controller.getSlideThumbnailFile(slide);
final isValid =
await thumbnailFile.exists() && (await thumbnailFile.length()) > 0;
if (isValid && !force) {
return thumbnailFile;
}
final imageData = await _slideCaptureService.capture(
slide: slide,
globalKey: _thumbnailKey,
);
await thumbnailFile.writeAsBytes(imageData, flush: false);
return thumbnailFile;
+ } catch (e) {
+ throw ThumbnailGenerationException(
+ 'Failed to generate thumbnail: $e',
+ slide: slide,
+ );
+ }
}📝 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.
| Future<File> _generateThumbnail({ | |
| required SlideConfiguration slide, | |
| required DeckController controller, | |
| bool force = false, | |
| }) async { | |
| final thumbnailFile = controller.getSlideThumbnailFile(slide); | |
| final isValid = | |
| await thumbnailFile.exists() && (await thumbnailFile.length()) > 0; | |
| if (isValid && !force) { | |
| return thumbnailFile; | |
| } | |
| final imageData = await _slideCaptureService.capture( | |
| slide: slide, | |
| globalKey: _thumbnailKey, | |
| ); | |
| await thumbnailFile.writeAsBytes(imageData, flush: false); | |
| return thumbnailFile; | |
| } | |
| // Future<void> _handleAction(_PopMenuAction action) async { | |
| // switch (action) { | |
| // case _PopMenuAction.refreshThumbnail: | |
| // final thumbnailFile = _getThumbnailFile(widget.slide); | |
| // if (await thumbnailFile.exists()) { | |
| // await thumbnailFile.delete(); | |
| // } | |
| // await _thumbnailGeneration.refresh(); | |
| // } | |
| // } | |
| Future<File> _load(BuildContext context) async { | |
| final controller = DeckController.of(context); | |
| return kCanRunProcess | |
| ? await _generateThumbnail( | |
| slide: widget.slideConfig, | |
| controller: controller, | |
| ) | |
| : controller.getSlideThumbnailFile(widget.slideConfig); | |
| } | |
| Future<File> _generateThumbnail({ | |
| required SlideConfiguration slide, | |
| required DeckController controller, | |
| bool force = false, | |
| }) async { | |
| try { | |
| final thumbnailFile = controller.getSlideThumbnailFile(slide); | |
| final isValid = | |
| await thumbnailFile.exists() && (await thumbnailFile.length()) > 0; | |
| if (isValid && !force) { | |
| return thumbnailFile; | |
| } | |
| final imageData = await _slideCaptureService.capture( | |
| slide: slide, | |
| globalKey: _thumbnailKey, | |
| ); | |
| await thumbnailFile.writeAsBytes(imageData, flush: false); | |
| return thumbnailFile; | |
| } catch (e) { | |
| throw ThumbnailGenerationException( | |
| 'Failed to generate thumbnail: $e', | |
| slide: slide, | |
| ); | |
| } | |
| } |
| Color _colorFromHex(String hexString) { | ||
| hexString = hexString.trim(); | ||
| if (hexString.isEmpty) { | ||
| return Colors.black; // Default color if null or empty | ||
| } | ||
| hexString = hexString.replaceAll(RegExp(r'[^a-fA-F0-9]'), ''); | ||
| hexString = hexString.replaceAll('#', ''); | ||
| if (hexString.length == 6) { | ||
| hexString = 'FF$hexString'; // Add opacity if not provided | ||
| } | ||
| return Color(int.parse(hexString, radix: 16)); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation for hex string length.
The function should validate that the hex string length is either 6 or 8 characters after cleaning. Invalid lengths could cause runtime errors.
Apply this diff to add validation:
Color _colorFromHex(String hexString) {
hexString = hexString.trim();
if (hexString.isEmpty) {
return Colors.black; // Default color if null or empty
}
hexString = hexString.replaceAll(RegExp(r'[^a-fA-F0-9]'), '');
hexString = hexString.replaceAll('#', '');
+ if (hexString.length != 6 && hexString.length != 8) {
+ return Colors.black; // Default color if invalid format
+ }
if (hexString.length == 6) {
hexString = 'FF$hexString'; // Add opacity if not provided
}
return Color(int.parse(hexString, radix: 16));
}📝 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.
| Color _colorFromHex(String hexString) { | |
| hexString = hexString.trim(); | |
| if (hexString.isEmpty) { | |
| return Colors.black; // Default color if null or empty | |
| } | |
| hexString = hexString.replaceAll(RegExp(r'[^a-fA-F0-9]'), ''); | |
| hexString = hexString.replaceAll('#', ''); | |
| if (hexString.length == 6) { | |
| hexString = 'FF$hexString'; // Add opacity if not provided | |
| } | |
| return Color(int.parse(hexString, radix: 16)); | |
| } | |
| Color _colorFromHex(String hexString) { | |
| hexString = hexString.trim(); | |
| if (hexString.isEmpty) { | |
| return Colors.black; // Default color if null or empty | |
| } | |
| hexString = hexString.replaceAll(RegExp(r'[^a-fA-F0-9]'), ''); | |
| hexString = hexString.replaceAll('#', ''); | |
| if (hexString.length != 6 && hexString.length != 8) { | |
| return Colors.black; // Default color if invalid format | |
| } | |
| if (hexString.length == 6) { | |
| hexString = 'FF$hexString'; // Add opacity if not provided | |
| } | |
| return Color(int.parse(hexString, radix: 16)); | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/test.yml (1)
23-26: Analyzer Step Configuration Check
The analyzer step usinginvertase/github-action-dart-analyzer@v1withfatal-infos: falseis now placed immediately after the Melos setup. Please confirm that running static analysis at this point (i.e., before installing dependencies) fits your intended workflow. Additionally, note that a similar analyzer step appears later; verify that both are necessary or if consolidating them could streamline the process.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (1)
.github/workflows/test.yml(1 hunks)
🔇 Additional comments (3)
.github/workflows/test.yml (3)
20-22: New Melos Setup Step Added
This new step integrates Melos into your workflow usingbluefireteam/melos-action@v3. It aligns with the project's shift toward centralized package management for building and testing. Please verify that yourmelos.yamlconfiguration is updated accordingly if needed.
31-32: Updated Build Runner Command
The build command has been updated to usemelos run gen:buildinstead of the previous build_runner invocation. This change aligns with the new integration of Melos. Please ensure that this change triggers the expected code generation and that all downstream processes are compatible with this new command.
37-38: Updated Test Command
The test step now runsmelos run testinstead offlutter test, consolidating test execution through Melos. This update supports the overall workflow refactor. Verify that all test suites execute as expected under this adjusted command.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (8)
.github/workflows/test.yml (4)
17-22: FVM Installation Step
The new step installs FVM and forces the use of the stable Flutter version. Please verify that FVM is added to the system PATH post-installation if subsequent steps depend on it.
26-30: Flutter Action with FVM Outputs
This step leverages the outputs generated by the FVM config action to set up Flutter throughsubosito/flutter-action@v2. Double-check that the dynamic values forflutter-versionandchannelmatch your expectations and that they integrate smoothly with the rest of your workflow.
34-36: Pre-Build Dart Analyzer Step – Potential Duplicate?
There is an analyzer step configured here withfatal-infosset to false. Since a similar analyzer step is executed later (lines 44-47), please double-check whether both instances are required. Running the analyzer before and after the build might be intentional; if not, consider consolidating to a single post-build analysis for efficiency.
44-47: Post-Build Dart Analyzer Step
The analyzer step after the build phase ensures that any generated code is compliant with your analysis rules. As noted earlier, verify the necessity of running the analyzer both before and after the build; if redundancy exists, consider removing the earlier instance to streamline workflow execution.packages/superdeck/lib/src/screens/export_screen.dart (3)
19-25: Add error handling to the show method.The
showmethod could fail ifDeckControlleris not found in the widget tree. Consider adding error handling to provide a better user experience.static void show(BuildContext context) { - final deckController = DeckController.of(context); - showDialog( - context: context, - builder: (context) => ExportDialogScreen(slides: deckController.slides), - ); + try { + final deckController = DeckController.of(context); + showDialog( + context: context, + builder: (context) => ExportDialogScreen(slides: deckController.slides), + ); + } catch (e) { + ScaffoldMessenger.of(context).showSnackBar( + SnackBar(content: Text('Failed to open export dialog: $e')), + ); + } }
74-117: Consider screen size constraints in the dialog layout.The dialog uses
SizedBox.fromSizewith a fixed size (kResolution), which might not fit on smaller screens. Consider making the dialog size responsive.return Dialog( insetPadding: const EdgeInsets.all(0), - child: SizedBox.fromSize( - size: kResolution, + child: LayoutBuilder( + builder: (context, constraints) { + final maxSize = Size( + constraints.maxWidth, + constraints.maxHeight, + ); + final size = kResolution.width > maxSize.width || + kResolution.height > maxSize.height + ? Size( + min(kResolution.width, maxSize.width), + min(kResolution.height, maxSize.height), + ) + : kResolution; + return SizedBox.fromSize( + size: size,
121-166: Add accessibility support for the export progress indicator.The progress indicator should have semantic labels to support screen readers.
child: Row( mainAxisSize: MainAxisSize.min, children: [ exportController.exportStatus == ExportStatus.complete ? Icon( Icons.check_circle, color: Theme.of(context).colorScheme.primary, size: 32, + semanticsLabel: 'Export completed', ) : SizedBox( height: 32, width: 32, - child: CircularProgressIndicator( - color: Theme.of(context).colorScheme.primary, - value: - exportController.exportStatus == ExportStatus.building - ? null - : exportController.progress, + child: Semantics( + label: 'Export progress', + value: '${(exportController.progress * 100).toInt()}%', + child: CircularProgressIndicator( + color: Theme.of(context).colorScheme.primary, + value: + exportController.exportStatus == ExportStatus.building + ? null + : exportController.progress, + ), ), ),packages/superdeck/example/lib/src/style.dart (1)
69-96: Extract shared gradient logic and update deprecated methods.
- The gradient configuration is duplicated from
CoverStyle. Consider extracting it to a shared method.- Replace deprecated
withOpacitycalls withwithValues.+ static Style get darkGradientBackground => Style( + $.blockContainer.gradient.linear( + begin: Alignment.topCenter, + end: Alignment.bottomCenter, + colors: [ + Colors.black.withValues(red: 0, green: 0, blue: 0, opacity: 0.5), + Colors.black.withValues(red: 0, green: 0, blue: 0, opacity: 0.95), + ], + ), + ); class AnnouncementStyle extends DeckStyle { AnnouncementStyle(); @override Style build() { return super.build().merge( Style( $.baseTextStyle.height(0.6), $.h1.chain ..style.fontSize(140) ..style.bold() ..style.color(const Color.fromARGB(255, 201, 195, 139)), $.h2.style.fontSize(140), $.h3.chain ..style.fontSize(60) ..style.color(Colors.white) ..style.fontWeight(FontWeight.w100), - $.blockContainer.gradient.linear( - begin: Alignment.topCenter, - end: Alignment.bottomCenter, - colors: [ - Colors.black.withOpacity(0.5), - Colors.black.withOpacity(0.95), - ], - ), + darkGradientBackground, ), ); } }🧰 Tools
🪛 GitHub Check: Test
[notice] 89-89: 'withOpacity' is deprecated and shouldn't be used. Use .withValues() to avoid precision loss.
Try replacing the use of the deprecated member with the replacement.
See https://dart.dev/diagnostics/deprecated_member_use to learn more about this problem.
[notice] 90-90: 'withOpacity' is deprecated and shouldn't be used. Use .withValues() to avoid precision loss.
Try replacing the use of the deprecated member with the replacement.
See https://dart.dev/diagnostics/deprecated_member_use to learn more about this problem.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/workflows/test.yml(1 hunks)packages/superdeck/example/lib/src/parts/header.dart(1 hunks)packages/superdeck/example/lib/src/style.dart(1 hunks)packages/superdeck/example/pubspec_overrides.yaml(0 hunks)packages/superdeck/lib/src/components/molecules/block_widget.dart(1 hunks)packages/superdeck/lib/src/components/molecules/bottom_bar.dart(1 hunks)packages/superdeck/lib/src/screens/export_screen.dart(1 hunks)
💤 Files with no reviewable changes (1)
- packages/superdeck/example/pubspec_overrides.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/superdeck/lib/src/components/molecules/bottom_bar.dart
- packages/superdeck/lib/src/components/molecules/block_widget.dart
🧰 Additional context used
🪛 GitHub Check: Test
packages/superdeck/example/lib/src/style.dart
[notice] 60-60: 'withOpacity' is deprecated and shouldn't be used. Use .withValues() to avoid precision loss.
Try replacing the use of the deprecated member with the replacement.
See https://dart.dev/diagnostics/deprecated_member_use to learn more about this problem.
[notice] 61-61: 'withOpacity' is deprecated and shouldn't be used. Use .withValues() to avoid precision loss.
Try replacing the use of the deprecated member with the replacement.
See https://dart.dev/diagnostics/deprecated_member_use to learn more about this problem.
[notice] 89-89: 'withOpacity' is deprecated and shouldn't be used. Use .withValues() to avoid precision loss.
Try replacing the use of the deprecated member with the replacement.
See https://dart.dev/diagnostics/deprecated_member_use to learn more about this problem.
[notice] 90-90: 'withOpacity' is deprecated and shouldn't be used. Use .withValues() to avoid precision loss.
Try replacing the use of the deprecated member with the replacement.
See https://dart.dev/diagnostics/deprecated_member_use to learn more about this problem.
[warning] 6-6: The declaration '_accent' isn't referenced.
Try removing the declaration of '_accent'.
See https://dart.dev/diagnostics/unused_element to learn more about this problem.
packages/superdeck/example/lib/src/parts/header.dart
[failure] 22-22: The getter 'title' isn't defined for the type 'SlideOptions'.
Try importing the library that defines 'title', correcting the name to the name of an existing getter, or defining a getter or field named 'title'.
See https://dart.dev/diagnostics/undefined_getter to learn more about this problem.
🪛 GitHub Actions: Test Workflow
packages/superdeck/example/lib/src/parts/header.dart
[error] 22-22: The getter 'title' isn't defined for the type 'SlideOptions'. Try importing the library that defines 'title', correcting the name to the name of an existing getter, or defining a getter or field named 'title'.
🔇 Additional comments (11)
.github/workflows/test.yml (4)
23-25: FVM Config Action Setup
The integration of thekuhnroyal/flutter-fvm-config-action@v2correctly assigns an ID for subsequent reference. Ensure that the outputs (FLUTTER_VERSIONandFLUTTER_CHANNEL) are accurately produced and utilized in later steps.
31-33: Melos Setup Integration
The addition of the Melos setup step viabluefireteam/melos-action@v3appears well integrated. Confirm that this version aligns with your workspace requirements and that it doesn’t interfere with any caching or dependency management strategies.
41-43: Build Command Update to Melos
Replacing the legacybuild_runnercommand withmelos run gen:buildis a good alignment with the project’s Melos-based management. Ensure that this new command reliably generates all expected artifacts and that any downstream dependencies are updated accordingly.
48-48: Test Command Update to Melos
Updating the test command tomelos run testis consistent with the overall Melos-driven refactor. Make sure that this command aggregates test reports as expected and that there are no discrepancies compared to the previousflutter testexecution.packages/superdeck/lib/src/screens/export_screen.dart (1)
1-10: LGTM!The imports are well-organized and appropriate for the functionality being implemented.
packages/superdeck/example/lib/src/style.dart (2)
8-44: Verify if empty style configuration is intentional.The
BaseStyleclass currently returns an empty style configuration as all style properties are commented out. This might lead to unexpected behavior if base styles are required.Please confirm if this is intentional or if the commented styles should be uncommented.
98-117: LGTM!The
QuoteStyleimplementation is clean and well-structured, with appropriate typography and styling choices.packages/superdeck/example/lib/src/parts/header.dart (4)
1-7: LGTM!The class declaration and imports are well-structured. Good use of PreferredSizeWidget for a header component.
9-10: LGTM!The preferredSize implementation is appropriate for a header component.
12-15: LGTM!The build method initialization and SlideConfiguration retrieval are well-implemented.
16-28:⚠️ Potential issueFix undefined 'title' getter in SlideOptions.
The static analysis tools and pipeline failures indicate that the 'title' getter is not defined in the SlideOptions class.
Let's verify the SlideOptions interface:
Please ensure one of the following:
- Add the 'title' property to the SlideOptions class
- Use an existing property name instead of 'title'
- Update the property access to match the correct API
Example fix if adding the title property:
class SlideOptions { final String? title; const SlideOptions({ this.title, }); }❌ Verification inconclusive
🏁 Script executed:
#!/bin/bash # Search for SlideOptions class definition ast-grep --pattern 'class SlideOptions { $$$ }'Length of output: 51
Let's further verify the SlideOptions interface since the initial AST grep didn't return any results. We'll run an additional search using ripgrep to determine where (and how) SlideOptions is defined. This will help us confirm if the 'title' property exists or if it needs to be added/updated.
Once we have the output from this command, we can confirm whether:
- The SlideOptions class is defined and, if so, whether it includes a 'title' getter.
- Or, if it’s absent, the 'title' property needs to be added as suggested in the review comment, or the widget’s property access updated to use an existing property name.
Please share the results of this command for further verification.
🏁 Script executed:
#!/bin/bash # Searching for every occurrence of 'class SlideOptions' in the repository. rg --heading --line-number "class SlideOptions" .Length of output: 370
Let's inspect the contents of the
SlideOptionsclass in the filepackages/superdeck_core/lib/src/models/slide_model.dartto verify if it defines atitleproperty. This will help us decide whether to add the missing getter or update its usage in the widget tree.Please share the output of this command for further verification.
🏁 Script executed:
#!/bin/bash sed -n '35,120p' packages/superdeck_core/lib/src/models/slide_model.dartLength of output: 647
SlideOptions already defines the 'title' property.
We verified that in
packages/superdeck_core/lib/src/models/slide_model.darttheSlideOptionsclass correctly includes a nullabletitleproperty. The static analysis error regarding an undefined getter appears to be a false positive or due to an import/configuration issue elsewhere, so no changes to theSlideOptionsinterface are required.Likely an incorrect or invalid review comment.
🧰 Tools
🪛 GitHub Check: Test
[failure] 22-22: The getter 'title' isn't defined for the type 'SlideOptions'.
Try importing the library that defines 'title', correcting the name to the name of an existing getter, or defining a getter or field named 'title'.
See https://dart.dev/diagnostics/undefined_getter to learn more about this problem.🪛 GitHub Actions: Test Workflow
[error] 22-22: The getter 'title' isn't defined for the type 'SlideOptions'. Try importing the library that defines 'title', correcting the name to the name of an existing getter, or defining a getter or field named 'title'.
| Future<void> _handleExport() async { | ||
| try { | ||
| await _exportController.export(); | ||
| } finally { | ||
| if (context.mounted) { | ||
| // ignore: use_build_context_synchronously | ||
| Navigator.of(context).pop(); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling and user feedback for export failures.
The _handleExport method should handle errors and provide feedback to the user when the export fails.
Future<void> _handleExport() async {
try {
await _exportController.export();
+ } catch (e) {
+ if (context.mounted) {
+ ScaffoldMessenger.of(context).showSnackBar(
+ SnackBar(content: Text('Export failed: $e')),
+ );
+ }
} finally {
if (context.mounted) {
// ignore: use_build_context_synchronously
Navigator.of(context).pop();
}
}
}📝 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.
| Future<void> _handleExport() async { | |
| try { | |
| await _exportController.export(); | |
| } finally { | |
| if (context.mounted) { | |
| // ignore: use_build_context_synchronously | |
| Navigator.of(context).pop(); | |
| } | |
| } | |
| } | |
| Future<void> _handleExport() async { | |
| try { | |
| await _exportController.export(); | |
| } catch (e) { | |
| if (context.mounted) { | |
| ScaffoldMessenger.of(context).showSnackBar( | |
| SnackBar(content: Text('Export failed: $e')), | |
| ); | |
| } | |
| } finally { | |
| if (context.mounted) { | |
| // ignore: use_build_context_synchronously | |
| Navigator.of(context).pop(); | |
| } | |
| } | |
| } |
| class CoverStyle extends DeckStyle { | ||
| CoverStyle(); | ||
|
|
||
| Style get quoteStyle { | ||
| return Style( | ||
| _blockquote.textStyle.as(GoogleFonts.notoSerif()), | ||
| _blockquote.decoration.border.left( | ||
| width: 4, | ||
| color: Colors.red, | ||
| ), | ||
| _paragraph.textStyle.fontSize(32), | ||
| _h6.textStyle.as(GoogleFonts.notoSerif()), | ||
| _h6.textStyle.fontSize(20), | ||
| ); | ||
| @override | ||
| Style build() { | ||
| return super.build().merge( | ||
| Style( | ||
| $.h1.chain | ||
| ..style.as(GoogleFonts.poppins()) | ||
| ..style.fontSize(100), | ||
| $.blockContainer.gradient.linear( | ||
| begin: Alignment.topCenter, | ||
| end: Alignment.bottomCenter, | ||
| colors: [ | ||
| Colors.black.withOpacity(0.5), | ||
| Colors.black.withOpacity(0.95), | ||
| ], | ||
| ), | ||
| ), | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace deprecated withOpacity with withValues.
The withOpacity method is deprecated. Use withValues instead to avoid precision loss.
- Colors.black.withOpacity(0.5),
- Colors.black.withOpacity(0.95),
+ Colors.black.withValues(red: 0, green: 0, blue: 0, opacity: 0.5),
+ Colors.black.withValues(red: 0, green: 0, blue: 0, opacity: 0.95),📝 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.
| class CoverStyle extends DeckStyle { | |
| CoverStyle(); | |
| Style get quoteStyle { | |
| return Style( | |
| _blockquote.textStyle.as(GoogleFonts.notoSerif()), | |
| _blockquote.decoration.border.left( | |
| width: 4, | |
| color: Colors.red, | |
| ), | |
| _paragraph.textStyle.fontSize(32), | |
| _h6.textStyle.as(GoogleFonts.notoSerif()), | |
| _h6.textStyle.fontSize(20), | |
| ); | |
| @override | |
| Style build() { | |
| return super.build().merge( | |
| Style( | |
| $.h1.chain | |
| ..style.as(GoogleFonts.poppins()) | |
| ..style.fontSize(100), | |
| $.blockContainer.gradient.linear( | |
| begin: Alignment.topCenter, | |
| end: Alignment.bottomCenter, | |
| colors: [ | |
| Colors.black.withOpacity(0.5), | |
| Colors.black.withOpacity(0.95), | |
| ], | |
| ), | |
| ), | |
| ); | |
| } | |
| } | |
| class CoverStyle extends DeckStyle { | |
| CoverStyle(); | |
| @override | |
| Style build() { | |
| return super.build().merge( | |
| Style( | |
| $.h1.chain | |
| ..style.as(GoogleFonts.poppins()) | |
| ..style.fontSize(100), | |
| $.blockContainer.gradient.linear( | |
| begin: Alignment.topCenter, | |
| end: Alignment.bottomCenter, | |
| colors: [ | |
| Colors.black.withValues(red: 0, green: 0, blue: 0, opacity: 0.5), | |
| Colors.black.withValues(red: 0, green: 0, blue: 0, opacity: 0.95), | |
| ], | |
| ), | |
| ), | |
| ); | |
| } | |
| } |
🧰 Tools
🪛 GitHub Check: Test
[notice] 60-60: 'withOpacity' is deprecated and shouldn't be used. Use .withValues() to avoid precision loss.
Try replacing the use of the deprecated member with the replacement.
See https://dart.dev/diagnostics/deprecated_member_use to learn more about this problem.
[notice] 61-61: 'withOpacity' is deprecated and shouldn't be used. Use .withValues() to avoid precision loss.
Try replacing the use of the deprecated member with the replacement.
See https://dart.dev/diagnostics/deprecated_member_use to learn more about this problem.
| final _h1 = _util.h1; | ||
| final _h2 = _util.h2; | ||
| final _h3 = _util.h3; | ||
| const _accent = Color(0xff73F8FB); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove unused constant.
The _accent color constant is not referenced in the active code. All its usages are commented out in the BaseStyle class.
-const _accent = Color(0xff73F8FB);📝 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.
| const _accent = Color(0xff73F8FB); |
🧰 Tools
🪛 GitHub Check: Test
[warning] 6-6: The declaration '_accent' isn't referenced.
Try removing the declaration of '_accent'.
See https://dart.dev/diagnostics/unused_element to learn more about this problem.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
packages/superdeck/lib/src/modules/common/markdown/builders/code_element_builder.dart (4)
13-17: Consider making the constructor parameter named for clarity.By converting
CodeElementBuilder(this.spec);to a named parameter, you can improve readability, especially if more parameters are added in the future.Here's a possible diff:
-class CodeElementBuilder extends MarkdownElementBuilder { - final MarkdownCodeblockSpec? spec; - CodeElementBuilder(this.spec); +class CodeElementBuilder extends MarkdownElementBuilder { + final MarkdownCodeblockSpec? spec; + + CodeElementBuilder({this.spec});
20-29: Handle complex language class attributes.Right now, you're only handling
'language-xxx'prefixes. If there are multiple classes, or classes that embed the language differently, the language extraction logic might fail. You might consider a more robust parsing approach to avoid missing edge cases.Would you like a follow-up implementation snippet that handles multiple or nested class attributes more reliably?
39-59: Consider using a single RichText or a scrollable layout for large code blocks.Building a new
RichTextwidget for each span can incur overhead for lengthy code blocks. If performance or memory usage becomes a concern, a singleRichTextwith multiple spans might be more efficient. Alternatively, a scrollable layout (e.g.,SingleChildScrollView) could provide a better user experience for large code blocks.- Column( - crossAxisAlignment: CrossAxisAlignment.start, - children: spans.map((span) { - return RichText( - text: TextSpan( - style: spec.textStyle, - children: [span], - ), - ); - }).toList(), - ), + SingleChildScrollView( + child: RichText( + text: TextSpan( + style: spec.textStyle, + children: spans, + ), + ), + ),
93-179: Ensure large code-block hero animations remain performant.Your hero animation involves rendering potentially large code blocks in
_CodeElementHero. For extremely large content, the layout and animation could be expensive. Consider using a scrollable container or dynamic clipping to optimize performance during the hero flight.packages/superdeck/example/macos/Flutter/GeneratedPluginRegistrant.swift (1)
19-26: Validate new plugin registrations.The new plugin registrations (
FilePickerPlugin,ScreenRetrieverMacosPlugin, andWebViewFlutterPlugin) look correct. Just confirm that each plugin is indeed required and utilized in the macOS build to avoid extraneous bundle size increases or potential runtime initialization overhead.packages/superdeck/lib/src/modules/common/initializer_provider.dart (1)
25-25: Remove commented code.Remove the commented line as it's not being used.
- // final titleBarHeight = await windowManager.getTitleBarHeight();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
packages/superdeck/example/macos/Podfile.lockis excluded by!**/*.lockpubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
melos.yaml(2 hunks)packages/superdeck/example/.gitignore(1 hunks)packages/superdeck/example/.superdeck/generated_assets.json(1 hunks)packages/superdeck/example/.superdeck/superdeck.json(1 hunks)packages/superdeck/example/macos/Flutter/GeneratedPluginRegistrant.swift(1 hunks)packages/superdeck/example/macos/Runner/AppDelegate.swift(1 hunks)packages/superdeck/example/pubspec.yaml(1 hunks)packages/superdeck/example/pubspec_overrides.yaml(1 hunks)packages/superdeck/lib/src/modules/common/initializer_provider.dart(1 hunks)packages/superdeck/lib/src/modules/common/markdown/builders/code_element_builder.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/superdeck/example/.superdeck/generated_assets.json
- packages/superdeck/example/pubspec_overrides.yaml
- packages/superdeck/example/.superdeck/superdeck.json
- packages/superdeck/example/pubspec.yaml
🔇 Additional comments (5)
packages/superdeck/example/.gitignore (1)
8-8: LGTM! Good additions to .gitignore.The new entries
.build/and.swiftpm/are appropriate additions that follow best practices by excluding build artifacts and Swift Package Manager metadata from version control.Also applies to: 12-12
packages/superdeck/example/macos/Runner/AppDelegate.swift (1)
10-12: Check for additional state restoration logic.Returning
trueis generally sufficient to declare secure restorable state support. Nonetheless, if you have custom restoration data or logic, ensure that the state restoration callbacks are thoroughly tested and secure.packages/superdeck/example/macos/Flutter/GeneratedPluginRegistrant.swift (1)
8-15: Confirm plugin import consistency.Adding these macOS-specific imports (e.g.,
screen_retriever_macos,sqflite_darwin,webview_flutter_wkwebview, etc.) ensures platform-appropriate functionality. Verify that none of these imports conflict with iOS or other platforms if you plan to share code across them.packages/superdeck/lib/src/modules/common/initializer_provider.dart (1)
9-17: LGTM! Well-structured initialization logic.The function efficiently initializes multiple dependencies concurrently using
Future.waitafter ensuring Flutter bindings are initialized.melos.yaml (1)
82-85: LGTM! Well-structured clean script.The
cleanscript follows the established pattern and is properly documented.
| Future<void> _initializeWindowManager() async { | ||
| if (kIsWeb) return; | ||
|
|
||
| // Must add this line. | ||
| await windowManager.ensureInitialized(); | ||
|
|
||
| // final titleBarHeight = await windowManager.getTitleBarHeight(); | ||
|
|
||
| final newSize = Size(kResolution.width, kResolution.height); | ||
|
|
||
| final windowOptions = WindowOptions( | ||
| size: newSize, | ||
| backgroundColor: Colors.black, | ||
| skipTaskbar: false, | ||
| minimumSize: newSize, | ||
| windowButtonVisibility: true, | ||
| title: 'Superdeck', | ||
| titleBarStyle: TitleBarStyle.hidden, | ||
| ); | ||
|
|
||
| windowManager.waitUntilReadyToShow(windowOptions, () async { | ||
| await windowManager.show(); | ||
| await windowManager.focus(); | ||
| }); | ||
|
|
||
| await windowManager.setAspectRatio(kAspectRatio); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling and platform checks.
The window manager operations should be wrapped in try-catch blocks and include platform checks to handle potential errors gracefully.
Here's a suggested implementation:
Future<void> _initializeWindowManager() async {
if (kIsWeb) return;
+ if (!Platform.isWindows && !Platform.isLinux && !Platform.isMacOS) return;
- // Must add this line.
- await windowManager.ensureInitialized();
+ try {
+ await windowManager.ensureInitialized();
- final newSize = Size(kResolution.width, kResolution.height);
+ final newSize = Size(kResolution.width, kResolution.height);
- final windowOptions = WindowOptions(
- size: newSize,
- backgroundColor: Colors.black,
- skipTaskbar: false,
- minimumSize: newSize,
- windowButtonVisibility: true,
- title: 'Superdeck',
- titleBarStyle: TitleBarStyle.hidden,
- );
+ final windowOptions = WindowOptions(
+ size: newSize,
+ backgroundColor: Colors.black,
+ skipTaskbar: false,
+ minimumSize: newSize,
+ windowButtonVisibility: true,
+ title: 'Superdeck',
+ titleBarStyle: TitleBarStyle.hidden,
+ );
- windowManager.waitUntilReadyToShow(windowOptions, () async {
- await windowManager.show();
- await windowManager.focus();
- });
+ windowManager.waitUntilReadyToShow(windowOptions, () async {
+ try {
+ await windowManager.show();
+ await windowManager.focus();
+ } catch (e) {
+ debugPrint('Error showing/focusing window: $e');
+ }
+ });
- await windowManager.setAspectRatio(kAspectRatio);
+ await windowManager.setAspectRatio(kAspectRatio);
+ } catch (e) {
+ debugPrint('Error initializing window manager: $e');
+ }
}Don't forget to add the import:
import 'dart:io' show Platform;| @@ -12,6 +12,7 @@ command: | |||
| flutter: ">=3.19.0" | |||
| dependencies: | |||
| collection: ^1.18.0 | |||
| mix: ^1.5.4 | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify the mix package version.
Let's verify if version ^1.5.4 is the latest stable version of the mix package.
🌐 Web query:
What is the latest version of the mix package on pub.dev?
💡 Result:
The latest version of the mix package on pub.dev is 1.5.3, released in December 2024[1][8]. This update includes features like spring curve animations, improved modifier order handling, and fixes for border alignment utilities[1].
For context:
- The package focuses on building Flutter design systems with utilities for styling, responsive layouts, and token-based theming[8].
- It has undergone significant updates since its initial release in July 2021[4], with recent versions introducing animated styles, code generation support[1], and improved widget state management[8].
Citations:
- 1: https://pub.dev/packages/mix/changelog
- 2: https://pub.dev/packages/mixpanel_flutter
- 3: https://pub.dev/documentation/mix/latest/mix/
- 4: https://fluttergems.dev/packages/mix/
- 5: https://pub.dev
- 6: https://pub.dev/packages/mix/versions/0.9.0
- 7: https://github.com/dart-lang/pub-dev
- 8: https://pub.dev/packages/mix
- 9: https://pub.dev/packages/mix/example
- 10: https://pub.dev/packages/mix/versions
Mix package version mismatch detected
The web query shows that the latest stable version of the mix package on pub.dev is 1.5.3, whereas the file uses ^1.5.4. This discrepancy suggests that the version in the file should be updated to ^1.5.3.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
packages/superdeck/lib/src/modules/common/helpers/measure_size.dart (1)
19-48: 🛠️ Refactor suggestionOptimize performance by reducing unnecessary post-frame callbacks.
The current implementation schedules a new post-frame callback on every build, which could impact performance.
As mentioned in the previous review, consider optimizing by:
- Only scheduling callbacks when necessary
- Adding
didUpdateWidgetto handle size changes@override Widget build(BuildContext context) { - WidgetsBinding.instance.addPostFrameCallback(_afterLayout); return Container( key: _widgetKey, child: widget.child, ); } +@override +void initState() { + super.initState(); + _widgetKey = GlobalKey(debugLabel: 'MeasureSize'); + // Schedule initial measurement + WidgetsBinding.instance.addPostFrameCallback(_afterLayout); +} + +@override +void didUpdateWidget(MeasureSize oldWidget) { + super.didUpdateWidget(oldWidget); + // Schedule measurement when child widget changes + if (widget.child != oldWidget.child) { + WidgetsBinding.instance.addPostFrameCallback(_afterLayout); + } +}
🧹 Nitpick comments (21)
packages/superdeck/lib/src/modules/common/styles/style.dart (3)
8-11: Consider adding parameterization for color.
Currently,_baseTextStyleis hard-coded to use a white color. This might pose challenges for dynamic theming or darker backgrounds. Allowing a configurable color parameter (or referencing a theme) can improve flexibility.
66-103: Double-check font scaling on large headings.
Your heading styles escalate font sizes dramatically (e.g.,h1: 96px). For mobile or smaller screens, this can result in text overflow. Potentially use responsive scaling or media queries for more robust UI.
193-219: Remove or justify commented-out code.
These lines are entirely commented out. If they are no longer needed, removing them can enhance readability. Otherwise, consider adding context for future usage.packages/superdeck/lib/src/modules/common/helpers/measure_size.dart (2)
5-17: Add documentation to improve code maintainability.Consider adding documentation comments to describe:
- The purpose of the widget
- The parameters and their usage
- Example usage
Add this documentation above the class declaration:
+/// A widget that measures the size of its child and notifies when the size changes. +/// +/// Example usage: +/// ```dart +/// MeasureSize( +/// onChange: (size) { +/// print('Widget size: ${size.width} x ${size.height}'); +/// }, +/// child: MyWidget(), +/// ) +/// ``` class MeasureSize extends StatefulWidget {
38-47: Use safer type casting to handle unexpected render objects.The current implementation assumes the render object is a RenderBox, which might not always be true.
Consider adding a type check:
- final renderBox = context.findRenderObject() as RenderBox?; + final renderObject = context.findRenderObject(); + if (renderObject is! RenderBox) return; + final renderBox = renderObject; if (_size == renderBox?.size) return; - if (renderBox != null && renderBox.hasSize) { + if (renderBox.hasSize) { _size = renderBox.size; widget.onChange(renderBox.size); }packages/superdeck/example/lib/src/style.dart (1)
67-94: Consider adjusting the line height for better readability.The base text style has a line height of 0.6, which is quite small and might cause text overlap or readability issues, especially with the large font sizes used (140px for h1 and h2).
Consider increasing the line height to a value between 1.0 and 1.5 for better readability.
- $.baseTextStyle.height(0.6), + $.baseTextStyle.height(1.2),.github/workflows/test.yml (1)
34-38: Consider stricter analyzer settings.Setting
fatal-warnings: falsemight allow code quality issues to slip through. Consider enabling it to maintain higher code quality standards.- uses: invertase/github-action-dart-analyzer@v1 with: fatal-infos: false - fatal-warnings: false + fatal-warnings: truepackages/superdeck/example/lib/src/parts/background.dart (5)
18-33: Consider making mesh dimensions configurable.The mesh dimensions (3x3) and colors are hardcoded. Consider making these configurable to improve reusability and flexibility.
-OMeshRect _meshBuilder(List<Color> colors) { +OMeshRect _meshBuilder( + List<Color> colors, { + int width = 3, + int height = 3, + Color fallbackColor = const Color(0xff0e0e0e), + Color backgroundColor = const Color(0x00d6d6d6), +}) { return OMeshRect( - width: 3, - height: 3, - fallbackColor: const Color(0xff0e0e0e), - backgroundColor: const Color(0x00d6d6d6), + width: width, + height: height, + fallbackColor: fallbackColor, + backgroundColor: backgroundColor, vertices: [ (0.0, 0.0).v, (0.5, 0.0).v, (1.0, 0.0).v, // Row 1
50-50: Fix typo in comment.The comment has a typo: "bwett" should be "between".
-// animate bwett colors and previous colors in duration +// animate between colors and previous colors in duration
62-92: Consider making animation duration configurable.The animation duration is hardcoded. Consider making it configurable through a parameter to allow for different animation speeds.
class _AnimatedSwitcherOMesh extends StatefulWidget { final SlideConfiguration slide; + final Duration duration; const _AnimatedSwitcherOMesh({ required this.slide, + this.duration = const Duration(milliseconds: 1000), });Then update the state class:
class _AnimatedSwitcherOMeshState extends State<_AnimatedSwitcherOMesh> with SingleTickerProviderStateMixin { late List<Color> _colors; - final _duration = const Duration(milliseconds: 1000); - @override Widget build(BuildContext context) { return AnimatedOMeshGradient( mesh: _meshBuilder(_colors), - duration: _duration, + duration: widget.duration, ); }
94-104: Document colors and remove duplicates.The color list contains:
- Undocumented colors making it hard to understand their purpose
- Duplicate colors (e.g., Color.fromARGB(255, 5, 5, 5))
Consider:
- Adding documentation to explain the color scheme
- Using named constants for better readability
- Removing duplicate colors
+/// Colors used for the background mesh gradient. +/// These colors create a dark theme with subtle variations. +const _darkBlue = Color.fromARGB(255, 5, 5, 28); +const _black = Color.fromARGB(255, 5, 5, 5); +const _navyBlue = Color.fromARGB(255, 3, 19, 48); +const _purple = Color.fromARGB(255, 41, 12, 56); +const _deepPurple = Color.fromARGB(255, 17, 0, 63); + final _buildColors = [ - const Color.fromARGB(255, 5, 5, 28), - const Color.fromARGB(255, 5, 5, 5), - const Color.fromARGB(255, 3, 19, 48), - const Color.fromARGB(255, 41, 12, 56), - const Color.fromARGB(255, 5, 5, 5), - const Color.fromARGB(255, 5, 5, 5), - const Color.fromARGB(255, 17, 0, 63), - const Color.fromARGB(255, 0, 0, 0), - const Color.fromARGB(255, 5, 5, 5), + _darkBlue, + _black, + _navyBlue, + _purple, + _deepPurple, + Colors.black, ];
105-108: Fix typo in function name.The function name has a typo: "determiniscOrderBasedOnIndex" should be "deterministicOrderBasedOnIndex".
-List<Color> _determiniscOrderBasedOnIndex(int index) { +List<Color> _deterministicOrderBasedOnIndex(int index) { return _buildColors.sublist(index % _buildColors.length) ..addAll(_buildColors.sublist(0, index % _buildColors.length)); }packages/superdeck/lib/src/components/organisms/webview_wrapper.dart (1)
75-101: Enhance user experience with loading and error states.The current implementation lacks visual feedback for loading and error states. Consider:
- Adding a loading indicator while the WebView is initializing
- Displaying error messages when navigation fails
- Using a more elegant solution for refresh than toggling visibility
@override Widget build(BuildContext context) { super.build(context); return Stack( children: [ - WebViewWidget(key: ValueKey(_hide), controller: _controller), + WebViewWidget(controller: _controller), + if (_isLoading) + const Center( + child: CircularProgressIndicator(), + ), + if (_error != null) + Center( + child: Text(_error!, style: TextStyle(color: Colors.red)), + ), Row( children: [ IconButton( onPressed: () { - setState(() { - _hide = !_hide; - }); + _controller.reload(); }, icon: const Icon(Icons.refresh), ),packages/superdeck/lib/src/modules/export/slide_capture_service.dart (5)
28-29: Empty constructor.
Since the constructor does not accept parameters or execute logic, consider removing it for brevity or adding relevant initialization logic if needed.-class SlideCaptureService { - SlideCaptureService(); +class SlideCaptureService {
34-37: Question default capture quality.
The default toSlideCaptureQuality.lowmight underrepresent typical needs. Consider adjusting or making it a parameter in calling code to ensure the desired quality is chosen by default.
71-73: Consider a default quality parameter.
Unlike thecapturemethod,captureFromKeymandates explicit quality. If there's a common usage pattern, providing a default might simplify call sites.
104-109: Evaluate the overhead of nested MaterialApp.
Embedding a fullMaterialAppjust for rendering may be heavier than needed, especially if repeated frequently. Consider using a minimal widget, likeWidgetsApp, or even a specialized approach for offscreen rendering if you do not need all Material features.
178-178: Refine artificial delay.
A hardcoded 100ms delay to ensure rendering might be brittle. Investigate usingSchedulerBinding.instance.addPostFrameCallbackor a more robust mechanism to detect completion of the render pipeline.packages/superdeck/lib/src/modules/export/export_controller.dart (1)
37-41: Typo in documentation.
The comment on line 37 includes “df” (“The list of slides to export.df”). Consider removing or clarifying it.-/// The list of slides to export.df +/// The list of slides to export.packages/superdeck/lib/src/modules/deck/deck_provider.dart (1)
59-90: Consider adding error handling for controller initialization.The controller initialization could benefit from try-catch blocks to handle potential errors during initialization and updates.
@override void initState() { super.initState(); + try { controller = DeckController.build( slides: widget.reference.slides, options: widget.options, dataStore: widget.dataStore, ); + } catch (e) { + debugPrint('Failed to initialize DeckController: $e'); + // Consider providing a fallback or showing an error state + } } @override void didUpdateWidget(_DeckControllerProvider oldWidget) { super.didUpdateWidget(oldWidget); final referenceChanged = widget.reference != oldWidget.reference; final optionsChanged = widget.options != oldWidget.options; if (referenceChanged || optionsChanged) { + try { controller.update( slides: widget.reference.slides, options: widget.options, ); + } catch (e) { + debugPrint('Failed to update DeckController: $e'); + // Consider providing error feedback to the user + } } }packages/superdeck/lib/src/components/atoms/loading_indicator.dart (1)
64-79: Consider optimizing the animation state check.While the build method is cleaner now, the animation state check could be more efficient.
@override Widget build(BuildContext context) { - if (widget.isLoading || _animationController.isAnimating) { + final bool shouldShow = widget.isLoading || _animationController.value < 1.0; + if (shouldShow) { return FadeTransition( opacity: _opacityAnimation, child: Container(
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/superdeck/example/.superdeck/assets/thumbnail_RiDZbaFZ.pngis excluded by!**/*.png
📒 Files selected for processing (12)
.github/workflows/test.yml(1 hunks)packages/superdeck/example/lib/src/parts/background.dart(1 hunks)packages/superdeck/example/lib/src/style.dart(1 hunks)packages/superdeck/lib/src/components/atoms/loading_indicator.dart(6 hunks)packages/superdeck/lib/src/components/molecules/slide_screen.dart(1 hunks)packages/superdeck/lib/src/components/organisms/webview_wrapper.dart(1 hunks)packages/superdeck/lib/src/modules/common/helpers/extensions.dart(1 hunks)packages/superdeck/lib/src/modules/common/helpers/measure_size.dart(1 hunks)packages/superdeck/lib/src/modules/common/styles/style.dart(1 hunks)packages/superdeck/lib/src/modules/deck/deck_provider.dart(1 hunks)packages/superdeck/lib/src/modules/export/export_controller.dart(1 hunks)packages/superdeck/lib/src/modules/export/slide_capture_service.dart(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/superdeck/lib/src/modules/common/helpers/extensions.dart
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Test
🔇 Additional comments (43)
packages/superdeck/lib/src/modules/common/styles/style.dart (1)
22-32: Verify zero padding for images.
Setting.padding.all(0)for image sections may cause visual edge collisions in certain layouts or devices. Consider whether a minimal padding or dynamic padding would be more user-friendly.packages/superdeck/lib/src/modules/common/helpers/measure_size.dart (1)
1-3: LGTM!The typedef declaration is clear and follows Dart conventions.
packages/superdeck/example/lib/src/style.dart (3)
6-41: Verify the intent of commented code.The
BaseStyleclass currently returns an empty style object since all style configurations are commented out. This might not provide any meaningful styling.Please verify if:
- The commented code should be removed if no longer needed.
- The commented code should be uncommented if the styles are still required.
44-65: LGTM!The implementation looks good:
- Proper font configuration using Google Fonts
- Correct gradient setup with appropriate opacity values using the non-deprecated
useOpacitymethod
96-115: LGTM!The implementation looks good:
- Consistent use of Noto Serif font for quotes and h6
- Clear visual hierarchy with appropriate font sizes
- Distinctive blockquote styling with red left border
.github/workflows/test.yml (2)
42-45: Verify Melos targets exist.Ensure that the Melos targets
gen:buildandtestare properly defined in yourmelos.yamlfile.✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Check if the required Melos targets are defined # Search for gen:build and test targets in melos.yaml rg -A 5 'gen:build|test:' melos.yamlLength of output: 794
Melos Targets Verified
Both the
gen:buildandtesttargets are defined in themelos.yamlfile as confirmed by the search results. No changes are needed.
31-32: Verify Melos action version.Let's confirm that v3 is the latest version of the Melos action.
✅ Verification successful
🌐 Web query:
What is the latest version of bluefireteam/melos-action?💡 Result:
The latest version of
bluefireteam/melos-actionis v3, as referenced in multiple sources discussing GitHub Actions workflows and configuration examples[1][2][9]. This version is used for activating Melos in CI/CD pipelines, managing dependencies, and automating tasks like package bootstrapping and publishing[2][9].Citations:
- 1: https://community.sonarsource.com/t/dart-support-melos-dependency-management/125041
- 2: https://github.com/marketplace/actions/melos-action
- 3: https://pub.dev/packages/nilts/versions/0.18.0/changelog
- 4: https://github.com/bluefireteam/melos-action/actions
- 5: https://github.com/bluefireteam/melos-action/actions
- 6: https://opencollective.ecosyste.ms/projects/183541/advisories
- 7: https://cjycode.com/flutter_rust_bridge/manual/integrate/library/ci
- 8: https://github.com/bluefireteam/melos-action/blob/main/action.yml
- 9: https://quickcoder.org/how-to-manage-a-flutter-monorepo-with-multiple-packages/
- 10: https://github.com/bluefireteam/melos-action/activity
Melos action version is up-to-date.
- The web query confirms that v3 is the latest version of the bluefireteam/melos-action.
packages/superdeck/example/lib/src/parts/background.dart (2)
5-16: Remove or fix the commented code.The
_colorFromHexfunction is commented out but not removed. If it's no longer needed, remove it. If it will be used in the future, fix the validation issue and uncomment it.The previous review comment about hex string length validation is still relevant if you decide to keep this function.
35-48: LGTM!The
BackgroundPartwidget is well-implemented, correctly obtaining the slide configuration from context and using it to build the animated mesh.packages/superdeck/lib/src/components/organisms/webview_wrapper.dart (3)
1-13: LGTM!The class structure and imports are well-organized, following Flutter conventions. The use of
GlobalKeyis appropriate for managing widget state uniquely.
31-35: Implement error handlers for better error management and user feedback.The navigation delegate has empty handlers for progress, errors, and page events.
49-73: Secure the JavaScript execution to prevent XSS attacks.The methods
clearDartPadEditorandsetDartPadEditorContentdirectly inject content into JavaScript execution.packages/superdeck/lib/src/modules/export/slide_capture_service.dart (6)
9-13: Imports appear consistent.
These newly added imports appropriately support the usage ofInheritedData,SlideView, andSlideConfiguration. No concerns here.
15-19: Enum-based quality configuration looks good.
Defining pixel ratios through an enum is a neat approach for readability and maintainability. Ensure your default ratio balances performance and clarity of the captured slides.
21-23: Straightforward constructor.
The enum constructor cleanly stores the numeric pixel ratio. No concerns here.
47-50: Explicit debugging flags.
Copying the slide withdebug: falseandisExporting: trueclarifies the export state. This is a well-structured approach to avoid side effects on the original slide.
53-59: Confirm context validity.
Passing theexportingSlidetoSlideViewwithinInheritedDatais correct in principle, but ensure thatglobalKey.currentContextis valid prior to calling_fromWidgetToImage. In unexpected scenarios, the context might still be null.
87-91: Proper image disposal.
Returning aUint8Listafter callingdispose()on the original image is a best practice that helps manage memory usage.packages/superdeck/lib/src/modules/export/export_controller.dart (14)
1-14: Imports and setup look valid.
All imported packages—file_saver, PDF libraries, universal HTML, etc.—are consistent with cross-platform export functionality. No immediate concerns.
15-15: Clear export status enum.
DefiningExportStatusclarifies the controller’s workflow stages. Straightforward approach.
17-28: Well-structured constructor.
Instantiating_pageControllerand embedding it in_slideKeysfeels coherent. Make sure that thekeyfields in slides are unique to avoid collisions when building theMap.
30-36: Private fields usage.
The practice of marking internal states (_exportStatus,_disposed) as private ensures controlled access to the export logic. Good encapsulation.
43-47: Configurable wait duration.
Allowing a custom waiting period between operations is helpful for controlling performance or UI transitions. Nicely flexible.
48-50: Simple key retrieval.
getSlideKeyneatly retrieves the stored key for a given slide. Straightforward approach.
52-66: Progress and status getters.
TheprogressandprogressTextgetters provide cohesive feedback for the UI. Sharp approach for user-friendly progress updates.
68-85: Robust render boundary wait.
The_waitForRenderBoundaryPaintmethod repeatedly checks for context readiness, then an attached boundary, then an end-of-frame. This careful approach mitigates race conditions in capturing the widget.
87-108: Capture with retries.
The_captureImageWithRetrymethod is well-designed for transient capture failures on some devices or platforms. Having a small delay between retries is a reasonable fallback.
110-124: Preparation step.
prepare()ensures each slide is navigated to and rendered prior to capture. This step is essential for reliable image capture. Good design choice.
126-180: Comprehensive export flow.
Theexport()method transitions through statuses, captures each slide, assembles the PDF, and resets state cleanly. Well-executed orchestration of the export pipeline.
182-209: File saving approach.
_savePdfhandles both web-based blob saving and desktop usage viaFileSaver. Logging the save result helps debug potential file system issues. Good coverage for multi-platform support.
211-216: Proper disposal.
Marking_disposed = trueand disposing_pageControlleris correct. Ensure calls toexport()are not triggered post-disposal.
219-247: PDF building logic is straightforward.
The_buildPdffunction individually inserts each captured image into a PDF page. This is easy to maintain and debug. Keeping the resolution consistent withkResolutionis a nice touch.packages/superdeck/lib/src/components/molecules/slide_screen.dart (5)
1-3: Imports for slide screen functionality.
These imports are consistent with the app's architecture for extended widget usage and custom helpers. No concerns.
4-7: InheritedData, SlideConfiguration, and SlideView references.
All relevant modules are correctly imported for rendering the slide data. Looks good.
8-13: New SlideScreen class introduced.
Provides a distinct widget for displaying a slide. Clear naming aligns with the broader codebase references to “slides.”
14-16: Configuration field is final.
StoringSlideConfigurationin a final field ensures immutability at runtime, thus maintaining consistent slide data.
17-36: Clean build layout.
Centering the container with a background color and shadow effect is visually straightforward. EmbeddingInheritedDataforSlideViewusage is a well-structured approach to pass configuration data.packages/superdeck/lib/src/modules/deck/deck_provider.dart (3)
11-40: LGTM! Well-structured implementation of the builder pattern.The class effectively manages data store initialization and follows Flutter's widget patterns. The conditional initialization of data store based on process capability is a good practice.
42-57: LGTM! Clean implementation of a stateful widget.The class structure follows Flutter's best practices for stateful widgets and properly manages the deck controller lifecycle.
92-99: LGTM! Effective use of InheritedNotifierData.The build method correctly uses InheritedNotifierData for dependency injection, making the controller available to child widgets.
packages/superdeck/lib/src/components/atoms/loading_indicator.dart (3)
28-56: LGTM! Improved animation handling.The changes enhance the animation system by:
- Using CurvedAnimation for smoother transitions
- Adding proper state management with mounted checks
- Better separation of animation logic
99-103: LGTM! Clean use of extension methods for color opacity.The use of the extension method
useOpacitymakes the code more readable and maintainable.
172-177: LGTM! Good use of constants with clear documentation.Converting the values to constants and adding explanatory comments improves code maintainability.
| extension TextDirectiveX<T extends Attribute> on TextDirectiveUtility<T> { | ||
| T sameWidthLines(int lines) => call((String text) { | ||
| final words = text.split(' '); | ||
| final averageLineLength = text.length / lines; | ||
|
|
||
| final formattedLines = <String>[]; | ||
| var currentLine = StringBuffer(); | ||
|
|
||
| for (var word in words) { | ||
| final newLineLength = currentLine.length + word.length + 1; | ||
| if (newLineLength > averageLineLength && currentLine.isNotEmpty) { | ||
| formattedLines.add(currentLine.toString()); | ||
| currentLine.clear(); | ||
| } | ||
| currentLine.write(currentLine.isEmpty ? word : ' $word'); | ||
| } | ||
|
|
||
| if (currentLine.isNotEmpty) { | ||
| formattedLines.add(currentLine.toString()); | ||
| } | ||
|
|
||
| return formattedLines.join('\n'); | ||
| }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Revisit the line-wrapping approach.
sameWidthLines(int lines) splits text by character count, which may produce unnatural breaks and does not account for actual text metrics. If precise alignment is essential, consider a text-measurement-based approach (e.g., using TextPainter) or fallback to standard Flutter line-wrapping with a maximum width constraint.
| - name: Install FVM | ||
| shell: bash | ||
| run: | | ||
| curl -fsSL https://fvm.app/install.sh | bash | ||
| fvm use stable --force |
There was a problem hiding this comment.
Enhance security of FVM installation.
The current installation method of piping curl directly to bash poses security risks. Consider these improvements:
- Download the script first and verify its checksum
- Pin to a specific version of the install script
- Add error handling for the installation process
Here's a safer implementation:
- - name: Install FVM
- shell: bash
- run: |
- curl -fsSL https://fvm.app/install.sh | bash
- fvm use stable --force
+ - name: Install FVM
+ shell: bash
+ run: |
+ INSTALL_SCRIPT="install.sh"
+ EXPECTED_SHA256="<insert_expected_sha256_here>" # Get this from FVM's official documentation
+
+ # Download the script
+ curl -fsSL https://fvm.app/install.sh -o "$INSTALL_SCRIPT"
+
+ # Verify checksum
+ echo "$EXPECTED_SHA256 $INSTALL_SCRIPT" | sha256sum -c || exit 1
+
+ # Make executable and run
+ chmod +x "$INSTALL_SCRIPT"
+ ./"$INSTALL_SCRIPT" || exit 1
+
+ # Verify FVM installation
+ command -v fvm >/dev/null 2>&1 || { echo "FVM installation failed" >&2; exit 1; }
+
+ fvm use stable --force || exit 1📝 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.
| - name: Install FVM | |
| shell: bash | |
| run: | | |
| curl -fsSL https://fvm.app/install.sh | bash | |
| fvm use stable --force | |
| - name: Install FVM | |
| shell: bash | |
| run: | | |
| INSTALL_SCRIPT="install.sh" | |
| EXPECTED_SHA256="<insert_expected_sha256_here>" # Get this from FVM's official documentation | |
| # Download the script | |
| curl -fsSL https://fvm.app/install.sh -o "$INSTALL_SCRIPT" | |
| # Verify checksum | |
| echo "$EXPECTED_SHA256 $INSTALL_SCRIPT" | sha256sum -c || exit 1 | |
| # Make executable and run | |
| chmod +x "$INSTALL_SCRIPT" | |
| ./"$INSTALL_SCRIPT" || exit 1 | |
| # Verify FVM installation | |
| command -v fvm >/dev/null 2>&1 || { echo "FVM installation failed" >&2; exit 1; } | |
| fvm use stable --force || exit 1 |
| super.initState(); | ||
| (widget._uniqueKey).currentState?.dispose(); | ||
| _controller = WebViewController() | ||
| ..setJavaScriptMode(JavaScriptMode.unrestricted) |
There was a problem hiding this comment.
Consider restricting JavaScript execution for security.
Using JavaScriptMode.unrestricted could expose the application to security vulnerabilities. Consider using a more restrictive mode if possible, or document the security implications and mitigation strategies.
| Future<void> executeInIframe(String code) { | ||
| return _controller.runJavaScript(code); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling and input validation.
The executeInIframe method should include:
- Input validation for the JavaScript code
- Error handling for failed JavaScript execution
- Logging for debugging purposes
Future<void> executeInIframe(String code) {
- return _controller.runJavaScript(code);
+ if (code.isEmpty) {
+ throw ArgumentError('JavaScript code cannot be empty');
+ }
+ try {
+ return _controller.runJavaScript(code);
+ } catch (e) {
+ debugPrint('Failed to execute JavaScript: $e');
+ rethrow;
+ }
}📝 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.
| Future<void> executeInIframe(String code) { | |
| return _controller.runJavaScript(code); | |
| } | |
| Future<void> executeInIframe(String code) { | |
| if (code.isEmpty) { | |
| throw ArgumentError('JavaScript code cannot be empty'); | |
| } | |
| try { | |
| return _controller.runJavaScript(code); | |
| } catch (e) { | |
| debugPrint('Failed to execute JavaScript: $e'); | |
| rethrow; | |
| } | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (10)
packages/superdeck/lib/src/components/parts/footer.dart (1)
3-23: Consider enhancing internationalization and customization options.The widget could be improved by:
- Using internationalized text instead of hardcoded strings
- Adding customization options for text style and appearance
Apply this diff to enhance the widget:
class FooterPart extends StatelessWidget implements PreferredSizeWidget { const FooterPart({ super.key, + this.text = 'SUPERDECK', + this.style, }); + final String text; + final TextStyle? style; @override Size get preferredSize => const Size.fromHeight(50); @override Widget build(context) { - return const Padding( + return Padding( padding: EdgeInsets.symmetric(horizontal: 16.0), child: Row( mainAxisAlignment: MainAxisAlignment.start, children: [ - Text('SUPERDECK'), + Text(text, style: style), ], ), ); } }packages/superdeck/lib/src/components/parts/slide_parts.dart (1)
6-22: Add a copyWith method for better immutability support.The class would benefit from a copyWith method to create modified instances while maintaining immutability.
Apply this diff to add the copyWith method:
class SlideParts { const SlideParts({ this.header, this.footer, this.background, }); final PreferredSizeWidget? header; final PreferredSizeWidget? footer; final Widget? background; + SlideParts copyWith({ + PreferredSizeWidget? header, + PreferredSizeWidget? footer, + Widget? background, + }) { + return SlideParts( + header: header ?? this.header, + footer: footer ?? this.footer, + background: background ?? this.background, + ); + } static const defaultParts = SlideParts( header: HeaderPart(), footer: FooterPart(), background: BackgroundPart(), ); }packages/superdeck/lib/src/components/parts/header.dart (1)
4-29: Enhance internationalization and slide count display.Consider the following improvements:
- Use internationalized text for the default title
- Display total slide count along with current index (e.g., "1/10")
Apply this diff to enhance the widget:
class HeaderPart extends StatelessWidget implements PreferredSizeWidget { const HeaderPart({ super.key, + this.defaultTitle = 'Generative UI with Flutter', }); + final String defaultTitle; @override Size get preferredSize => const Size.fromHeight(50); @override Widget build(context) { final slide = SlideConfiguration.of(context); final index = slide.slideIndex; + final total = slide.totalSlides; return Padding( padding: const EdgeInsets.symmetric(horizontal: 16.0), child: Row( mainAxisAlignment: MainAxisAlignment.end, children: [ - Text(slide.options.title ?? 'Generative UI with Flutter'), + Text(slide.options.title ?? defaultTitle), const SizedBox(width: 20), - Text('${index + 1}'), + Text('${index + 1}/$total'), ], ), ); } }packages/superdeck/lib/src/components/parts/background.dart (1)
4-15: Consider implementing the color utility function.The commented-out
_colorFromHexfunction appears to be part of planned color functionality. Consider implementing it if color support is needed.Apply this diff to implement the function:
-// Color _colorFromHex(String hexString) { -// hexString = hexString.trim(); -// if (hexString.isEmpty) { -// return Colors.black; // Default color if null or empty -// } -// hexString = hexString.replaceAll(RegExp(r'[^a-fA-F0-9]'), ''); -// hexString = hexString.replaceAll('#', ''); -// if (hexString.length == 6) { -// hexString = 'FF$hexString'; // Add opacity if not provided -// } -// return Color(int.parse(hexString, radix: 16)); -// } +Color _colorFromHex(String hexString) { + hexString = hexString.trim(); + if (hexString.isEmpty) { + return Colors.black; // Default color if null or empty + } + hexString = hexString.replaceAll(RegExp(r'[^a-fA-F0-9]'), ''); + hexString = hexString.replaceAll('#', ''); + if (hexString.length == 6) { + hexString = 'FF$hexString'; // Add opacity if not provided + } + return Color(int.parse(hexString, radix: 16)); +}packages/superdeck/lib/src/modules/deck/slide_configuration.dart (1)
12-48: Add documentation for public API.The class has good structure but lacks documentation for its public methods and properties. Consider adding documentation to improve code maintainability.
Add documentation for public members:
@MappableClass() class SlideConfiguration with SlideConfigurationMappable { + /// The index of the current slide in the deck final int slideIndex; + /// The style configuration for the slide final Style style; final Slide _slide; + /// Flag to enable debug mode final bool debug; + /// Optional parts configuration for the slide final SlideParts? parts; final Map<String, WidgetBlockBuilder> _widgets; + /// Flag indicating if the slide is being exported final bool isExporting; + /// Creates a new slide configuration SlideConfiguration({ required this.slideIndex, required this.style, required Slide slide, this.debug = false, this.parts, Map<String, WidgetBlockBuilder> widgets = const {}, this.isExporting = false, }) : _slide = slide, _widgets = widgets; + /// Returns the slide options, defaults to empty options if none set SlideOptions get options => _slide.options ?? const SlideOptions(); + /// Returns the unique key for this slide String get key => _slide.key; + /// Returns the underlying slide data Slide get data => _slide; + /// Returns the sections in this slide List<SectionBlock> get sections => _slide.sections; + /// Returns the comments associated with this slide List<String> get comments => _slide.comments; + /// Returns the widget builder for the given name if it exists WidgetBlockBuilder? getWidget(String name) => _widgets[name]; + /// Retrieves the slide configuration from the widget tree static SlideConfiguration of(BuildContext context) { return InheritedData.of(context); }packages/superdeck/lib/src/modules/deck/deck_options.dart (2)
47-49: Remove duplicate documentation comment.The same documentation comment appears twice.
- /// Returns the value for [key] converted to type [T], or `null` if the conversion fails. - /// Returns the value for [key] converted to type [T], or `null` if the conversion fails.
50-72: Simplify type conversion logic.The type conversion logic in
_getMaybeAscould be simplified using a map of conversion functions.Consider refactoring to:
- T? _getMaybeAs<T>(String key) { - final value = this[key]; - if (value == null) return null; - if (value is T) return value; - - if (T == int) { - if (value is num) return value.toInt() as T; - if (value is String) return int.tryParse(value) as T?; - } else if (T == double) { - if (value is num) return value.toDouble() as T; - if (value is String) return double.tryParse(value) as T?; - } else if (T == bool) { - if (value is String) { - final lower = value.toLowerCase(); - if (lower == 'true') return true as T; - if (lower == 'false') return false as T; - } - } else if (T == String) { - return value.toString() as T; - } - - return null; + static final _converters = <Type, dynamic Function(dynamic)>{ + int: (value) { + if (value is num) return value.toInt(); + if (value is String) return int.tryParse(value); + return null; + }, + double: (value) { + if (value is num) return value.toDouble(); + if (value is String) return double.tryParse(value); + return null; + }, + bool: (value) { + if (value is String) { + final lower = value.toLowerCase(); + if (lower == 'true') return true; + if (lower == 'false') return false; + } + return null; + }, + String: (value) => value.toString(), + }; + + T? _getMaybeAs<T>(String key) { + final value = this[key]; + if (value == null) return null; + if (value is T) return value; + + final converter = _converters[T]; + if (converter != null) { + final result = converter(value); + if (result != null) return result as T; + } + return null; }packages/superdeck/lib/src/components/organisms/app_shell.dart (3)
192-195: Extract magic numbers into named constants.The hardcoded height value should be extracted into a named constant for better maintainability.
+ static const _bottomPanelHeight = 200.0; + // ... SizedBox( - height: 200, + height: _bottomPanelHeight, child: _buildPanel(context), ),
205-208: Extract magic numbers into named constants.The hardcoded width value should be extracted into a named constant for better maintainability.
+ static const _sidePanelWidth = 300.0; + // ... SizedBox( - width: 300, + width: _sidePanelWidth, child: _buildPanel(context), ),
123-148: Simplify panel layout logic.The conditional rendering of panels could be simplified by extracting the layout logic into separate methods.
Consider refactoring to:
+ Widget _buildSmallLayoutPanel(bool isNotesOpen, Widget thumbnailPanel, Widget commentsPanel) { + return Row( + children: [ + !isNotesOpen + ? Expanded(child: thumbnailPanel) + : Expanded(child: commentsPanel), + ], + ); + } + + Widget _buildRegularLayoutPanel(bool isNotesOpen, Widget thumbnailPanel, Widget commentsPanel) { + return Column( + children: [ + Expanded( + flex: 3, + child: thumbnailPanel, + ), + if (isNotesOpen) + Expanded( + flex: 1, + child: commentsPanel, + ), + ], + ); + } + Widget _buildPanel(BuildContext context) { // ... existing setup code ... - if (widget.isSmallLayout) { - return Row( - children: [ - !isNotesOpen - ? Expanded(child: thumbnailPanel) - : Expanded(child: commentsPanel), - ], - ); - } else { - return Column( - children: [ - Expanded( - flex: 3, - child: thumbnailPanel, - ), - if (isNotesOpen) - Expanded( - flex: 1, - child: commentsPanel, - ), - ], - ); - } + return widget.isSmallLayout + ? _buildSmallLayoutPanel(isNotesOpen, thumbnailPanel, commentsPanel) + : _buildRegularLayoutPanel(isNotesOpen, thumbnailPanel, commentsPanel); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
packages/superdeck/lib/src/components/organisms/app_shell.dart(1 hunks)packages/superdeck/lib/src/components/organisms/thumbnail_panel.dart(1 hunks)packages/superdeck/lib/src/components/parts/background.dart(1 hunks)packages/superdeck/lib/src/components/parts/footer.dart(1 hunks)packages/superdeck/lib/src/components/parts/header.dart(1 hunks)packages/superdeck/lib/src/components/parts/slide_parts.dart(1 hunks)packages/superdeck/lib/src/modules/deck/deck_options.dart(1 hunks)packages/superdeck/lib/src/modules/deck/slide_configuration.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/superdeck/lib/src/components/organisms/thumbnail_panel.dart
🧰 Additional context used
🪛 GitHub Check: Test
packages/superdeck/lib/src/components/parts/background.dart
[warning] 24-24: The value of the local variable 'configuration' isn't used.
Try removing the variable or using it.
See https://dart.dev/diagnostics/unused_local_variable to learn more about this problem.
| class BackgroundPart extends StatelessWidget { | ||
| const BackgroundPart({ | ||
| super.key, | ||
| }); | ||
|
|
||
| @override | ||
| Widget build(BuildContext context) { | ||
| final configuration = SlideConfiguration.of(context); | ||
|
|
||
| return const SizedBox.shrink(); | ||
| } | ||
| } |
There was a problem hiding this comment.
Implement background functionality or remove unused code.
The widget retrieves the configuration but doesn't use it. Either implement the background functionality or remove the unused code.
Apply one of these diffs:
Option 1 - Implement background functionality:
class BackgroundPart extends StatelessWidget {
const BackgroundPart({
super.key,
});
@override
Widget build(BuildContext context) {
final configuration = SlideConfiguration.of(context);
+ final backgroundColor = configuration.options.backgroundColor;
+ return Container(
+ color: backgroundColor != null ? _colorFromHex(backgroundColor) : null,
+ );
- return const SizedBox.shrink();
}
}Option 2 - Remove unused code:
class BackgroundPart extends StatelessWidget {
const BackgroundPart({
super.key,
});
@override
Widget build(BuildContext context) {
- final configuration = SlideConfiguration.of(context);
return const SizedBox.shrink();
}
}📝 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.
| class BackgroundPart extends StatelessWidget { | |
| const BackgroundPart({ | |
| super.key, | |
| }); | |
| @override | |
| Widget build(BuildContext context) { | |
| final configuration = SlideConfiguration.of(context); | |
| return const SizedBox.shrink(); | |
| } | |
| } | |
| class BackgroundPart extends StatelessWidget { | |
| const BackgroundPart({ | |
| super.key, | |
| }); | |
| @override | |
| Widget build(BuildContext context) { | |
| return const SizedBox.shrink(); | |
| } | |
| } |
🧰 Tools
🪛 GitHub Check: Test
[warning] 24-24: The value of the local variable 'configuration' isn't used.
Try removing the variable or using it.
See https://dart.dev/diagnostics/unused_local_variable to learn more about this problem.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (19)
packages/superdeck/lib/src/components/atoms/slide_thumbnail.dart (1)
20-29: Add documentation for the SlideThumbnail widget.Consider adding documentation to describe the widget's purpose, parameters, and usage.
+/// A widget that displays a thumbnail of a slide with selection state. +/// +/// The thumbnail is retrieved using [ThumbnailController] and displayed with +/// proper styling based on the [selected] state. class SlideThumbnail extends StatelessWidget { + /// Whether the thumbnail is currently selected. final bool selected; + /// The configuration of the slide to display. final SlideConfiguration slide; const SlideThumbnail({ super.key, required this.selected, required this.slide, });packages/superdeck/lib/src/components/organisms/app_shell.dart (4)
15-17: Enhance class documentation.While the documentation is clear, consider adding more details about:
- The purpose of the app shell in the application architecture
- When to use small vs regular layout
- Example usage
57-102: Consider extracting animation duration to constants.The animation duration is currently hardcoded. Consider moving it to a constants file for better maintainability and reuse.
- static const _animationDuration = Duration(milliseconds: 200);Add to constants.dart:
const kPanelAnimationDuration = Duration(milliseconds: 200);Then update the reference:
+ static const _animationDuration = kPanelAnimationDuration;
104-159: Consider extracting panel building logic to improve maintainability.The
_buildPanelmethod is quite complex with nested conditional logic. Consider extracting the layout-specific panel building into separate methods for better maintainability.Example refactor:
Widget _buildSmallLayoutPanel(BuildContext context, bool isNotesOpen, Widget thumbnailPanel, Widget commentsPanel) { return Row( children: [ !isNotesOpen ? Expanded(child: thumbnailPanel) : Expanded(child: commentsPanel), ], ); } Widget _buildRegularLayoutPanel(BuildContext context, bool isNotesOpen, Widget thumbnailPanel, Widget commentsPanel) { return Column( children: [ Expanded( flex: 3, child: thumbnailPanel, ), if (isNotesOpen) Expanded( flex: 1, child: commentsPanel, ), ], ); }
203-205: Extract panel dimensions to constants.The panel dimensions (height: 200, width: 300) are hardcoded. Consider moving these to constants for better maintainability and responsive design.
Add to constants.dart:
const kSmallLayoutPanelHeight = 200.0; const kRegularLayoutPanelWidth = 300.0;Then update the references:
- height: 200, + height: kSmallLayoutPanelHeight,- width: 300, + width: kRegularLayoutPanelWidth,Also applies to: 216-218
packages/superdeck/lib/src/modules/deck/deck_controller.dart (3)
20-36: Consider always callingnotifyListeners().Currently,
notifyListeners()is only invoked if eitherslidesoroptionsis non-null. If the intention is to allow updates even when the provided arguments are the same or null, you might want to remove the condition to ensure that downstream listeners always receive an update notification.
59-82: Consider making_buildSlidesa static or private method within theDeckController.While a top-level function can work, making
_buildSlidesa static method or a private method ofDeckControllerwould enhance discoverability and organization, and keep data transformations closer to the controller's domain logic.
121-131: Confirm visibility of_emptySlide.
_emptySlideis declared as a top-level variable in this file. Although Dart enforces privacy through the underscore naming convention, consider surfacing_emptySlideas a private static class member ofDeckControllerif you want it tightly coupled with the controller logic. This helps keep the deck concept self-contained.packages/superdeck/lib/src/modules/slide_capture/slide_capture_service.dart (1)
96-190: Evaluate performance of repeated layout flushes.The
_fromWidgetToImagemethod triggers a loop of building, laying out, and painting the widget up to 10 times, with 100ms delays. This approach, while robust for ensuring a fully laid-out widget, can slow batch operations if capturing many slides. Consider a more direct approach or an event-based system that signals when the layout is stable to reduce repeated attempts.packages/superdeck/lib/src/modules/slide_capture/thumbnail_controller.dart (2)
24-105: Address potential concurrency in_generate.Multiple calls to
_generatemay overlap if an external caller invokesload()while a previous load is still running. You might want to add synchronization logic or an internal check to prevent multiple loads from interfering with each other. This can avoid redundant disk I/O or random state changes.
142-150: Consider chunked thumbnail generation for large decks.Calling
generateThumbnailswith a large list of slides could trigger many asynchronous file operations simultaneously. For large decks or slower devices, consider chunking the generation process or limiting concurrency to avoid potential I/O bottlenecks or resource exhaustion.packages/superdeck/lib/src/modules/deck/deck_options.mapper.dart (1)
104-178: Comprehensive copy mechanism
TheDeckOptionsCopyWithclasses and extension add robust copy functionality. This metadata-driven approach works well though it can seem verbose. If performance becomes critical in large data sets, verify that these reflective mappings don’t introduce unnecessary overhead.packages/superdeck/lib/src/modules/slide_capture/pdf_controller.dart (2)
75-113: Render boundary paint waiting
The_waitForRenderBoundaryPaintmethod correctly loops until the widget is attached and rendered. This approach works but can introduce performance hits in certain edge cases if the UI is slow to build. Maintain awareness of potential time spent in these loops.
114-131: Capture retry logic
The_captureImageWithRetryfunction with a maximum of three attempts is straightforward. Consider exponential backoff or additional logging for repeated failures. The approach is fine, but you might want to add more context to error logs on the final failure.packages/superdeck/lib/src/modules/deck/slide_configuration.mapper.dart (1)
129-208: Deep copy with reflection
The_SlideConfigurationCopyWithImplclass includes deep copy logic for nested objects, including_slide. This is powerful and convenient but can be a performance consideration for large or frequently updated sets of slides. Keep an eye on potential overhead.packages/superdeck/lib/src/modules/deck/slide_configuration.dart (4)
12-21: Add class and field documentation.While the class structure is good, adding documentation would improve maintainability:
- Add a class-level doc comment explaining the purpose and usage of
SlideConfiguration- Document important fields, especially those that affect slide rendering like
style,parts, and_widgetsExample documentation:
/// Configuration for a single slide in a deck. /// /// Manages the slide's content, styling, and rendering options. @MappableClass() class SlideConfiguration with SlideConfigurationMappable { /// The index of this slide in the deck. final int slideIndex; /// The style to apply to this slide. final Style style; // ... more field documentation
23-33: Add constructor parameter documentation.The constructor parameters would benefit from documentation to explain their purpose and requirements.
Example documentation:
/// Creates a new slide configuration. /// /// [slideIndex] The position of this slide in the deck. /// [style] The style to apply to this slide. /// [slide] The slide data to configure. /// [debug] Whether to show debug information. /// [parts] Optional slide parts for custom rendering. /// [thumbnailFile] The path to the slide's thumbnail image. /// [widgets] Custom widget builders, defaults to empty. /// [isExporting] Whether the slide is being exported, defaults to false. SlideConfiguration({ required this.slideIndex, // ... rest of the constructor
47-49: Add error handling to theofmethod.The
ofmethod should handle cases where theSlideConfigurationis not found in the widget tree.static SlideConfiguration of(BuildContext context) { - return InheritedData.of(context); + final config = InheritedData.of<SlideConfiguration>(context); + if (config == null) { + throw FlutterError( + 'SlideConfiguration.of() was called with a context that does not ' + 'contain a SlideConfiguration.\n' + 'No SlideConfiguration ancestor could be found starting from the ' + 'context that was passed to SlideConfiguration.of().' + ); + } + return config; }
45-45: Consider using a more null-safe approach forgetWidget.The
getWidgetmethod could be more explicit about null safety and provide better documentation.- WidgetBlockBuilder? getWidget(String name) => _widgets[name]; + /// Returns the widget builder for the given name, if one exists. + /// + /// Returns `null` if no builder is registered for [name]. + WidgetBlockBuilder? getWidget(String name) { + assert(name.isNotEmpty, 'Widget name cannot be empty'); + return _widgets[name]; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (21)
packages/superdeck/example/.superdeck/assets/thumbnail_0zqy1l5c.pngis excluded by!**/*.pngpackages/superdeck/example/.superdeck/assets/thumbnail_14RbmSW5.pngis excluded by!**/*.pngpackages/superdeck/example/.superdeck/assets/thumbnail_3sLdrfsM.pngis excluded by!**/*.pngpackages/superdeck/example/.superdeck/assets/thumbnail_9BmK4SPw.pngis excluded by!**/*.pngpackages/superdeck/example/.superdeck/assets/thumbnail_9mHDFwa9.pngis excluded by!**/*.pngpackages/superdeck/example/.superdeck/assets/thumbnail_9y5hBeTm.pngis excluded by!**/*.pngpackages/superdeck/example/.superdeck/assets/thumbnail_CwxHOCpO.pngis excluded by!**/*.pngpackages/superdeck/example/.superdeck/assets/thumbnail_F2fTbXOG.pngis excluded by!**/*.pngpackages/superdeck/example/.superdeck/assets/thumbnail_H2GzZVSx.pngis excluded by!**/*.pngpackages/superdeck/example/.superdeck/assets/thumbnail_RiDZbaFZ.pngis excluded by!**/*.pngpackages/superdeck/example/.superdeck/assets/thumbnail_SJncL4H2.pngis excluded by!**/*.pngpackages/superdeck/example/.superdeck/assets/thumbnail_WyJ8NjmF.pngis excluded by!**/*.pngpackages/superdeck/example/.superdeck/assets/thumbnail_XeDZiCNk.pngis excluded by!**/*.pngpackages/superdeck/example/.superdeck/assets/thumbnail_Z40wIUYP.pngis excluded by!**/*.pngpackages/superdeck/example/.superdeck/assets/thumbnail_aTAXFyQ7.pngis excluded by!**/*.pngpackages/superdeck/example/.superdeck/assets/thumbnail_cS8UY7ii.pngis excluded by!**/*.pngpackages/superdeck/example/.superdeck/assets/thumbnail_nPPBLQ6k.pngis excluded by!**/*.pngpackages/superdeck/example/.superdeck/assets/thumbnail_oglBIjM0.pngis excluded by!**/*.pngpackages/superdeck/example/.superdeck/assets/thumbnail_pukXIjvK.pngis excluded by!**/*.pngpackages/superdeck/example/.superdeck/assets/thumbnail_ybLDY8oi.pngis excluded by!**/*.pngpackages/superdeck/example/.superdeck/assets/thumbnail_z34aal1W.pngis excluded by!**/*.png
📒 Files selected for processing (12)
packages/superdeck/lib/src/components/atoms/slide_thumbnail.dart(1 hunks)packages/superdeck/lib/src/components/molecules/bottom_bar.dart(1 hunks)packages/superdeck/lib/src/components/organisms/app_shell.dart(1 hunks)packages/superdeck/lib/src/modules/deck/deck_controller.dart(1 hunks)packages/superdeck/lib/src/modules/deck/deck_options.mapper.dart(1 hunks)packages/superdeck/lib/src/modules/deck/deck_provider.dart(1 hunks)packages/superdeck/lib/src/modules/deck/slide_configuration.dart(1 hunks)packages/superdeck/lib/src/modules/deck/slide_configuration.mapper.dart(1 hunks)packages/superdeck/lib/src/modules/slide_capture/pdf_controller.dart(1 hunks)packages/superdeck/lib/src/modules/slide_capture/slide_capture_service.dart(4 hunks)packages/superdeck/lib/src/modules/slide_capture/thumbnail_controller.dart(1 hunks)packages/superdeck/lib/src/screens/export_screen.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/superdeck/lib/src/components/molecules/bottom_bar.dart
- packages/superdeck/lib/src/screens/export_screen.dart
- packages/superdeck/lib/src/modules/deck/deck_provider.dart
🧰 Additional context used
🪛 GitHub Check: Test
packages/superdeck/lib/src/components/atoms/slide_thumbnail.dart
[warning] 53-53: A value for optional parameter 'key' isn't ever given.
Try removing the unused parameter.
See https://dart.dev/diagnostics/unused_element_parameter to learn more about this problem.
[warning] 80-80: The declaration '_showOverlayMenu' isn't referenced.
Try removing the declaration of '_showOverlayMenu'.
See https://dart.dev/diagnostics/unused_element to learn more about this problem.
🔇 Additional comments (21)
packages/superdeck/lib/src/components/atoms/slide_thumbnail.dart (3)
30-46: Consider implementing error handling for thumbnail generation.The thumbnail retrieval process should handle potential errors to provide a better user experience.
48-78: Well-structured styling implementation!The
_PreviewContainerwidget effectively uses themixpackage for styling and animations. The implementation is clean and maintainable.🧰 Tools
🪛 GitHub Check: Test
[warning] 53-53: A value for optional parameter 'key' isn't ever given.
Try removing the unused parameter.
See https://dart.dev/diagnostics/unused_element_parameter to learn more about this problem.
8-18:✅ Verification successful
Verify if the popup menu functionality is implemented.
The code defines a popup menu system with a refresh thumbnail action, but it appears to be unused. If this functionality is intended to be used, ensure it's properly integrated with the
SlideThumbnailwidget.Also applies to: 80-112
🏁 Script executed:
#!/bin/bash # Description: Search for usages of _showOverlayMenu and _PopMenuAction # Search for any usage of the popup menu rg -A 5 '_showOverlayMenu|_PopMenuAction' --type dartLength of output: 2869
Popup Menu Functionality Confirmed
The popup menu functionality is indeed implemented and integrated within the
SlideThumbnailwidget. The enum_PopMenuActionis correctly used in the_showOverlayMenufunction (lines 80–112) to generate menu items such as the refresh action. No further action is required, but please verify that any additional integrations or tests for the popup menu behavior remain up-to-date.packages/superdeck/lib/src/components/organisms/app_shell.dart (2)
17-37: LGTM! Clean and focused implementation.The AppShell widget effectively:
- Wraps content with keyboard shortcuts for navigation
- Handles responsive layout detection
- Manages navigation state
39-55: LGTM! Well-structured widget definition.The SplitView widget is well-documented and properly structured with clear property declarations.
packages/superdeck/lib/src/modules/deck/deck_controller.dart (2)
27-27: Validate slide data conversions.When
slidesis null, the code falls back tothis.slides.map((slide) => slide.data).toList(). Ensure that eachSlideConfigurationinthis.slidesactually contains a valid.datafield and that thedataproperty is guaranteed to be aSlide, to avoid potential type mismatches or runtime errors.
84-119: Guard against empty or missingblock.name.In the
_convertSlidefunction,widgetBuilderis retrieved fromoptions.widgets[block.name]. Ifblock.nameis null or empty, this could cause unexpected behavior. You might want to add checks or constraints to ensure block names exist and are non-empty before attempting to retrieve widget builders.packages/superdeck/lib/src/modules/slide_capture/slide_capture_service.dart (1)
39-39: Hash collision risk.Using
shortHash(slide.key + quality.name)could lead to collisions if different slides end up producing the same short hash. This can inadvertently purge or stall tasks in the generation queue. Consider using a more collision-resistant hashing mechanism or a unique ID approach.packages/superdeck/lib/src/modules/slide_capture/thumbnail_controller.dart (1)
68-78: Ensure ignoring force in certain status states is intended.When
_statusisloadingordone, the code returns immediately, ignoring theforceparameter logic. This might be intentional to save repeated loads, but confirm whether a user explicitly requesting a “force update” should override the current status.packages/superdeck/lib/src/modules/deck/deck_options.mapper.dart (4)
1-6: Ignore coverage for generated code
These lines indicate standard practice for generated files, which are typically excluded from test coverage and lint checks due to their auto-generated nature. This is acceptable if the actual customization resides elsewhere.
9-18: Singleton initialization
Using a singleton pattern withensureInitializedis consistent with typical mapper usage. Just ensure it’s thread-safe and consider if any concurrency is possible in how_instanceis set. No immediate issue here, but keep it in mind for multi-threaded use.
23-41: Sensible defaults in fields
The fields provide defaults such asconst DeckStyle(), empty maps, and booleans. This helps avoid null references in downstream code. Ensure that these defaults align with the intended fallback behavior for deck styling and widget mappings.
73-102: Mixin usage for convenient mapping
TheDeckOptionsMappablemixin nicely centralizes serialization (toJson,toMap) and also thecopyWith,toString, and equality overrides. This is a clean approach that keeps the model class lighter.packages/superdeck/lib/src/modules/slide_capture/pdf_controller.dart (3)
15-31: Export status enum
ThePdfExportStatusenum provides a clear state machine (idle, capturing, building, etc.). This is helpful for front-end updates or debugging. Just ensure transitions between statuses always follow a logical sequence.
198-225: Differentiated file saving strategy
Handling the PDF save differently for web vs. native is correct. Consider user feedback or progress indication for large PDFs (especially on web). The try/catch block for logging is a good start; rethrowing or informing the UI might further improve error visibility.
235-266: Building the PDF in a separate isolate
Usingcompute(_buildPdf, [..._images])helps avoid blocking the main UI thread. This is a clean approach. Verify that thepw.MemoryImageusage pluscenteralignment matches your final layout expectations (e.g., for slides with aspect ratios different fromkResolution).packages/superdeck/lib/src/modules/deck/slide_configuration.mapper.dart (4)
1-6: Ignore coverage for generated code
As with other mapper files, these directives are typical for auto-generated classes. No concerns here, provided custom logic resides in the primary class file.
9-19: Singleton for mapper initialization
SlideConfigurationMapperuses similar singleton logic as other mappers. TheSlideMapper.ensureInitialized()call in line 16 ensures dependencies are also registered. This helps maintain a consistent setup across the app.
24-51: Field definitions
These fields match the properties inSlideConfiguration. The optional and default values fordebug,parts,_widgets, andisExportingallow robust fallback behavior. Ensure that fields with underscore prefixes are intentionally hidden from external usage or if they should be renamed for clarity.
88-120: Mixin-based utility
TheSlideConfigurationMappablemixin provides consistent JSON serialization, mapping,copyWith, and string/equality overrides. This architecture is in line with previous mappers.packages/superdeck/lib/src/modules/deck/slide_configuration.dart (1)
1-10: LGTM! Well-organized imports.The imports are logically grouped and necessary for the class functionality. The part directive correctly links to the generated mapper file.
| itemBuilder: (index, selected) { | ||
| return SlideThumbnail( | ||
| selected: selected, | ||
| slide: deckController.slides[index], | ||
| ); | ||
| }, |
There was a problem hiding this comment.
Add bounds checking for slide access.
The array access deckController.slides[index] could throw an index out of bounds exception if the slides array is empty or the index is invalid.
- slide: deckController.slides[index],
+ slide: index < deckController.slides.length
+ ? deckController.slides[index]
+ : 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.
| itemBuilder: (index, selected) { | |
| return SlideThumbnail( | |
| selected: selected, | |
| slide: deckController.slides[index], | |
| ); | |
| }, | |
| itemBuilder: (index, selected) { | |
| return SlideThumbnail( | |
| selected: selected, | |
| slide: index < deckController.slides.length | |
| ? deckController.slides[index] | |
| : null, | |
| ); | |
| }, |
| Future<Uint8List> capture({ | ||
| SlideCaptureQuality quality = SlideCaptureQuality.thumbnail, | ||
| required SlideConfiguration slide, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Potential indefinite wait condition.
When the queue exceeds _maxConcurrentGenerations, the code waits in 50ms intervals but never checks for an overall timeout or maximum wait time. In rare cases, if a generation never completes or errors out, new captures could remain stuck. Consider adding a timeout or an error path for long-stalled tasks.
| Future<File> _generateThumbnail({ | ||
| required SlideConfiguration slide, | ||
| required BuildContext context, | ||
| required bool force, | ||
| }) async { | ||
| final thumbnailFile = File(slide.thumbnailFile); | ||
|
|
||
| if (!kCanRunProcess) { | ||
| return thumbnailFile; | ||
| } | ||
|
|
||
| final isValid = | ||
| await thumbnailFile.exists() && (await thumbnailFile.length()) > 0; | ||
|
|
||
| if (isValid && !force) { | ||
| return thumbnailFile; | ||
| } | ||
|
|
||
| final imageData = await _slideCaptureService.capture( | ||
| slide: slide, | ||
| // ignore: use_build_context_synchronously | ||
| context: context, | ||
| ); | ||
|
|
||
| await thumbnailFile.writeAsBytes(imageData, flush: false); | ||
|
|
||
| return thumbnailFile; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Evaluate file overwrites and partial writes.
In _generateThumbnail(), if the operation fails during File.writeAsBytes, the existing file could become corrupted. Consider writing to a temporary file first and then moving it into place to ensure consistency and reduce the risk of partial overwrites.
| /// Starts the export process | ||
| /// | ||
| /// Captures slides as images and combines them into a PDF document. | ||
| Future<void> export() async { | ||
| _exportStatus = PdfExportStatus.preparing; | ||
| notifyListeners(); | ||
|
|
||
| await prepare(); | ||
|
|
||
| _exportStatus = PdfExportStatus.capturing; | ||
| notifyListeners(); | ||
|
|
||
| for (var i = 0; i < _slideKeys.length; i++) { | ||
| final slide = slides[i]; | ||
| final key = _slideKeys[slide.key]!; | ||
|
|
||
| await _pageController.animateToPage( | ||
| i, | ||
| duration: const Duration(milliseconds: 1), | ||
| curve: Curves.linear, | ||
| ); | ||
|
|
||
| await _waitForRenderBoundaryPaint(key); | ||
| if (disposed) return; | ||
|
|
||
| final image = await _captureImageWithRetry(key); | ||
| if (disposed) return; | ||
|
|
||
| _images.add(image); | ||
| notifyListeners(); | ||
| } | ||
|
|
||
| _exportStatus = PdfExportStatus.building; | ||
| notifyListeners(); | ||
| await Future.delayed(_waitDuration); | ||
| if (disposed) return; | ||
|
|
||
| final pdf = await compute(_buildPdf, [..._images]); | ||
| if (disposed) return; | ||
|
|
||
| notifyListeners(); | ||
|
|
||
| _savePdf(pdf); | ||
|
|
||
| _exportStatus = PdfExportStatus.complete; | ||
| _images.clear(); | ||
| notifyListeners(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Export workflow
The export() method orchestrates preparing slides, capturing images, building, and finally saving the PDF. It transitions _exportStatus states appropriately. Consider error handling for each step (e.g., partial capture or PDF failures) to let the UI reflect partial progress or cancellations gracefully.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
packages/superdeck/lib/src/modules/slide_capture/pdf_export_screen.dart (2)
38-56: Add error handling to show export failures to the user.While the export process is properly managed, errors are silently handled. Consider showing an error dialog or snackbar when the export fails.
Future<void> _handleExport() async { try { await _exportController.export(); + } catch (e) { + if (context.mounted) { + ScaffoldMessenger.of(context).showSnackBar( + SnackBar( + content: Text('Failed to export PDF: ${e.toString()}'), + backgroundColor: Theme.of(context).colorScheme.error, + ), + ); + } } finally { if (context.mounted) { Navigator.of(context).pop();
73-119: Consider using semi-transparent overlay for better visual feedback.The black overlay completely hides the slides. Using a semi-transparent overlay would allow users to see the export progress visually while still keeping the progress bar readable.
- color: Colors.black, + color: Colors.black.withOpacity(0.7),packages/superdeck/lib/src/modules/slide_capture/pdf_controller.dart (4)
15-83: Add documentation for public members.While the class and enum are well-documented, some public members like
progressandprogressTupleare missing documentation. Consider adding doc comments to improve code maintainability.+ /// Returns the current export progress as a value between 0.0 and 1.0 double get progress => _slideKeys.isNotEmpty ? _images.length / _slideKeys.length : 0.0; + /// Returns a tuple of (current, total) slides processed (int current, int total) get progressTuple => (_images.length, _slideKeys.length);
110-128: Make retry delay configurable.The delay between retry attempts is hardcoded to 100ms. Consider making this configurable like
waitDurationto allow adjustment based on platform performance.class PdfController extends ChangeNotifier { PdfController({ required this.slides, required this.slideCaptureService, Duration waitDuration = const Duration(milliseconds: 100), + Duration retryDelay = const Duration(milliseconds: 100), }) : _waitDuration = waitDuration { _pageController = PageController(initialPage: 0); _slideKeys = { for (var slide in slides) slide.key: GlobalKey(), }; } + final Duration _retryDelay; Future<Uint8List> _captureImageWithRetry(GlobalKey key) async { const maxAttempts = 3; for (int attempt = 1; attempt <= maxAttempts; attempt++) { try { return await slideCaptureService.captureFromKey( quality: kIsWeb ? SlideCaptureQuality.thumbnail : SlideCaptureQuality.better, key: key, ); } catch (error) { if (attempt == maxAttempts) rethrow; - await Future.delayed(const Duration(milliseconds: 100)); + await Future.delayed(_retryDelay); } } throw Exception('Failed to capture image after $maxAttempts attempts.'); }
207-234: Improve error handling with specific error messages.While errors are logged, consider providing more specific error messages for different failure scenarios to aid in debugging.
} catch (e) { - log('Error saving pdf: $e'); + final platform = kIsWeb ? 'web' : 'native'; + log('Failed to save PDF on $platform platform: $e'); + if (e is SecurityException) { + log('Permission denied while saving PDF'); + } else if (e is FileSystemException) { + log('File system error while saving PDF'); + } }
248-279: Consider adding progress updates during PDF building.The PDF building process runs in a separate isolate but doesn't provide progress updates. Consider implementing a way to track and report building progress, especially for large presentations.
Future<Uint8List> _buildPdf(List<Uint8List> images) async { final pdf = pw.Document(); + final total = images.length; + int current = 0; for (final imageData in images) { final image = pw.MemoryImage(imageData); // ... existing code ... pdf.addPage(/* ... */); + current++; + // Send progress through a SendPort if needed } return await pdf.save(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/superdeck/lib/src/components/molecules/bottom_bar.dart(1 hunks)packages/superdeck/lib/src/modules/slide_capture/pdf_controller.dart(1 hunks)packages/superdeck/lib/src/modules/slide_capture/pdf_export_screen.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/superdeck/lib/src/components/molecules/bottom_bar.dart
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Test
🔇 Additional comments (3)
packages/superdeck/lib/src/modules/slide_capture/pdf_export_screen.dart (2)
11-27: LGTM! Well-structured dialog implementation.The class follows Flutter best practices with a clean separation between the widget and its state. The static
showmethod provides a convenient way to display the dialog while properly retrieving slides from the DeckController.
122-180: LGTM! Well-implemented progress indicator.The progress bar provides clear visual feedback with appropriate status messages and properly handles all export states. Good use of theme colors for consistent styling.
packages/superdeck/lib/src/modules/slide_capture/pdf_controller.dart (1)
147-206: LGTM! Robust export implementation with proper cancellation handling.The export workflow is well-structured with appropriate status updates and cancellation checks at critical points. The implementation properly handles the export process stages: preparing, capturing, building, and completion.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (7)
packages/superdeck/example/lib/src/parts/header.dart (1)
9-10: Consider making the header height configurable or responsive.The fixed height of 50 pixels might not be optimal for all screen sizes or orientations. Consider:
- Making the height configurable through the constructor
- Using MediaQuery to make it responsive to screen size
Example implementation:
+ final double height; const HeaderPart({ super.key, + this.height = 50, }); @override - Size get preferredSize => const Size.fromHeight(50); + Size get preferredSize => Size.fromHeight(height);packages/superdeck/example/.superdeck/generated_assets.json (1)
25-74: Duplicate Asset Entry WarningThe asset entry
".superdeck/assets/mermaid_srHRIuii.png"appears multiple times. If this repetition is unintentional, please consider de-duplicating the list to avoid any potential processing overhead or confusion during asset lookup.packages/superdeck/lib/src/modules/slide_capture/pdf_controller.dart (2)
37-52: Consider adding validation for constructor parameters.The constructor accepts required parameters but doesn't validate them. Consider adding validation for:
- Empty slides list
- Null slideCaptureService
- Non-positive waitDuration
PdfController({ required this.slides, required this.slideCaptureService, Duration waitDuration = const Duration(milliseconds: 100), }) : _waitDuration = waitDuration { + if (slides.isEmpty) { + throw ArgumentError.value(slides, 'slides', 'Slides list cannot be empty'); + } + if (waitDuration.isNegative) { + throw ArgumentError.value(waitDuration, 'waitDuration', 'Wait duration cannot be negative'); + } _pageController = PageController(initialPage: 0); _slideKeys = { for (var slide in slides) slide.key: GlobalKey(), }; }
254-282: Consider memory optimization for PDF building.The
_buildPdffunction loads all images into memory at once. For presentations with many slides, this could lead to high memory usage.Consider:
- Processing images in batches
- Using a memory-efficient PDF library
- Adding image compression options
packages/superdeck/lib/src/components/molecules/block_widget.dart (3)
64-73: Consider performance impact of Wrap widget.Using
Wrapfor non-scrollable content might impact performance for large blocks. Consider using a more efficient layout widget likeSizedBoxorContainerwith clipping.- current = Wrap( - clipBehavior: Clip.hardEdge, - children: [current], - ); + current = SizedBox( + width: widget.size.width, + height: widget.size.height, + child: ClipRect( + child: current, + ), + );
153-196: Enhance error visualization in WidgetBlockWidget.The error display in
WidgetBlockWidgetcould be more informative. Consider:
- Adding stack trace information
- Making error messages more user-friendly
- Adding retry capability
} catch (e) { + final error = e is Error ? e.stackTrace.toString() : e.toString(); return Container( padding: const EdgeInsets.all(16), decoration: BoxDecoration( color: Colors.red, border: Border.all( color: Colors.red, width: 2, ), borderRadius: BorderRadius.circular(8), ), child: Center( - child: Text(''' -Error building widget: ${data.block.name} - -${e.toString()}'''), + child: Column( + mainAxisSize: MainAxisSize.min, + children: [ + Text('Failed to build widget: ${data.block.name}', + style: const TextStyle(fontWeight: FontWeight.bold)), + const SizedBox(height: 8), + Text(error), + const SizedBox(height: 16), + ElevatedButton( + onPressed: () => setState(() {}), + child: const Text('Retry'), + ), + ], + ), ), ); }
261-270: Optimize layout calculation in SectionBlockWidget.The layout calculation in
SectionBlockWidgetcould be optimized by:
- Caching calculations when section configuration hasn't changed
- Using more efficient data structures
+ late final List<double> _blockLeftOffset; + late final double _widthPerFlex; + + void _calculateLayout() { final blockLeftOffset = List.filled(section.blocks.length, 0.0); double cumulativeLeftOffset = 0; final widthPerFlex = size.width / section.totalBlockFlex; - // get index for (var index = 0; index < section.blocks.length; index++) { final block = section.blocks[index]; final blockWidth = widthPerFlex * block.flex; blockLeftOffset[index] = cumulativeLeftOffset; cumulativeLeftOffset = cumulativeLeftOffset + blockWidth; } + _blockLeftOffset = blockLeftOffset; + _widthPerFlex = widthPerFlex; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
packages/superdeck/example/.superdeck/assets/thumbnail_cS8UY7ii.pngis excluded by!**/*.pngpackages/superdeck/example/.superdeck/assets/thumbnail_nPPBLQ6k.pngis excluded by!**/*.png
📒 Files selected for processing (8)
packages/superdeck/example/.superdeck/generated_assets.json(1 hunks)packages/superdeck/example/.superdeck/superdeck.json(1 hunks)packages/superdeck/example/lib/src/parts/header.dart(1 hunks)packages/superdeck/lib/src/components/molecules/block_widget.dart(1 hunks)packages/superdeck/lib/src/components/organisms/comments_panel.dart(1 hunks)packages/superdeck/lib/src/components/organisms/webview_wrapper.dart(1 hunks)packages/superdeck/lib/src/modules/slide_capture/pdf_controller.dart(1 hunks)packages/superdeck/lib/src/modules/slide_capture/pdf_export_screen.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/superdeck/lib/src/components/organisms/comments_panel.dart
- packages/superdeck/lib/src/modules/slide_capture/pdf_export_screen.dart
- packages/superdeck/lib/src/components/organisms/webview_wrapper.dart
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Test
🔇 Additional comments (10)
packages/superdeck/example/lib/src/parts/header.dart (2)
1-2: LGTM!The imports are appropriate and necessary for the widget's functionality.
4-7: LGTM!The class structure is well-designed:
- Appropriate use of StatelessWidget for a stateless header
- Correctly implements PreferredSizeWidget for AppBar-like behavior
- Clean constructor implementation
packages/superdeck/example/.superdeck/generated_assets.json (1)
1-24: JSON Structure ValidityThe file’s overall JSON structure is well-formed. The ISO 8601-formatted
"last_modified"timestamp and the clear array of asset paths make it easy to integrate with the asset management system.packages/superdeck/example/.superdeck/superdeck.json (4)
1-10: Overall JSON OrganizationThe slide deck configuration is organized in a clear and modular fashion with a top-level
"slides"array and a"config"object. The consistent use of keys and nested structures for slides, sections, and blocks supports flexible and dynamic rendering.
423-435: Typo Correction Needed: Header TextIn the slide with key
"Z40wIUYP", the header text currently reads:- "content": "## How can LLMs Understtand Your UI?\n", + "content": "## How can LLMs Understand Your UI?\n",Please correct the typo from “Understtand” to “Understand”.
436-440: Asset Path Consistency CheckThe subsequent block in the same slide references an image using the path
"assets/structured_output.png". Please verify that this path is consistent with your asset management strategy (e.g., whether it should be prefixed with.superdeck/or match paths listed ingenerated_assets.json) to ensure the asset is resolved at runtime.
1-567: Comprehensive Slide Deck ValidationThe new slide deck JSON file demonstrates a robust configuration with a mix of content types—including markdown text, code snippets, and widgets. Ensure that:
- All markdown formatting (e.g.,
{.animate},{.here},{.cover}) renders as intended.- Code snippets, particularly in Dart, display correctly.
- Widget configurations (like the color palette widget) behave as expected during slide rendering.
A final integration test with the presentation system is recommended to validate the dynamic behavior.
packages/superdeck/lib/src/modules/slide_capture/pdf_controller.dart (1)
149-208: Add progress tracking for PDF building phase.The export process tracks progress during image capture but not during PDF building. Consider adding progress tracking for the PDF building phase.
Additionally, consider adding error handling for specific exceptions that might occur during the export process.
#!/bin/bash # Search for potential error cases in the codebase rg -A 5 "throw.*Exception|error.*pdf" --type dartpackages/superdeck/lib/src/components/molecules/block_widget.dart (2)
17-32: LGTM! Good use of sealed class.The use of a sealed class for
BlockWidgetensures type-safe handling of different block types and makes the code more maintainable.
239-257: Consider accessibility in debug info.The debug information overlay might have insufficient color contrast and text size for accessibility.
#!/usr/bin/python3 from wcag_contrast_ratio import contrast # Check color contrast background_color = (0, 255, 255) # cyan text_color = (0, 0, 0) # black ratio = contrast(background_color, text_color) print(f"Contrast ratio: {ratio}") print(f"Meets WCAG AA for small text: {ratio >= 4.5}")
| Widget build(context) { | ||
| final slide = SlideConfiguration.of(context); | ||
|
|
||
| final index = slide.slideIndex; | ||
| return Padding( | ||
| padding: const EdgeInsets.symmetric(horizontal: 16.0), | ||
| child: Row( | ||
| mainAxisAlignment: MainAxisAlignment.end, | ||
| children: [ | ||
| Text(slide.options.title ?? 'Flame Game'), | ||
| const SizedBox(width: 20), | ||
| Text('${index + 1}'), | ||
| ], | ||
| ), | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling and improve UI consistency.
The build method has several areas for improvement:
- Add error handling for SlideConfiguration
- Apply consistent text styles using Theme
- Extract magic numbers to named constants
Here's a suggested implementation:
+ static const double _horizontalPadding = 16.0;
+ static const double _spacing = 20.0;
@override
Widget build(BuildContext context) {
- final slide = SlideConfiguration.of(context);
+ final slide = SlideConfiguration.of(context)
+ ?? (throw FlutterError('HeaderPart must be used within a SlideConfiguration'));
+ final textTheme = Theme.of(context).textTheme;
final index = slide.slideIndex;
return Padding(
- padding: const EdgeInsets.symmetric(horizontal: 16.0),
+ padding: const EdgeInsets.symmetric(horizontal: _horizontalPadding),
child: Row(
mainAxisAlignment: MainAxisAlignment.end,
children: [
- Text(slide.options.title ?? 'Flame Game'),
+ Text(
+ slide.options.title ?? 'Flame Game',
+ style: textTheme.titleMedium,
+ ),
- const SizedBox(width: 20),
+ const SizedBox(width: _spacing),
- Text('${index + 1}'),
+ Text(
+ '${index + 1}',
+ style: textTheme.titleMedium,
+ ),
],
),
);
}📝 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.
| Widget build(context) { | |
| final slide = SlideConfiguration.of(context); | |
| final index = slide.slideIndex; | |
| return Padding( | |
| padding: const EdgeInsets.symmetric(horizontal: 16.0), | |
| child: Row( | |
| mainAxisAlignment: MainAxisAlignment.end, | |
| children: [ | |
| Text(slide.options.title ?? 'Flame Game'), | |
| const SizedBox(width: 20), | |
| Text('${index + 1}'), | |
| ], | |
| ), | |
| ); | |
| } | |
| class HeaderPart extends StatelessWidget { | |
| // Extracted named constants for magic numbers. | |
| static const double _horizontalPadding = 16.0; | |
| static const double _spacing = 20.0; | |
| @override | |
| Widget build(BuildContext context) { | |
| final slide = SlideConfiguration.of(context) | |
| ?? (throw FlutterError('HeaderPart must be used within a SlideConfiguration')); | |
| final textTheme = Theme.of(context).textTheme; | |
| final index = slide.slideIndex; | |
| return Padding( | |
| padding: const EdgeInsets.symmetric(horizontal: _horizontalPadding), | |
| child: Row( | |
| mainAxisAlignment: MainAxisAlignment.end, | |
| children: [ | |
| Text( | |
| slide.options.title ?? 'Flame Game', | |
| style: textTheme.titleMedium, | |
| ), | |
| const SizedBox(width: _spacing), | |
| Text( | |
| '${index + 1}', | |
| style: textTheme.titleMedium, | |
| ), | |
| ], | |
| ), | |
| ); | |
| } | |
| } |
| Future<void> _savePdf(Uint8List pdf) async { | ||
| try { | ||
| if (kIsWeb) { | ||
| final blob = html.Blob([pdf], 'application/pdf'); | ||
| final url = html.Url.createObjectUrlFromBlob(blob); | ||
|
|
||
| html.AnchorElement(href: url) | ||
| ..setAttribute('download', 'superdeck.pdf') | ||
| ..click(); | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| log('Saving pdf'); | ||
| final result = await FileSaver.instance.saveAs( | ||
| name: 'superdeck', | ||
| bytes: pdf, | ||
| ext: 'pdf', | ||
| mimeType: MimeType.pdf, | ||
| ); | ||
| log('Save result: $result'); | ||
| } catch (e) { | ||
| log('Error saving pdf: $e'); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance error handling for PDF saving.
The error handling in _savePdf only logs errors but doesn't propagate them. Consider:
- Propagating errors to allow proper error handling by callers
- Adding specific error types for different failure scenarios
- Cleaning up resources in web platform case
Future<void> _savePdf(Uint8List pdf) async {
try {
if (kIsWeb) {
final blob = html.Blob([pdf], 'application/pdf');
final url = html.Url.createObjectUrlFromBlob(blob);
-
- html.AnchorElement(href: url)
- ..setAttribute('download', 'superdeck.pdf')
- ..click();
-
- return;
+ try {
+ html.AnchorElement(href: url)
+ ..setAttribute('download', 'superdeck.pdf')
+ ..click();
+ } finally {
+ html.Url.revokeObjectUrl(url);
+ }
+ return;
}
log('Saving pdf');
final result = await FileSaver.instance.saveAs(
name: 'superdeck',
bytes: pdf,
ext: 'pdf',
mimeType: MimeType.pdf,
);
if (result == null) {
- log('Save result: $result');
+ throw PdfSaveException('Failed to save PDF: null result');
}
} catch (e) {
log('Error saving pdf: $e');
+ throw PdfSaveException('Failed to save PDF: $e');
}
}
+class PdfSaveException implements Exception {
+ final String message;
+ PdfSaveException(this.message);
+ @override
+ String toString() => message;
+}📝 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.
| Future<void> _savePdf(Uint8List pdf) async { | |
| try { | |
| if (kIsWeb) { | |
| final blob = html.Blob([pdf], 'application/pdf'); | |
| final url = html.Url.createObjectUrlFromBlob(blob); | |
| html.AnchorElement(href: url) | |
| ..setAttribute('download', 'superdeck.pdf') | |
| ..click(); | |
| return; | |
| } | |
| log('Saving pdf'); | |
| final result = await FileSaver.instance.saveAs( | |
| name: 'superdeck', | |
| bytes: pdf, | |
| ext: 'pdf', | |
| mimeType: MimeType.pdf, | |
| ); | |
| log('Save result: $result'); | |
| } catch (e) { | |
| log('Error saving pdf: $e'); | |
| } | |
| } | |
| Future<void> _savePdf(Uint8List pdf) async { | |
| try { | |
| if (kIsWeb) { | |
| final blob = html.Blob([pdf], 'application/pdf'); | |
| final url = html.Url.createObjectUrlFromBlob(blob); | |
| try { | |
| html.AnchorElement(href: url) | |
| ..setAttribute('download', 'superdeck.pdf') | |
| ..click(); | |
| } finally { | |
| html.Url.revokeObjectUrl(url); | |
| } | |
| return; | |
| } | |
| log('Saving pdf'); | |
| final result = await FileSaver.instance.saveAs( | |
| name: 'superdeck', | |
| bytes: pdf, | |
| ext: 'pdf', | |
| mimeType: MimeType.pdf, | |
| ); | |
| if (result == null) { | |
| throw PdfSaveException('Failed to save PDF: null result'); | |
| } | |
| } catch (e) { | |
| log('Error saving pdf: $e'); | |
| throw PdfSaveException('Failed to save PDF: $e'); | |
| } | |
| } | |
| class PdfSaveException implements Exception { | |
| final String message; | |
| PdfSaveException(this.message); | |
| @override | |
| String toString() => message; | |
| } |
| Future<void> _waitForRenderBoundaryPaint(GlobalKey key) async { | ||
| while (key.currentContext == null) { | ||
| await Future.delayed(const Duration(milliseconds: 10)); | ||
| } | ||
|
|
||
| final repaintBoundary = key.currentContext!.findRenderObject()!; | ||
| final isAttached = repaintBoundary.attached; | ||
|
|
||
| while (!isAttached) { | ||
| await Future.delayed(const Duration(milliseconds: 10)); | ||
| } | ||
|
|
||
| await WidgetsBinding.instance.endOfFrame; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add timeout to prevent infinite waiting.
The _waitForRenderBoundaryPaint method could potentially wait indefinitely. Consider adding a timeout to prevent hanging.
Future<void> _waitForRenderBoundaryPaint(GlobalKey key) async {
+ final timeout = const Duration(seconds: 10);
+ final startTime = DateTime.now();
while (key.currentContext == null) {
+ if (DateTime.now().difference(startTime) > timeout) {
+ throw TimeoutException('Timeout waiting for render boundary to be painted');
+ }
await Future.delayed(const Duration(milliseconds: 10));
}
// ... rest of the method
}📝 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.
| Future<void> _waitForRenderBoundaryPaint(GlobalKey key) async { | |
| while (key.currentContext == null) { | |
| await Future.delayed(const Duration(milliseconds: 10)); | |
| } | |
| final repaintBoundary = key.currentContext!.findRenderObject()!; | |
| final isAttached = repaintBoundary.attached; | |
| while (!isAttached) { | |
| await Future.delayed(const Duration(milliseconds: 10)); | |
| } | |
| await WidgetsBinding.instance.endOfFrame; | |
| } | |
| Future<void> _waitForRenderBoundaryPaint(GlobalKey key) async { | |
| final timeout = const Duration(seconds: 10); | |
| final startTime = DateTime.now(); | |
| while (key.currentContext == null) { | |
| if (DateTime.now().difference(startTime) > timeout) { | |
| throw TimeoutException('Timeout waiting for render boundary to be painted'); | |
| } | |
| await Future.delayed(const Duration(milliseconds: 10)); | |
| } | |
| final repaintBoundary = key.currentContext!.findRenderObject()!; | |
| final isAttached = repaintBoundary.attached; | |
| while (!isAttached) { | |
| await Future.delayed(const Duration(milliseconds: 10)); | |
| } | |
| await WidgetsBinding.instance.endOfFrame; | |
| } |
Summary by CodeRabbit
New Features
WebViewWrapperfor better web content integration.AsyncThumbnailandThumbnailControllerfor efficient thumbnail management.DeckOptionsandSlideConfigurationclasses for improved configuration handling.ConverterHelperclass for flexible alignment and fit conversions.Chores
.gitignoreto exclude additional build artifacts and temporary files.