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

Capturing camera parameters within the URL #334

Merged
merged 4 commits into from
Dec 21, 2023

Conversation

maxkuznetsov-actionengine
Copy link
Collaborator

No description provided.

Copy link

This pull request is automatically being deployed by Amplify Hosting (learn more).

Access this pull request here: https://pr-334.d3nof3l3x2sso4.amplifyapp.com

Copy link
Collaborator

@belom88 belom88 left a comment

Choose a reason for hiding this comment

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

We have src\utils\url-utils.ts with some logic for tileset param. I'd add there functions viewStateToUrlParams and urlParamsToViewState.

Copy link
Collaborator

@belom88 belom88 left a comment

Choose a reason for hiding this comment

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

Could you add tests for new functions

new URLSearchParams(window.location.search)
);
const { longitude, latitude, pitch, bearing, zoom } = viewState.main;
setSearchParams(
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't like having a state setter outside of react. I was about returning some ready to use object and set it in the component.
We can have it here but need to make a TSDoc comments anyway.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

@@ -57,3 +57,35 @@ const prepareTilesetUrl = (parsedUrl) => {
.concat("layers/0");
return `${parsedUrl.origin}${replacedPathName}${parsedUrl.search}`;
};

export const viewStateToUrlParams = (viewState, setSearchParams) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add TSDoc and tests for new functions

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

);
};

export const urlParamsToViewState = (viewState, setStateUrlViewStateParams) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ditto

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

urlViewStateParams[viewStateParam[0]] = parseFloat(viewStateParam[1]);
}
}
setStateUrlViewStateParams(urlViewStateParams);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ditto: I prefer returning a value and setting the state in the component.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

@maxkuznetsov-actionengine maxkuznetsov-actionengine merged commit 459cf37 into master Dec 21, 2023
3 checks passed
@maxkuznetsov-actionengine maxkuznetsov-actionengine deleted the feature/ES-672_camera_params_url branch December 21, 2023 12:59
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