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

[BUG] Repositioning the map through MapController in onMapReady prevents initial tiles loading #1507

Closed
MatyasK opened this issue May 8, 2023 · 9 comments
Labels
bug This issue reports broken functionality or another error

Comments

@MatyasK
Copy link

MatyasK commented May 8, 2023

What is the bug?

After migrating to v4 the tiles doesn't load. if you move the map around manually the tiles start to load.

How can we reproduce it?

class CcaMapWidget extends StatelessWidget with CcaLogger {
  CcaMapWidget({
    required this.tripLegs,
    MapController? mapController,
    PopupController? popupLayerController,
    super.key,
  })  : _mapController = mapController ?? MapController(),
        _popupLayerController = popupLayerController ?? PopupController();

  final MapController _mapController;
  final PopupController _popupLayerController;
  final List<TripDetailLeg> tripLegs;

  @override
  Widget build(BuildContext context) {
    return BlocProvider(
      create: (context) => CcaMapCubit()..screenLoaded(tripLegs),
      child: BlocBuilder<CcaMapCubit, CcaMapState>(
        builder: (context, state) {
          if (state.status == MapStatus.loading) {
            return const CcaLoadingIndicator();
          }

          return FlutterMap(
            mapController: _mapController,
            options: MapOptions(
              maxZoom: 13,
              onMapReady: () {
                final markerBounds = LatLngBounds.fromPoints(state.points);

                _mapController.fitBounds(
                  markerBounds,
                  options: const FitBoundsOptions(padding: EdgeInsets.all(70)),
                );
              },
            ),
            children: [
              TileLayer(
                urlTemplate: 'https://api.tomtom.com/map/1/tile/basic/main/'
                    '{z}/{x}/{y}.png?key={apiKey}',
                additionalOptions: {
                  'apiKey': getIt<UserRepository>().tomtomApiKey,
                },
                errorTileCallback: (image, error, stackTrace) {
                  logger.warning('Error: $error, $stackTrace');
                },
              ),
              PolylineLayer(
                polylines: [
                  Polyline(
                    strokeWidth: 3,
                    points: state.points,
                    color: kcPrimary,
                  ),
                ],
              ),
              PopupMarkerLayerWidget(
                options: PopupMarkerLayerOptions(
                  popupController: _popupLayerController,
                  markers: state.waypoints
                      .map((e) => WaypointMarker(wayPoint: e))
                      .toList(),
                  markerRotateAlignment:
                      PopupMarkerLayerOptions.rotationAlignmentFor(
                    AnchorAlign.top,
                  ),
                  popupBuilder: (BuildContext context, Marker marker) {
                    if (marker is WaypointMarker) {
                      return WaypointMarkerPopup(wayPoint: marker.wayPoint);
                    }

                    return const Card(child: Text('Not a waypoint'));
                  },
                ),
              ),
            ],
          );
        },
      ),
    );
  }
}

Do you have a potential solution?

No response

Platforms

Android

Severity

Obtrusive: Prevents normal functioning but causes no errors in the console

@MatyasK MatyasK added bug This issue reports broken functionality or another error needs triage This new bug report needs reproducing and prioritizing labels May 8, 2023
@JaffaKetchup
Copy link
Member

Hey @MatyasK,
Can you please strip down your code to create an MRE? At the moment, it's difficult for us to test, as there's lots of state management surrounding it.

@MatyasK
Copy link
Author

MatyasK commented May 9, 2023

I created a sample app, the app bevahes the same way, so the map only loads after you move the camera around. (I created a test api key so you can use that).

import 'package:flutter/material.dart';
import 'package:flutter_map/plugin_api.dart';
import 'package:latlong2/latlong.dart';

void main() {
  runApp(const MyApp());
}

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

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      title: 'Flutter Demo',
      theme: ThemeData(
      ),
      home: MapView(),
    );
  }
}

class MapView extends StatelessWidget {
  MapView({Key? key}) : super(key: key);

  final MapController _mapController = MapController();
  final mapPoints = <LatLng>[
    LatLng(51.5, -0.09),
    LatLng(53.3498, -6.2603),
    LatLng(48.8566, 2.3522),
    LatLng(52.5200, 13.4050),
    LatLng(51.5074, -0.1278),
  ];

  @override
  Widget build(BuildContext context) {
    return Scaffold(
        appBar: AppBar(title: const Text('Map View')),
        body: FlutterMap(
          mapController: _mapController,
          options: MapOptions(
            maxZoom: 13,
            onMapReady: () {
              final markerBounds = LatLngBounds.fromPoints(mapPoints);

              _mapController.fitBounds(
                markerBounds,
                options: const FitBoundsOptions(padding: EdgeInsets.all(70)),
              );
            },
          ),
          children: [
            TileLayer(
              urlTemplate: 'https://api.tomtom.com/map/1/tile/basic/main/'
                  '{z}/{x}/{y}.png?key={apiKey}',
              additionalOptions: const {
                'apiKey': 'FnfFpG2MWmBYXNsvAFOAyJCJB7ftnoGd',
              },
            ),
            PolylineLayer(
              polylines: [
                Polyline(
                  strokeWidth: 3,
                  points: mapPoints,
                  color: Colors.green,
                ),
              ],
            ),
         
          ],
        ));
  }
}

@JaffaKetchup JaffaKetchup added non-fatal and removed needs triage This new bug report needs reproducing and prioritizing labels May 9, 2023
@JaffaKetchup
Copy link
Member

Reproducible. The issue seems to be with running the fitBounds call when the map loads. Can you use bounds in MapOptions instead?

@MatyasK
Copy link
Author

MatyasK commented May 9, 2023

@JaffaKetchup That solves the issue, thanks for your quick reply!

@JaffaKetchup JaffaKetchup changed the title [BUG] Tiles doesn't load after migrating to V4 [BUG] Repositioning the map through MapController in onMapReady prevents tiles loading May 9, 2023
@JaffaKetchup JaffaKetchup changed the title [BUG] Repositioning the map through MapController in onMapReady prevents tiles loading [BUG] Repositioning the map through MapController in onMapReady prevents initial tiles loading May 9, 2023
@JaffaKetchup
Copy link
Member

This is likely related to #1475.
@ibrierley @mootw @TesteurManiak Is this issue worth fixing? Users shouldn't setup their map in this way - except from when they need data from asynchronous sources first. But in that case, they can just delay the whole map from loading.

@TesteurManiak
Copy link
Contributor

I agree with you @JaffaKetchup. Maybe we should mention something about this in the documentation of either MapController or onMapReady.

@ibrierley
Copy link
Contributor

hmm I think it depends if this is part of a more subtle bug. I think its fine to leave this as is for this current issue (i.e there's a more correct fix).

But I'm also wondering if its related to something like some bits in rorystephenson@a13353f (it may also be completely unrelated, just maybe the first place I'd look by the description of the problem). eg

Remove unnecessary setState in FlutterMapState’s emitMapEvent. The
event emitting does not change the map's state and was causing double
calls to setState in some code paths.

  • Removed the workaround for the initial map size being zero in TileLayer
    which caused tiles to not be loaded. The new MapEventNonRotatedSizeChange
    event removes the need for this workaround as during startup either:
    - The platform has already set flutter's resolution and thus all
    tiles are successfully loaded.
    - The platform has not set flutter's resolution but when it does it
    will cause LayoutBuilder to rebuild with the new size which will
    trigger a MapEventNonRotatedSizeChange event. This event will in
    turn trigger a tile load in TileLayer.

For example, if there's a condition where previously we didn't have a size in the layout builder, and previously didn't send a map ready event until the size was resolved but now we do. We may find there are some other sneaky bugs that come in where people are doing things on a map ready when it isn't correctly sized or tiles loaded. I don't know this is the case at all, just kinda worth being aware that it could be part of a larger issue that pops up a few times. The whole layoutbuilder sizing thing has always been a bit of a pita :).

@JaffaKetchup
Copy link
Member

just kinda worth being aware that it could be part of a larger issue that pops up a few times. The whole layoutbuilder sizing thing has always been a bit of a pita :).

Absolutely, whatever anyone does, it never seems to properly fix it!

Maybe we should mention something about this in the documentation of either MapController or onMapReady.

I'll mention it in https://docs.fleaflet.dev/usage/controller#usage-in-initstate - on a related note, I've recently changed the guidance for MapController in initState again to use onMapReady, because some people were having issues with the SchedulerBinding methods, and it's more clear now.

I think its fine to leave this as is for this current issue (i.e there's a more correct fix).

Maybe this should be converted to a general tracking issue then, for issues with initial sizing?

@JaffaKetchup
Copy link
Member

Closing for now. More specific issues can be opened as necessary.
Fixed (temporarily-ish?) through documentation update.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue reports broken functionality or another error
Projects
None yet
Development

No branches or pull requests

4 participants