packages icon indicating copy to clipboard operation
packages copied to clipboard

[go_router_builder] Add ShellRoute support to go_router_builder

Open johnpryan opened this issue 2 years ago • 2 comments

Depends on https://github.com/flutter/packages/pull/2730

Fixes flutter/flutter#111909

johnpryan avatar Feb 22 '23 20:02 johnpryan

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

flutter-dashboard[bot] avatar Feb 22 '23 20:02 flutter-dashboard[bot]

I'm trying to add an example for ShellRouteData, but I'm stuck because the .g.dart file isn't getting generated:

// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

// ignore_for_file: public_member_api_docs

import 'package:flutter/material.dart';
import 'package:go_router/go_router.dart';

part 'shell_route_example.g.dart';

void main() => runApp(App());

class App extends StatelessWidget {
  App({super.key});

  @override
  Widget build(BuildContext context) => MaterialApp.router(
        routerConfig: _router,
      );

  final GoRouter _router = GoRouter(routes: $appRoutes);
}

class HomeScreen extends StatelessWidget {
  const HomeScreen({super.key});

  @override
  Widget build(BuildContext context) => Scaffold(
    appBar: AppBar(title: const Text('foo')),
  );
}

@TypedShellRoute<MyShellRouteData>(
  routes: <TypedRoute<RouteData>>[
    TypedGoRoute<FooRouteData>(path: '/foo'),
    TypedGoRoute<BarRouteData>(path: '/bar'),
  ],
)
class MyShellRouteData extends ShellRouteData {
  const MyShellRouteData();

  @override
  Widget builder(
    BuildContext context,
    GoRouterState state,
    Widget navigator,
  ) {
    return MyShellRouteScreen(child: navigator);
  }
}

class FooRouteData extends GoRouteData {
  const FooRouteData();

  @override
  Widget build(BuildContext context, GoRouterState state) {
    return const FooScreen();
  }
}

class BarRouteData extends GoRouteData {
  const BarRouteData();

  @override
  Widget build(BuildContext context, GoRouterState state) {
    return const BarScreen();
  }
}

class MyShellRouteScreen extends StatelessWidget {
  const MyShellRouteScreen({required this.child, super.key});

  final Widget child;

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      bottomNavigationBar: BottomNavigationBar(
        items: const <BottomNavigationBarItem>[
          BottomNavigationBarItem(
            icon: Icon(Icons.home),
            label: 'Foo',
          ),
          BottomNavigationBarItem(
            icon: Icon(Icons.business),
            label: 'Bar',
          ),
        ],
        onTap: (int index) {
          switch (index) {
            case 0:
              // const FooRouteData().go(context);
              break;
            case 1:
              // const BarRouteData().go(context);
              break;
          }
        },
      ),
    );
  }
}

class FooScreen extends StatelessWidget {
  const FooScreen({super.key});

  @override
  Widget build(BuildContext context) {
    return const Text('Foo');
  }
}

class BarScreen extends StatelessWidget {
  const BarScreen({super.key});

  @override
  Widget build(BuildContext context) {
    return const Text('Bar');
  }
}

johnpryan avatar Feb 23 '23 21:02 johnpryan

Hi @johnpryan Do you know what's the state of this PR?

chunhtai avatar Mar 09 '23 22:03 chunhtai

Sorry, this isn't ready to land yet. I'm currently busy with other projects, if someone else can work on this, feel free.

The issue I mentioned in the comment above is becuase there's no generator for the ShellRoute annotation. To fix it, I think you need to wire it up in lib/go-router_builder.dart, here:

/// Supports `package:build_runner` creation and configuration of
/// `go_router`.
///
/// Not meant to be invoked by hand-authored code.
Builder goRouterBuilder(BuilderOptions options) => SharedPartBuilder(
      const <Generator>[GoRouterGenerator()],
      'go_router',
    );

so you add another GoRouterShellGenerator() or something to that array. I would also like to add more tests and a separate example for ShellRoute, similar to my comment above.

johnpryan avatar Mar 09 '23 23:03 johnpryan

@chunhtai I created a PR https://github.com/johnpryan/flutter_packages/pull/1 against @johnpryan 's branch:

I created a GoRouterShellGenerator by duplicating the code from the GoRouterGenerator. It's exactly the same except that I just replaced TypedGoRoute with TypedShellRoute and GoRouteData with ShellRouteData. I should probably refactor it to avoid having duplicated code. I could:

  1. create an abstract class named (?) which both GoRouterGenerator and GoRouterShellGenerator would implement
  2. add 2 String parameters to GoRouterGenerator (typedRoute and routeData) and create the generator like this: GoRouterGenerator(typedRoute: 'TypedGoRoute', routeData: 'GoRouteData') and GoRouterGenerator(typedRoute: 'TypedShellRoute', routeData: 'ShellRouteData')
  3. (?)

Based on your feedback, I'll add a test similar to the existing one for the GoRouterGenerator.

I've also squashed merged the changes from master to solve the conflicts (pubspec and changelog) but maybe I shouldn't have because now the github action added all the labels...

GP4cK avatar Mar 10 '23 03:03 GP4cK

Hi @GP4cK thanks for helping, can you create a PR directly against the flutter:main? I will close this pr.

chunhtai avatar Mar 10 '23 16:03 chunhtai