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

Implement some Australian parking signs and presets #44

Closed
wants to merge 5 commits into from

Conversation

jakecoppinger
Copy link
Contributor

@jakecoppinger jakecoppinger commented Sep 1, 2021

This PR implements Australian parking signs and some of the corresponding presets.

They're fairly different to European signs so I've found the app quite confusing thus far.

I've set the name of the current signs to be Russian - I'm not sure if this is the case, I just noted that the Wikipedia URL of the icons has Russia in it. Please let me know if I should change Russia to Europe.

Parking signs in Australia are represented by a sign taller
than wide.

I've found it very confusing to map parking in Australia using
European style parking signs.

I'm unsure if the existing signs are Russia specific or applicable to
Europe - I named them "Russian" as the URL points to Russia Wikipedia.
Also improve country title styling
Also show `:default` all the time
@jakecoppinger
Copy link
Contributor Author

Screen Shot 2021-09-01 at 7 47 51 pm

Example of the new preset picker

@zlant
Copy link
Owner

zlant commented Sep 2, 2021

Very big editor panel. It is not user friendly. We need to think about a solution.

@riQQ
Copy link

riQQ commented Sep 2, 2021

I checked Germany, Denmark and Belgium. They all have the first two (no_stopping and no_parking), the third and fourth seem to be Russia / former Soviet union only.

@tordans
Copy link
Collaborator

tordans commented Sep 2, 2021

A quick solution to declutter the UI would be to create a preset or traffic sign select option above the colored editor areas. One could switch between Russia (Default) and Australia and only see the corresponding presets.


A much more generic solution would be, to use this opportunity and create a country-traffic-sign-osm-peset-collection. We could collect sign + associated tags per region. The interface could then use https://github.com/ideditor/country-coder to select the presets automatically. Having such a preset-collection with traffic sign images would IMO be a worthy effort that other osm projects could benefit from as well.

@jakecoppinger
Copy link
Contributor Author

jakecoppinger commented Sep 2, 2021

Thanks for all the feedback. I really like that idea of a preset collection and an option to change traffic sign preset region. I agree, the extra space taken up is far from ideal!

I feel quite uneasy when making changes as I don't know the codebase well, and there aren't any unit tests or type checking. I was thinking of migrating the repo to Typescript before making larger changes - would you be comfortable with that? I understand it'd be a fairly big change and you might prefer not. It'd be really cool to have typescript interfaces and types for the OSM tags to make sure everything is accounted for.

I've got a bunch of commits on https://github.com/jakecoppinger/open-parking-map/tree/typescript but I broke something in the process so I'm planning to have another go while being more methodical.

@zlant
Copy link
Owner

zlant commented Sep 3, 2021

I was thinking of migrating the repo to Typescript before making larger changes - would you be comfortable with that?

I think this is the right way. But won't this increase the entry threshold for those who do not know typescript?

@jakecoppinger
Copy link
Contributor Author

But won't this increase the entry threshold for those who do not know typescript?

Possibly - though if somebody wasn't comfortable with Typescript types they could use the any type or // @ts-ignore and then write plain JavaScript, with no less safety than currently exists.

On the plus side, newcomers (like me!) would be more confident their changes aren't breaking something.

@jakecoppinger
Copy link
Contributor Author

@zlant I created a Typescript PR: #46

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.

4 participants