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

fix: LateInitializationError when specifying initialCameraFit #1691

Merged

Conversation

TesteurManiak
Copy link
Collaborator

Fix #1684

Copy link
Member

@JaffaKetchup JaffaKetchup left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well this is strange. It seems to work, sometimes. Testing on Windows.
If I put it on my primary display (monitor), it works correctly. If I put it on my secondary display (slightly smaller, my laptop screen), regardless of where VS Code is (so window focus isn't an issue), it still centers over Russia and never applies the camera fit. Also no effect regardless of the window size on the primary display as far as I can see, but a non-full screen window on the secondary display also works.

Not sure if this is related to this, or there's some other reason why it refuses to fit, but I'm using the following fits:

initialCameraFit: CameraFit.insideBounds(
  bounds: LatLngBounds(
    const LatLng(-90, -180),
    const LatLng(90, 180),
  ),
),
// `cameraConstraint` makes no difference

@TesteurManiak
Copy link
Collaborator Author

With such an inconsistent behavior with no clear reason I'm starting to think that maybe we should remove the initialCameraFit and advise to perform a fitCamera in the onMapReady callback until we re-implement it properly 🤔
But no matter what we shouldn't let a runtime exception such as this LateInitializationError going in the package.

@JaffaKetchup
Copy link
Member

Can't we just call the methods in the same place as onMapReady?

I'm very surprised at this behaviour as well, so I'm going to unplug my primary display and just use my laptop screen to see if that resolves the issue as well.

@ibrierley
Copy link
Collaborator

This smells a little like the old "cannot get a size" problem there used to be, where there would be a race condition with the layout not knowing what the size is before it displays and having some hacks for the first frame. That may be a redherring and be totally unrelated though!

@JaffaKetchup
Copy link
Member

JaffaKetchup commented Oct 17, 2023

Here's a screen recording demoing the issue without a second screen complicating things (so all on my laptop/secondary display). It only works correctly when the window is maximized.

2023-10-17.09-47-33.mp4

@TesteurManiak
Copy link
Collaborator Author

@JaffaKetchup @ibrierley I've made a small update, now _applyInitialCameraFit is triggered in the same WidgetsBinding.instance.addPostFrameCallback as _updateAndEmitSizeIfConstraintsChanged, for me it has been working pretty well.

@JaffaKetchup
Copy link
Member

Here's something even stranger. Whether initialCenter and initialZoom are defined and have any effect is different between the two window sizes:

2023-10-17.09-47-33.mp4

It can only assume there is some weird Flutter bug, or something else weird going on, so I think we have to just forget about this issue.

@JaffaKetchup
Copy link
Member

If someone else can verify and approve, that'd be great, as I don't want this to affect the results in any way.

@TesteurManiak
Copy link
Collaborator Author

It can only assume there is some weird Flutter bug, or something else weird going on, so I think we have to just forget about this issue.

Yeah I think we should consider first that this PR will solve the LateInitializationError and take time to investigate this weird behavior in a separate issue if we can reproduce it accurately to confirm if it's coming from the Flutter framework or is due to the big state refactor the package had.

@JaffaKetchup
Copy link
Member

Agreed. It's worth noting that this issue doesn't happen in the v5 example app home page, with the equivalent modifications.

@JaffaKetchup JaffaKetchup dismissed their stale review October 18, 2023 17:15

Testing environment appears to be potentially unstable, or another bug is preventing accurate testing

Copy link
Member

@JaffaKetchup JaffaKetchup left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's get this merged. We can sort out the remaining bug later, but not having it red-box would be nice :D

@JaffaKetchup JaffaKetchup merged commit 83d773d into fleaflet:master Oct 23, 2023
6 checks passed
@TesteurManiak TesteurManiak deleted the fix/#1684_initial-camera-fit branch October 24, 2023 09:36
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.

[BUG] v6: LateInitializationError when specifying initialCameraFit
3 participants