woltapp / wolt_modal_sheet

This package provides a responsive modal with multiple pages, motion animation for page transitions, and scrollable content within each page.
MIT License
512 stars 65 forks source link

[Demo App] Add widgetbook to coffee maker demo app #337

Closed ulusoyca closed 5 days ago

ulusoyca commented 3 weeks ago

Description

This PR only adds Widgetbook to demo app for education purposes on how to use Widgetbook

Widgetbook Demo

Related Issues

No related issue.

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

github-actions[bot] commented 3 weeks ago

It appears that this PR does not include any tests. It is recommended to add tests, especially for critical changes, to ensure code quality and prevent regressions. However, if this PR is only updating samples or documentation, feel free to skip adding tests and disregard this comment.

github-actions[bot] commented 3 weeks ago

It appears that this PR does not include any tests. It is recommended to add tests, especially for critical changes, to ensure code quality and prevent regressions. However, if this PR is only updating samples or documentation, feel free to skip adding tests and disregard this comment.

github-actions[bot] commented 3 weeks ago

It appears that this PR does not include any tests. It is recommended to add tests, especially for critical changes, to ensure code quality and prevent regressions. However, if this PR is only updating samples or documentation, feel free to skip adding tests and disregard this comment.

github-actions[bot] commented 3 weeks ago

It appears that this PR does not include any tests. It is recommended to add tests, especially for critical changes, to ensure code quality and prevent regressions. However, if this PR is only updating samples or documentation, feel free to skip adding tests and disregard this comment.

yousinix commented 1 week ago

Hello @ulusoyca 👋

Lucas told me you have some problems with the assets. Here's how to quickly fix it:

  1. Move assets from coffee_maker_navigator_2/lib/assets to coffee_maker_navigator_2/assets.

  2. Add pubspec.yaml to coffee_maker_navigator_2/assets with the following content.

    # coffee_maker_navigator_2/assets/pubspec.yaml
    name: assets
    description: >
      Holds the assets for the app. The assets are included in a
      separate package, to be able to share it with Widgetbook app.
    
    version: 0.0.0
    publish_to: none
    
    environment:
      sdk: ">=3.1.0 <4.0.0"
    
    flutter:
      assets:
        - images/
    1. Remove assets key from your app's pubspec.yaml:

      # coffee_maker_navigator_2/pubspec.yaml
      flutter:
      uses-material-design: true
      
      - assets:
      -   - assets/images/
  3. Add assets dependency to your app's pubspec.yaml:

    # coffee_maker_navigator_2/pubspec.yaml
    dependencies:
      ...
      assets:
        path: assets
  4. Add assets dependency to your widgetbook's pubspec.yaml:

    # coffee_maker_navigator_2/widgetbook/pubspec.yaml
    dependencies:
      ...
      assets:
        path: ../assets
  5. Change all occurrences of AssetImage in your code as follows:

    // Before
    AssetImage('lib/assets/images/order_not_found.webp');
    
    // After
    AssetImage(
      'images/order_not_found.webp',
      package: 'assets',
    );
yousinix commented 1 week ago

I have also realized that your widgets look a bit unstyled, and that's because there was no Theme widget injected in your tree. That's why you need to use the MaterialThemeAddon.

  1. As the addon doesn't have access to the context when you pass the theme, you should remove the context parameter from your `

    # demo_ui_components/lib/src/theme_data/app_theme_data.dart
    class AppThemeData {
    
      static ThemeData themeData() {
        // This should be equal to Theme.of(context).textTheme
        // as it's using the default TextTheme. But this way
        // you don't need a context 
        final textTheme = ThemeData().textTheme;
    
        // ...
      }
    }
  2. Then add this addon to Widgetbook as follows:

    # coffee_maker_navigator_2/widgetbook/lib/main.dart
    import 'package:demo_ui_components/demo_ui_components.dart';
    
    // ...
    
    @widgetbook.App()
    class WidgetbookApp extends StatelessWidget {
      const WidgetbookApp({super.key});
    
      @override
      Widget build(BuildContext context) {
        return Widgetbook.material(
          // ...
          addons: [
            // ...
            MaterialThemeAddon(
              themes: [
                WidgetbookTheme(
                  name: 'Default',
                  data: AppThemeData.themeData(),
                )
              ],
            )
          ],
        );
      }
    }

Alternatively, if you still think that the context is important to your theme, you can still use a custom ThemeAddon as follows:

(In my testing, both had same results, but the solution above is simpler IMO)

ThemeAddon(
  themes: [
    const WidgetbookTheme(
      name: 'Default',
      data: AppThemeData.themeData,
    )
  ],
  themeBuilder: (context, themeBuilder, child) {
    return Theme(
      data: themeBuilder(context),
      child: child,
    );
  },
)
github-actions[bot] commented 1 week ago

It appears that this PR does not include any tests. It is recommended to add tests, especially for critical changes, to ensure code quality and prevent regressions. However, if this PR is only updating samples or documentation, feel free to skip adding tests and disregard this comment.

ulusoyca commented 1 week ago

Hi @yousinix 👋 Thank you for the detailed answer. Can you please check 2237e2561b8d6b6bc0f2c42a24f9b066fc7246e7 ? I still have error.

image
github-actions[bot] commented 1 week ago

It appears that this PR does not include any tests. It is recommended to add tests, especially for critical changes, to ensure code quality and prevent regressions. However, if this PR is only updating samples or documentation, feel free to skip adding tests and disregard this comment.

ulusoyca commented 1 week ago

Hi @yousinix 👋 Thank you for the detailed answer. Can you please check 2237e25 ? I still have error.

image

Ok this worked! I just needed to remove the assets/ from the path

const String _imagePath = 'images/coffee_maker_state';

image
github-actions[bot] commented 1 week ago

It appears that this PR does not include any tests. It is recommended to add tests, especially for critical changes, to ensure code quality and prevent regressions. However, if this PR is only updating samples or documentation, feel free to skip adding tests and disregard this comment.

github-actions[bot] commented 1 week ago

It appears that this PR does not include any tests. It is recommended to add tests, especially for critical changes, to ensure code quality and prevent regressions. However, if this PR is only updating samples or documentation, feel free to skip adding tests and disregard this comment.

github-actions[bot] commented 1 week ago

It appears that this PR does not include any tests. It is recommended to add tests, especially for critical changes, to ensure code quality and prevent regressions. However, if this PR is only updating samples or documentation, feel free to skip adding tests and disregard this comment.

github-actions[bot] commented 1 week ago

It appears that this PR does not include any tests. It is recommended to add tests, especially for critical changes, to ensure code quality and prevent regressions. However, if this PR is only updating samples or documentation, feel free to skip adding tests and disregard this comment.

ulusoyca commented 5 days ago

Thanks @TahaTesser 🙇