Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Feat/ animate maps camera #351

Merged
merged 1 commit into from
Oct 28, 2024
Merged

Feat/ animate maps camera #351

merged 1 commit into from
Oct 28, 2024

Conversation

simon-the-shark
Copy link
Member

Nagranie.z.ekranu.2024-10-27.o.23.55.51.mov

@simon-the-shark simon-the-shark added the enhancement New feature or request label Oct 27, 2024
@simon-the-shark simon-the-shark self-assigned this Oct 27, 2024
@simon-the-shark simon-the-shark linked an issue Oct 27, 2024 that may be closed by this pull request
@simon-the-shark simon-the-shark force-pushed the feat/animate-map-camera branch from 2bfbea6 to a8391f1 Compare October 27, 2024 23:02
Copy link
Member

@mikolaj-jalocha mikolaj-jalocha left a comment

Choose a reason for hiding this comment

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

LGTM! Nicely worked-around.

Left few questions.

@@ -27,7 +29,9 @@ class BuildingTile extends ConsumerWidget {
subtitle: context.changeNull(building.addressFormatted),
isActive: isActive,
onTap: () {
ref.read(buildingsMapControllerProvider.notifier).onMarkerTap(building);
unawaited(
Copy link
Member

Choose a reason for hiding this comment

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

Is this unawaited here because of linter complaints, or sth changed in whole logic that now it's async?

Copy link
Member Author

Choose a reason for hiding this comment

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

it wasn't async and now it is due to few reasons:

  1. we do not have controller straight away, we use Completer and await its completion
  2. animation also takes time and is async, and previously camera jump wasn't async

@@ -35,7 +37,9 @@ class BuildingsView extends ConsumerWidget {
point: item.location,
child: GestureDetector(
onTap: () {
ref.read(buildingsMapControllerProvider.notifier).onMarkerTap(item);
unawaited(
Copy link
Member

Choose a reason for hiding this comment

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

Same here?

Copy link
Member Author

Choose a reason for hiding this comment

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

same

fl_map.MapController build() {
return fl_map.MapController();
// nasty, but no other option I think
final _controllerCompleter = Completer<AnimatedMapController>();
Copy link
Member

Choose a reason for hiding this comment

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

Is this Completer here because of build errors? Or could we just create AnimatedMapController.future?

Copy link
Member Author

Choose a reason for hiding this comment

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

Completer is just a native dart way to await somewhere until someother part of the code completes with value (in very imperative way with .complete(value) - so imo ideal way in this example. But sure, there are few alternative ways to handle this. Riverpod provider could be a solution too, but more bulky

@simon-the-shark simon-the-shark merged commit 57b4836 into main Oct 28, 2024
2 checks passed
@simon-the-shark simon-the-shark deleted the feat/animate-map-camera branch October 28, 2024 13:09
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement New feature or request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Animations instead of instant camera jump
2 participants