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

Flat field added to apartment building #7896

Merged
merged 4 commits into from
Aug 20, 2020

Conversation

ogbeche77
Copy link
Contributor

@ogbeche77 ogbeche77 commented Aug 12, 2020

Hey Maintainers, I am new here and willing to contribute to this community.
I observed that this issue not been resolved, #7856 so I went ahead to implement the change as described by @TheAdventurer64.
A flat field was added to indicate how many flats are in an apartment building. If label needs to be reworded/rephrased, I will gladly do that again.
Cheers

Copy link
Collaborator

@quincylvania quincylvania left a comment

Choose a reason for hiding this comment

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

Looks pretty good, @ogbeche77, just a couple changes needed before we can merge.

data/presets/fields/flats.json Outdated Show resolved Hide resolved
"key": "flats",
"type": "number",
"minValue": 0,
"label": "Flats",
Copy link
Collaborator

Choose a reason for hiding this comment

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

So iD's default language is American English. Could we make the label "Units" instead?

@quincylvania quincylvania added the preset An issue with an OpenStreetMap preset or tag label Aug 17, 2020
@ogbeche77
Copy link
Contributor Author

Hi @quincylvania
Thank you for the feedback. I have made the changes as requested.

@@ -1,5 +1,8 @@
{
"icon": "maki-building",
"fields": [
"building/flats"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh, you should also have "{building}" here the way you did before.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So it should be "fields": [ "{building}"] ? Without "flats"? or "fields": [ "{building}","flats"] ?

I think I need more time to get more familiar with the code base :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh.. I think I figured it out. It should be "fields": [ "{building}", "building/flats" ]
:)

@quincylvania quincylvania merged commit 2d270f8 into openstreetmap:develop Aug 20, 2020
@quincylvania quincylvania added this to the 2.18.5 milestone Aug 20, 2020
@quincylvania
Copy link
Collaborator

@ogbeche77 Looks great, thanks for the contribution and for working through the technicalities! Welcome to iD development.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
preset An issue with an OpenStreetMap preset or tag
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants