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
Add icons for memorial subtags #3356
Merged
Merged
Changes from 19 commits
Commits
Show all changes
21 commits
Select commit
Hold shift + click to select a range
c3864c8
add icons for memorial subtags
jragusa 2c1f2f4
restore monument icon
jragusa 7809e2b
remove war_memorial tag
jragusa cd95225
change folder path
jragusa 3a6699f
change folder path
jragusa d36e33e
Merge branch 'master' into memorial
jragusa 1404814
remove memorial_bench icon
jragusa aee4e02
Merge branch 'memorial' of https://github.com/jragusa/openstreetmap-c…
jragusa 1c3b1d7
render differently artwork_type=statue and change artwork icon
jragusa 408f9f7
remove unnecessary white spaces
jragusa 38519d2
fix pixel-aligned icon of stone
jragusa a3845c9
remove stolperstein
jragusa ad2c48f
change zoom level
jragusa 0b58262
fix memorial_stele
jragusa e096de1
change zoom level of memorial=stone to z18
jragusa 60301c9
fix generic memorial zoom level
jragusa a70e5eb
fix zoom level of text
jragusa f3e38e8
move statue to z17
jragusa 99e06e9
code cleaned
jragusa f1a320a
fix filtering of non-usual memorial tags
jragusa 65eb88a
remove exclusion of statue and add missing comma
jragusa File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
The last issue: I think this should be z17+, since we have no idea what can it be, so it's kind of generic.
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.
Are your sure for
memorial=blue_plaque
? They seem to be not so large. Take a look at some examples from the Guardian.I would put the same zoom level than stolperstein.
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.
I meant all the
memorial
values different than what we have on z18+ and z19+. Look at the similar case here:openstreetmap-carto/amenity-points.mss
Line 474 in 9d16b99
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.
Is the fix satisfy you ?
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, it looks promising and I will test it soon. I just think that excluding statue is not necessary, because it will be rendered on z17+ anyway (that's why I told only about excluding z18+ and z19+ objects). It adds a bit more of clarity, so it's not an error, but I think this line is long enough.
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.
I have found the problem - this war memorial is not showing any icon at all (instead of a standard one), while the label is still visible from z17:
https://www.openstreetmap.org/node/3183626423#map=19/52.26992/20.99322
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.
I resolved both problems. The second one was related to a missing comma.