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

Add Street cabinet quest #470

Merged
merged 24 commits into from
Nov 1, 2023
Merged

Add Street cabinet quest #470

merged 24 commits into from
Nov 1, 2023

Conversation

mcliquid
Copy link

@mcliquid mcliquid commented Oct 27, 2023

Related StreetComplete issues:
streetcomplete#4505
streetcomplete#3157

@mcliquid
Copy link
Author

See this JOSM preset for translations and values: https://josm.openstreetmap.de/wiki/Presets/StreetCabinet

@mcliquid
Copy link
Author

mcliquid commented Oct 28, 2023

Current implementation:

@mcliquid
Copy link
Author

mcliquid commented Oct 28, 2023

@Helium314 I'm not really happy with the icon - nevertheless you could do the code review. The icon should have grey background to match the others service type quests. But a grey street cabinet on a grey background looks strange. Will have a second look later.
The getHighlightedElements does not work but I don't now why 😫
Current icon:
image

@mcliquid mcliquid marked this pull request as ready for review October 28, 2023 12:23
@Helium314
Copy link
Owner

The getHighlightedElements does not work but I don't now why

Strange, the filter looks ok at first glance. There is no need for the parentheses, but that should not change anything... I'll have time for a closer look tomorrow

@mcliquid
Copy link
Author

@Helium314 When you create a new POI (street cabinet) it's working:
image

@Helium314
Copy link
Owner

For me it always highlights nearby street cabinets, don't know why it doesn't / didn't work for you.

I finally had a look at the quest, and I think the list looks pretty long... some other form would be better here, maybe AImageListQuestForm.
But then you would need to find more icons (some from service building can be re-used). If that's too much work for you I'm also ok with keeping the list.

mcliquid and others added 4 commits October 31, 2023 20:28
remove commas inside when
import de.westnordost.streetcomplete.quests.street_cabinet.StreetCabinetType.*
remove not needed sealed interface StreetCabinetTypeAnswer
@mcliquid
Copy link
Author

@mcliquid
Copy link
Author

I've added some improvements to the UI - from my side this one is ready 👍

@Helium314
Copy link
Owner

The UI looks good, but the icons on the street cabintes are a little small. Does it still look ok if you make the icons larger and move them to the center of the street cabinet (maybe remove the handle)?

street_cabinet_power.xml seems unused

@mcliquid
Copy link
Author

mcliquid commented Nov 1, 2023

@Helium314
Copy link
Owner

Looks a little weird in full size view of Android Studio, but works great on the phone, thanks!

@Helium314
Copy link
Owner

Strings quest_street_cabinet_postal_service_description and quest_street_cabinet_other are not needed, right?

@mcliquid
Copy link
Author

mcliquid commented Nov 1, 2023

Yes, you're right - removed them!

@Helium314 Helium314 merged commit afd1b0c into Helium314:modified Nov 1, 2023
@mcliquid mcliquid deleted the street_cabinet branch November 1, 2023 10:08
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