feat: YaruWindowTitleBar implement - #1763
Conversation
There was a problem hiding this comment.
1 issue found across 2 files
Confidence score: 2/5
- There is a high-confidence, user-facing crash risk:
lib/main.dartcallsYaruWindowTitleBar.ensureInitialized()unconditionally, which can throwMissingPluginExceptionon unsupported platforms like Android/iOS. - Given the severity (8/10) and confidence (8/10), this is likely to cause runtime regressions on mobile builds, so this is not a low-risk merge in its current state.
- Pay close attention to
lib/main.dart- gate platform-specific window initialization so unsupported platforms skip this call.
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1763 +/- ##
==========================================
+ Coverage 51.76% 51.81% +0.04%
==========================================
Files 133 133
Lines 9467 9483 +16
==========================================
+ Hits 4901 4914 +13
- Misses 4566 4569 +3 ☔ View full report in Codecov by Harness. |
|
YO YARU IS GEM, HOW DIDN'T I KNOW THIS |
adil192
left a comment
There was a problem hiding this comment.
Your screenshots look nice, thanks. Just some nitpicks to address now
| if (Platform.isWindows || Platform.isLinux || Platform.isMacOS) | ||
| await YaruWindowTitleBar.ensureInitialized(); |
There was a problem hiding this comment.
This should be moved into the await Future.wait([ block on line ~90 so it can be awaited in parallel to avoid slowing down app startup.
Also it should only run on Windows, not Linux/macOS.
| TransitionBuilder? buildWithDesktopShell(TransitionBuilder? builder) { | ||
| if (platform == .windows) { | ||
| return (context, child) => Scaffold( | ||
| appBar: YaruWindowTitleBar( | ||
| title: Row( | ||
| children: [ | ||
| SizedBox( | ||
| width: 16, | ||
| height: 16, | ||
| child: SvgPicture.asset('assets/icon/icon.svg'), | ||
| ), | ||
| const SizedBox(width: 8), | ||
| const Text('Saber'), | ||
| ], | ||
| ), | ||
| centerTitle: false, | ||
| buttonPadding: const EdgeInsets.only(bottom: 1), | ||
| ), | ||
| body: child, | ||
| ); | ||
| } | ||
| return builder; | ||
| } |
There was a problem hiding this comment.
This is a little inelegant, especially because we're passing the same arguments to ExplicitlyThemedApp each time. We should move this stuff into ExplicitlyThemedApp instead of DynamicMaterialApp to avoid repetition.
I'd recommend moving this widget tree into its own StatelessWidget for performance reasons. (Flutter can skip rebuilding a widget when its arguments remain the same.)
class _WindowsTitleBarWrapper extends StatelessWidget {
const _WindowsTitleBarWrapper({required this.child});
final Widget child;
@override
Widget build(BuildContext context) {
return Scaffold(.....);
}
}And then in ExplicitlyThemedApp, pass builder like this:
return MaterialApp.router(
// ...
builder: theme.platform == .windows
? (context, child) => _WindowsTitleBarWrapper(child: child)
: null,
);|
Was procastinating :( |
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>

Before:

After:

Summary by cubic
Added a custom Windows title bar with
YaruWindowTitleBar, showing the Saber SVG icon and app title. Windows-only initialization and UI wrapper; macOS/Linux/other platforms are unchanged.YaruWindowTitleBar.ensureInitialized()at startup inmain.dartonly on Windows.MaterialApp.builderinExplicitlyThemedAppto wrap with_WindowsTitleBarWrapperon Windows, composing with any existingbuilder.Note: Saber is trialing Cubic AI code review. AI output may contain mistakes, misconceptions, and hallucinations. You can act on the AI feedback if you find it helpful and are sure it's correct; otherwise you can disregard it and wait for a human reviewer.
Written for commit daf93b6. Summary will update on new commits.