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

Render both housenumber and housename #2351

Merged
merged 2 commits into from
Sep 23, 2016

Conversation

sommerluk
Copy link
Collaborator

Render both housenumber and housename if both are present. Same colour for all address-related features. A little bit of documentation.

Fixes #2328
Fixes #735

@nebulon42
Copy link
Contributor

I think that darkening the line colour of address interpolation requires previews.
Why not merge the whole housenumber, housename business into one layer?

@sommerluk
Copy link
Collaborator Author

sommerluk commented Sep 19, 2016

On the left : before – On the right: after
screenshot 1

http://www.openstreetmap.org/#map=18/41.86157/12.56747

@sommerluk
Copy link
Collaborator Author

Why not merge the whole housenumber, housename business into one layer?

Currently housename has line wrap, but housenumber doesn’t. This seems okay for me. I think unifying it – if desired – should be separate PR.

@polarbearing
Copy link
Contributor

Can somebody guide me into what difference I am supposed to see in that left/right comparison?

@sommerluk
Copy link
Collaborator Author

@nebulon42 had requested at #2351 (comment) a preview:

I think that darkening the line colour of address interpolation requires previews.

Indeed the line colour has slightly changed – though the change is almost invisible.

@nebulon42
Copy link
Contributor

Thanks, as you said this seems to be neglectable.

@pnorman
Copy link
Collaborator

pnorman commented Sep 20, 2016

addr:housename is very seldom found in North America, so I don't feel able to review this.

Copy link
Contributor

@nebulon42 nebulon42 left a comment

Choose a reason for hiding this comment

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

Looks quite good to me. Only on z17 the house names might be hard to read because they might overlap with other things. Since this is not different from the house name only rendering I'm not going to suggest it to be changed in this PR.

However, I really think that as follow-up we should

  • give the house numbers and names a faint halo
  • unify house name and house number layers

@nebulon42
Copy link
Contributor

Some screenshots:

z17
z17_a
z17_b

house names only (current rendering)
z17_housename_only

z18
z18
z18_b
z18_c

@sommerluk
Copy link
Collaborator Author

New commit: Unify address layers and use halo.

Screenshot (aritificial example data):
screenshot 1

["addr_housenumber" != null] {
text-name: [addr_housenumber];
["addr_housename" != null] {
text-name: [addr_housenumber] + "\n" + [addr_housename];
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wouldn't this add a spurious \n for nodes with house name and without house number?

Copy link
Contributor

Choose a reason for hiding this comment

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

No, as this line matches only if addr_housenumber is not null. So addr_housenumber and addr_housename must be different from null.

@matthijsmelissen matthijsmelissen merged commit dbf4e25 into gravitystorm:master Sep 23, 2016
@aceman444
Copy link

See #2328. What is the purpose of rendering addr:housename on 10 shops inside a building that itself has that name?

@sommerluk
Copy link
Collaborator Author

@aceman444 We do not render addr:housename (nor addr:housenumber) for shops. We render it only for buildings.

@aceman444
Copy link

OK, that's better. But on a building, the same value would be in 'name' so what is the benefit of rendering addr:housename ?

@sommerluk
Copy link
Collaborator Author

But on a building, the same value would be in 'name'
No. Only 44% of the 280 000 elements with “addr:housename” have also “name”.

so what is the benefit of rendering addr:housename ?
Getting rendered the “addr:housename” on buildings when there is no “name” tag.

As stated in the source code documentation:

Building names (rendered differently from addresses because they are no official postal addresses)

On a building, “name” is not related to postal addresses, but “addr:housename” is. If both are present, “name” has higher priority.

@aceman444
Copy link

'addr:housename' on a building without 'name' makes no sense to me. But it is true the addr: wiki page is ambiguous about when to use addr:housename.

But rendering it may encourage tagging addr:housename instead of name (which I have already seen in the wild).

@sommerluk sommerluk deleted the addresses05 branch July 26, 2017 09:27
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.

None yet

6 participants