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

Recover from "scrolling away" from the map #207

Merged
merged 3 commits into from
Feb 9, 2020

Conversation

LDeeJay1969
Copy link
Contributor

Resets the viewport of the client back to 0,0

Resets the viewport of the client back to 0,0
Copy link
Owner

@Kruptein Kruptein left a comment

Choose a reason for hiding this comment

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

Nice to see that you managed to figure it out :), see my two remarks inline

@@ -68,6 +68,12 @@ export function onKeyDown(event: KeyboardEvent): void {
event.preventDefault();
event.stopPropagation();
gameStore.toggleUI();
} else if (event.key === "r" && event.ctrlKey) {
Copy link
Owner

Choose a reason for hiding this comment

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

Ctrl+R is a common keybinding to reload a webpage and I would thus prefer not to override it, as it could be that a user wants to actually hard reload the page if something seems wrong. I would either choose a different combo (be it a different modifier or letter) or even just use the R key without any modifier.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right. But it's difficult to find one that's really unique and maybe not in use by other apps. In the most ideal situation you'd let the user pick a combo. I wouldn't use a non-modified letter since that might interfere with typing actions during 'normal' operations.

Copy link
Owner

Choose a reason for hiding this comment

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

I understand the sentiment, but this is something that should already be properly handled. The 'd' key deselects the current selection, but not if the user is typing in a textfield/textarea as can be seen in lines 21-23

gameStore.setPanX(0);
gameStore.setPanY(0);
sendClientOptions(gameStore.locationOptions);
window.location.reload();
Copy link
Owner

@Kruptein Kruptein Feb 8, 2020

Choose a reason for hiding this comment

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

This unnecessarily reloads the entire page including all the data transfers and bootup logic.
What you want to do instead is tell the canvas to rerender, which you do by telling the various layers that they are no longer valid with: layerManager.invalidate()

(this just pertains to the window.location.reload() line, the other lines are required. I messed up the selection apparently)

Copy link
Contributor Author

@LDeeJay1969 LDeeJay1969 Feb 8, 2020

Choose a reason for hiding this comment

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

Ok, I've updated the code. CTRL-0 (zero) is now the hot-key and the layermanager invalidated. No more reloads. BTW, if I want to write some settings to the db, how would I approach this? (Think user settings/key settings etc) This might also help with #178. I can have a look at this combination if you want.

@Kruptein
Copy link
Owner

Kruptein commented Feb 9, 2020

This looks good to go, one last thing I forgot to mention is to update the CHANGELOG file in the root of the repository with your change. Feel free to add something like [contributed by LDeeJay1969 or something at the end of the line.

@Kruptein Kruptein merged commit 1699200 into Kruptein:dev Feb 9, 2020
@LDeeJay1969 LDeeJay1969 deleted the LDeeJay1969-reset-viewport branch February 9, 2020 19:51
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.

2 participants