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

Unexpected image cropping in WoltModalSheetPage heroImage #125

Closed
motucraft opened this issue Jan 22, 2024 · 9 comments
Closed

Unexpected image cropping in WoltModalSheetPage heroImage #125

motucraft opened this issue Jan 22, 2024 · 9 comments

Comments

@motucraft
Copy link

Bug report

Describe the bug
An image displayed as the heroImage within a WoltModalSheetPage is unexpectedly cropped around the edges. This issue does not occur when the same image is displayed elsewhere in the application, such as within a Column widget, where it appears correctly without cropping.

Steps to reproduce

Steps to reproduce the behavior:

  1. Run the sample code.
  2. On the main screen, click the 'Modal Bottom Sheet' button to open the modal sheet.
  3. Notice the cropped image at the top of the modal, which serves as the hero image.
Sample Code
import 'package:cached_network_image/cached_network_image.dart';
import 'package:flutter/material.dart';
import 'package:flutter_hooks/flutter_hooks.dart';
import 'package:go_router/go_router.dart';
import 'package:hooks_riverpod/hooks_riverpod.dart';
import 'package:riverpod_annotation/riverpod_annotation.dart';
import 'package:wolt_modal_sheet/wolt_modal_sheet.dart';

part 'main.g.dart';

void main() {
  runApp(const ProviderScope(child: MyApp()));
}

class MyApp extends ConsumerWidget {
  const MyApp({super.key});

  @override
  Widget build(BuildContext context, WidgetRef ref) {
    return MaterialApp.router(
      routerConfig: ref.watch(routerProvider),
      theme: ThemeData(useMaterial3: false),
    );
  }
}

class Home extends HookWidget {
  const Home({super.key});

  @override
  Widget build(BuildContext context) {
    final pageBuilderNotifier =
        useValueNotifier<WoltModalSheetPageListBuilder>((context) => []);

    final width = MediaQuery.of(context).size.width;
    final heroImageHeight = width / 1.9;

    final image = CachedNetworkImage(
      width: width,
      height: heroImageHeight,
      imageUrl: 'https://consumer-static-assets.wolt.com/og_image_mall_web.jpg',
      fit: BoxFit.fitWidth,
    );

    return Scaffold(
      body: Center(
        child: ElevatedButton(
          onPressed: () {
            pageBuilderNotifier.value = (context) => [
                  WoltModalSheetPage(
                    heroImageHeight: heroImageHeight,
                    leadingNavBarWidget: Padding(
                      padding: const EdgeInsets.only(left: 16, top: 16),
                      child: FloatingActionButton(
                        child: const Icon(Icons.close,
                            color: Colors.white, size: 48),
                        onPressed: () => context.pop(),
                      ),
                    ),
                    heroImage: image,
                    child: Column(
                      children: [
                        const Text('Hero image above is cropped.',
                            style: TextStyle(fontSize: 24)),
                        image,
                      ],
                    ),
                  ),
                ];

            context.push('/bottom-sheet', extra: pageBuilderNotifier);
          },
          child: const Text('Modal Bottom Sheet'),
        ),
      ),
    );
  }
}

class SheetPage extends Page<void> {
  final ValueNotifier<WoltModalSheetPageListBuilder> pageListBuilderNotifier;
  final ValueNotifier<int>? pageIndexNotifier;

  const SheetPage({
    required this.pageListBuilderNotifier,
    this.pageIndexNotifier,
  });

  @override
  Route<void> createRoute(BuildContext context) {
    return WoltModalSheetRoute<void>(
      pageListBuilderNotifier: pageListBuilderNotifier,
      pageIndexNotifier: pageIndexNotifier,
      onModalDismissedWithBarrierTap: () => context.pop(),
      routeSettings: this,
    );
  }
}

@riverpod
GoRouter router(RouterRef ref) {
  final router = GoRouter(
    routes: [
      GoRoute(
        path: '/',
        pageBuilder: (_, __) => const MaterialPage(child: Home()),
      ),
      GoRoute(
        path: '/bottom-sheet',
        pageBuilder: (_, state) {
          final extra = state.extra!;
          if (extra is! ValueNotifier<WoltModalSheetPageListBuilder>) {
            throw UnsupportedError('Unsupported parameter. extra=$extra');
          }
          return SheetPage(pageListBuilderNotifier: extra);
        },
      ),
    ],
  );

  ref.onDispose(router.dispose);
  return router;
}
pubspec.yaml
name: wolt_modal_sheet_issue
description: A new Flutter project.
publish_to: 'none'
version: 1.0.0+1

environment:
  sdk: '>=3.2.0 <4.0.0'

dependencies:
  flutter:
    sdk: flutter

  cupertino_icons: ^1.0.2
  hooks_riverpod: ^3.0.0-dev.3
  flutter_hooks: ^0.20.4
  riverpod_annotation: ^3.0.0-dev.3
  wolt_modal_sheet: ^0.3.0
  go_router: ^13.0.1
  cached_network_image: ^3.3.1

dev_dependencies:
  flutter_test:
    sdk: flutter

  flutter_lints: ^3.0.1
  riverpod_generator: ^3.0.0-dev.11
  build_runner: ^2.4.8
  custom_lint: ^0.5.8
  riverpod_lint: ^3.0.0-dev.4

flutter:
  uses-material-design: true

Expected behavior

The expectation is that the heroImage displayed in the WoltModalSheetPage should show the entire image without any cropping


Additional context


@motucraft motucraft added bug Something isn't working in triage labels Jan 22, 2024
@Drugjudy
Copy link

@motucraft hey dude, im having problem recreating the same , getting some error at @override Widget build(BuildContext context, WidgetRef ref) { return MaterialApp.router( routerConfig: ref.watch(routerProvider), theme: ThemeData(useMaterial3: false), ); } } routerProvider as Undefined name 'routerProvider'.
Try correcting the name to one that is defined, or defining the name.dartundefined_identifier
Type: InvalidType so could you help me out a bit, Thanks

@motucraft
Copy link
Author

@Drugjudy
routerProvider is defined in the function annotated with @riverpod. Please try running flutter pub run build_runner build --delete-conflicting-outputs.

@ulusoyca
Copy link
Collaborator

ulusoyca commented Jan 26, 2024

@motucraft This is expected. When you check the source code, you will see that the hero image has animation as the scrolling starts. The image scale animation is from 1.1 to 1.0. It looks like this is something that effects your case. We assumed that 0.1 would not hurt. Can we learn about your case, and understand why it is critical to have exact same width so that I can forward this to our motion designer?

    final double scale =
        WoltLayoutTransformationUtils.calculateTransformationValue(
      startValue: 1.1,
      endValue: 1.0,
      rangeInPx: heroImageHeight - topBarHeight - 8,
      progressInRangeInPx: currentScrollPosition,
    );

@ulusoyca ulusoyca added requires-design and removed bug Something isn't working labels Jan 26, 2024
@motucraft
Copy link
Author

motucraft commented Jan 26, 2024

@ulusoyca
The issue I'm encountering is that the edges of images are being clipped, which creates a problem when the images contain text near the edges. This results in the text being cut off. For example, you wouldn't want the name of the product in the image to be cut off, would you?

In relation to this, I previously commented on a similar issue here: #105 (comment)

(Additionally, I would like to note that I am not planning to use the heroImage property as it results in the images being cropped, which is not suitable for my application's requirements. This further limits my options for working around the issue with the FloatingActionButton.)

At that time, I was advised to use heroImage and heroImageHeight.

@ulusoyca
Copy link
Collaborator

@motucraft By coincidence we have the same problem at Wolt 😮‍💨 We will fix this very soon.

image

@motucraft
Copy link
Author

motucraft commented Feb 29, 2024

@ulusoyca

We will fix this very soon.

Has this issue been scheduled for a fix?

@ulusoyca
Copy link
Collaborator

@motucraft We have been super busy at Wolt with building a design system and library. I am aiming to start working on small PRs including this one tomorrow. Aiming the release for next week.

@motucraft
Copy link
Author

Thanks. I appreciate the hard work and look forward to the upcoming fixes.

@ulusoyca
Copy link
Collaborator

@motucraft this is merged now and published part of 0.5.0

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

No branches or pull requests

4 participants