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

Make playground maps shareable by persisting data to the URL #18

Merged

Conversation

connorlindsey
Copy link
Contributor

@connorlindsey connorlindsey commented May 15, 2024

Description

  • Stores map data in the url to make maps created in the playground map editor shareable

Resolves #13

Todo

  • Handle id and slug

@connorlindsey connorlindsey marked this pull request as ready for review May 15, 2024 00:32
Comment on lines 26 to 71
const toMapObject = (data: string | null): MapObject | null => {
if (!data) {
return null;
}

try {
return JSON.parse(data);
} catch {
return null;
}
};
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like we should actually parse and validate each field here to match the shape of MapObject, otherwise if data input is wrong it will fail later in the code and is confusing.

Copy link
Contributor Author

@connorlindsey connorlindsey May 16, 2024

Choose a reason for hiding this comment

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

Do you have a preferred strategy for the validation? We could introduce something like zod or just add some lightweight validation for each field checking if it's present and if not, returning a default/empty value. I'd lean towards the latter in this case.

Copy link
Contributor

@cpojer cpojer left a comment

Choose a reason for hiding this comment

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

This is looking great. I'm sorry for the slow review as I'm at a conference right now.

I added a few comments with minor modifications, do you mind doing an update?

@connorlindsey
Copy link
Contributor Author

@cpojer No rush, and yes, I can definitely update it! All good feedback

@cpojer cpojer force-pushed the add-share-link-to-playground-map-editor branch from 0c876cf to d89db7d Compare May 16, 2024 19:17
@cpojer cpojer merged commit 1c07471 into nkzw-tech:main May 16, 2024
2 checks passed
@cpojer
Copy link
Contributor

cpojer commented May 16, 2024

Thanks for the contribution, this is great!

@connorlindsey connorlindsey deleted the add-share-link-to-playground-map-editor branch May 16, 2024 19:24
tjamesmac pushed a commit to tjamesmac/athena-crisis that referenced this pull request May 17, 2024
cpojer added a commit that referenced this pull request May 19, 2024
Co-authored-by: Connor Lindsey <[email protected]>
GitOrigin-RevId: 80499df94c87243652a05c36fa5c43bd079fae6b
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.

[Open Source] Add share link to the Map Editor
2 participants