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

Reset map viewport when returning to Choose project and opening a pro… #211

Merged
merged 1 commit into from
Mar 8, 2025

Conversation

dabreegster
Copy link
Collaborator

@@ -65,6 +65,8 @@ export let currentProjectKey: Writable<string> = writable("");
// False until user activates
export let showAbout: Writable<boolean> = writable(false);

export let appFocus: Writable<"global" | "cnt"> = writable("global");
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Context is hard to access outside of a component, so I moved it to a store. If you prefer, we could instead try a ChooseProjectLink component, so that context is visible

Copy link
Contributor

Choose a reason for hiding this comment

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

A store seems fine!

if (get(appFocus) == "cnt") {
bounds = [-8.943, 54.631, -0.901, 59.489];
}
get(map)?.fitBounds(bounds, { duration: 500 });
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

500 is maybe a little fast, but anything too long gets annoying. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

It feels maybe a little fast (definitely not too slow!), but not unreasonable. Let's go with it and we can always tweak it later.

@@ -154,7 +154,7 @@ export function afterProjectLoaded() {
writable(0),
),
);
get(map)!.fitBounds(get(backend)!.getBounds(), { animate: false });
get(map)!.fitBounds(get(backend)!.getBounds(), { duration: 500 });
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

And the other way: when you open a project, animate the zoom in a bit -- again, quickly.

This zoom happens after the load screen. I wanted to do this during the load screen, but we'd have to mimic the backend getBounds, which isn't too clear to do just from the GJ savefile. We could start storing a bbox in there, but I'm not sure it's worth the hassle.

Copy link
Contributor

Choose a reason for hiding this comment

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

Doing it during loading rather than after would be nice, but I think that could be a low priority followup task.

Copy link
Contributor

@michaelkirk michaelkirk left a comment

Choose a reason for hiding this comment

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

👏

@@ -154,7 +154,7 @@ export function afterProjectLoaded() {
writable(0),
),
);
get(map)!.fitBounds(get(backend)!.getBounds(), { animate: false });
get(map)!.fitBounds(get(backend)!.getBounds(), { duration: 500 });
Copy link
Contributor

Choose a reason for hiding this comment

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

Doing it during loading rather than after would be nice, but I think that could be a low priority followup task.

if (get(appFocus) == "cnt") {
bounds = [-8.943, 54.631, -0.901, 59.489];
}
get(map)?.fitBounds(bounds, { duration: 500 });
Copy link
Contributor

Choose a reason for hiding this comment

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

It feels maybe a little fast (definitely not too slow!), but not unreasonable. Let's go with it and we can always tweak it later.

@@ -65,6 +65,8 @@ export let currentProjectKey: Writable<string> = writable("");
// False until user activates
export let showAbout: Writable<boolean> = writable(false);

export let appFocus: Writable<"global" | "cnt"> = writable("global");
Copy link
Contributor

Choose a reason for hiding this comment

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

A store seems fine!

@dabreegster dabreegster merged commit 21f50fe into main Mar 8, 2025
1 check passed
@dabreegster dabreegster deleted the zoom_transitions branch March 8, 2025 20:50
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.

Zoom out after clicking Choose project
2 participants