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

Adding country capitals in bold with a star. #130

Merged
merged 2 commits into from
Jul 28, 2013

Conversation

AndrewBuck
Copy link

capitals - z4

capitals - z6

#country_capitals[zoom>=4][zoom<=15] {
::capital_star[zoom>=4] {
text-face-name:@sans_bold;
text-placement:point;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it's a good habit not to redefine default values (here text-placement: point is the default).
I will add it in the contribution guide.

@yohanboniface
Copy link
Collaborator

How it works from zoom 8 to 15 with place::city?

I think that having a specific layer for the capitals was a good choice, not to retrieve every place=* at low zoom level. But I think we should avoid overlap between layers. As the place layer is already used at low zoom level, the other option is just to add is_capital as a retrieved column.

But at this point, I think the best option for optimization is to have two layers, low_places and places. In low_places we just narrow the WHERE clause to country, state, city, and we add the is_capital column to both the layers, to be able to factorize. We keep the breakpoint at 9 or 10.

Can you do it? If not, I can merge as is and do it myself.

@AndrewBuck
Copy link
Author

I can look into doing something like this, yes. Will try this later today.

@yohanboniface
Copy link
Collaborator

@AndrewBuck Any chance to tackle this on your side?

If not, it's not a problem, I can do it myself.

@yohanboniface yohanboniface merged commit e6e8a29 into hotosm:master Jul 28, 2013
@yohanboniface
Copy link
Collaborator

Done in 1d48f67!

@yohanboniface
Copy link
Collaborator

@AndrewBuck btw, this is a really nice enhancement!

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