Skip to content

feat: YaruWindowTitleBar implement - #1763

Open
airon-main wants to merge 6 commits into
saber-notes:mainfrom
airon-main:feat/yaruwindowtitlebar_implement
Open

feat: YaruWindowTitleBar implement#1763
airon-main wants to merge 6 commits into
saber-notes:mainfrom
airon-main:feat/yaruwindowtitlebar_implement

Conversation

@airon-main

@airon-main airon-main commented Jun 6, 2026

Copy link
Copy Markdown

Before:
image

After:
image


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.

  • Refactors
    • Run YaruWindowTitleBar.ensureInitialized() at startup in main.dart only on Windows.
    • Use MaterialApp.builder in ExplicitlyThemedApp to wrap with _WindowsTitleBarWrapper on Windows, composing with any existing builder.

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.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

1 issue found across 2 files

Confidence score: 2/5

  • There is a high-confidence, user-facing crash risk: lib/main.dart calls YaruWindowTitleBar.ensureInitialized() unconditionally, which can throw MissingPluginException on 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

Comment thread lib/main.dart Outdated
@codecov

codecov Bot commented Jun 6, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 86.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 51.81%. Comparing base (f9859c5) to head (daf93b6).
⚠️ Report is 13 commits behind head on main.

Files with missing lines Patch % Lines
lib/main.dart 0.00% 2 Missing ⚠️
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.
📢 Have feedback on the report? Share it here.

@airon-main

Copy link
Copy Markdown
Author

YO YARU IS GEM, HOW DIDN'T I KNOW THIS

@airon-main

airon-main commented Jun 8, 2026

Copy link
Copy Markdown
Author
ishowspeed-talking-tom-news

@adil192 adil192 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Your screenshots look nice, thanks. Just some nitpicks to address now

Comment thread lib/main.dart Outdated
Comment on lines +50 to +51
if (Platform.isWindows || Platform.isLinux || Platform.isMacOS)
await YaruWindowTitleBar.ensureInitialized();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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.

Comment on lines +97 to +119
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;
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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,
  );

@airon-main

Copy link
Copy Markdown
Author

Was procastinating :(

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

Comment thread lib/components/theming/dynamic_material_app.dart Outdated
airon-main and others added 2 commits July 7, 2026 08:45
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
@airon-main

Copy link
Copy Markdown
Author
ishowspeed-talking-tom-news

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants