Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace ToolBar with PreferredSizeWidget #477

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from

Conversation

Otacon
Copy link

@Otacon Otacon commented Aug 20, 2023

TL;DR

As per SOLID principles, MacOsScaffold's toolBar should be a PreferredSizeWidget.

The longer story.

Hello guys, hope you are well. I was going to write an app for MacOS. I have adopted this amazing library.
However, I have hit a wall with this scenario:
My MainScreen contains a MacOsScaffold, which contains a ToolBar and many children:

MacosScaffold(
    toolBar: const ToolBar(),
    children: [
        ...
    ],
);

My ToolBar has many options, conditions and buttons, so I would like to create a separate Widget to deal with those.
However, the MacosScaffold doesn't accept anything else that is not a ToolBar.

Other frameworks (e.g. Material) accepts a PreferredSizeWidget, which allows me to write

import 'package:macos_ui/macos_ui.dart'; //kToolbarHeight is now publicly available!

class MyToolbar extends StatelessWidget implements PreferredSizeWidget {
    @override
    Widget build(BuildContext context) {
        return ToolBar(
            actions: [
                ...
            ],
        )
    }

    @override
    Size get preferredSize => const Size.fromHeight(kToolbarHeight);
} 

And this solves my problem.

BONUS: this respect's Flutter's phylosophy of composition over inheritance.

Pre-launch Checklist

  • I have incremented the package version as appropriate and updated CHANGELOG.md with my changes
  • I have added/updated relevant documentation
  • I have run "optimize/organize imports" on all changed files
  • I have addressed all analyzer warnings as best I could

@GroovinChip
Copy link
Collaborator

Hello @Otacon and thanks for this PR. I apologize for my lateness in reviewing it.

Does this change hold any implications for SliverToolBar?

@GroovinChip
Copy link
Collaborator

@Otacon checking in

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