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

fix: geo panel issue #411

Merged
merged 3 commits into from
Aug 26, 2024
Merged

fix: geo panel issue #411

merged 3 commits into from
Aug 26, 2024

Conversation

islxyqwe
Copy link
Member

@islxyqwe islxyqwe commented Aug 26, 2024

  1. fixed the geoKey won't be saved when geoData not set.
    image
  2. change the default World Countries data.
  3. added the preview when selecting preset geo data.
    image
    image

Copy link

vercel bot commented Aug 26, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
graphic-walker ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 26, 2024 8:58am

Copy link
Contributor

github-actions bot commented Aug 26, 2024

Risk Level 2 - /home/runner/work/graphic-walker/graphic-walker/packages/graphic-walker/src/components/leafletRenderer/geoConfigPanel.tsx

The code changes in this file are mostly additions of new features and minor modifications. There are no apparent bugs or security issues. However, there are a few areas where the code could be improved for readability and maintainability.

  1. Use of magic numbers: The numbers -1 and -2 are used in several places in the code. It would be better to replace these with named constants to improve readability.

  2. Error handling: In the handleSubmit function, there is a try-catch block where the error is simply logged to the console. It would be better to handle the error in a more user-friendly way, such as showing an error message to the user.

  3. Code duplication: The code to set showGeoJSONConfigPanel to false is duplicated in several places. This could be extracted into a separate function to reduce duplication.

Example code snippet for the third point:

const closeGeoJSONConfigPanel = () => {
    vizStore.setShowGeoJSONConfigPanel(false);
};

// Then replace all instances of `vizStore.setShowGeoJSONConfigPanel(false);` with `closeGeoJSONConfigPanel();`

🔮💡🔁


Powered by Code Review GPT

@islxyqwe islxyqwe merged commit 436e7bf into main Aug 26, 2024
6 checks passed
islxyqwe added a commit that referenced this pull request Aug 27, 2024
* fix: geo panel issue

* fix: geoList in playground

* fix: geoList
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