-
Notifications
You must be signed in to change notification settings - Fork 189
Adding Provinces and Administrative Territories of Pakistan #34
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
base: master
Are you sure you want to change the base?
Conversation
VictorCazanave
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks again for your contribution!
The naming looks good to me 👍
About the city maps, I think you can use the same naming convention as usa.utah package and call it pakistan.karachi.
However if you plan to add many cities, I'm worried that this repository will become messy mixing countries, states, cities... I also thought about it before merging Utah map, but at this time there were only few maps and I didn't expect much more 😅
Maybe the structure of the folders/packages should be reviewed. Or a new repository only for the cities of Pakistan could be created and a link added in this one 🤔
If needed, I can publish the script to generate the JS file from the SVG.
| <svg | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| viewBox="0 0 1628 1544" | ||
| aria-label="Map of Pakistan" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you use the label "Map of Pakistan provinces" or "Map of provinces and territories of Pakistan" please?
| * Replace title by name attributes | ||
| * Use English names | ||
| * Add viewBox | ||
| * Rename ids |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you also indicate that you removed the districts from the original map please?
| @@ -0,0 +1,22 @@ | |||
| { | |||
| "name": "@svg-maps/pakistan.provinces", | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think the package should be called paskitan.administrative-units to be more clear/consistent? 🤔I don't know if people really use this official name or just call them "provinces" .
da10c5a to
4309a09
Compare
Hey Victor,
Here goes, map with provinces and administrative territories. Together they are called administrative units of Pakistan. Let me know if you would like to discuss about the naming convention.
I am also working with a team to go on a drill-down version for individual cities as well. For example, Karachi is gigantic and further divided into multiple districts. How can we cater cities so huge? :D Just a suggestion to use same wrapper you have written but maps for cities.