From f6c511e39076417bb9e7b1c7c01bc65abb524d62 Mon Sep 17 00:00:00 2001 From: Helium314 Date: Wed, 26 Jul 2023 18:57:54 +0200 Subject: [PATCH 1/4] ask AddCrossing for sidwalk and crossing ways --- .../quests/crossing/AddCrossing.kt | 6 ------ .../quests/crossing/AddCrossingForm.kt | 21 +++++++++++++++++-- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/crossing/AddCrossing.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/crossing/AddCrossing.kt index 835d2a7d40..073346b171 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/quests/crossing/AddCrossing.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/quests/crossing/AddCrossing.kt @@ -25,16 +25,10 @@ class AddCrossing : OsmElementQuestType { private val footwaysFilter by lazy { """ ways with (highway ~ footway|steps or highway ~ path|cycleway and foot ~ designated|yes) - and footway !~ sidewalk|crossing and area != yes and access !~ private|no """.toElementFilterExpression() } - /* It is neither asked for sidewalks nor crossings (=separately mapped sidewalk infrastructure) - * because a "no" answer would require to also delete/adapt the crossing ways, rather than just - * tagging crossing=no on the vertex. - * See https://github.com/streetcomplete/StreetComplete/pull/2999#discussion_r681516203 */ - override val changesetComment = "Specify whether there are crossings at intersections of paths and roads" override val wikiLink = "Tag:highway=crossing" override val icon = R.drawable.ic_quest_pedestrian diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/crossing/AddCrossingForm.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/crossing/AddCrossingForm.kt index 731ef122b7..c1f8395577 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/quests/crossing/AddCrossingForm.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/quests/crossing/AddCrossingForm.kt @@ -1,15 +1,32 @@ package de.westnordost.streetcomplete.quests.crossing import de.westnordost.streetcomplete.R +import de.westnordost.streetcomplete.data.osm.edits.MapDataWithEditsSource import de.westnordost.streetcomplete.quests.AListQuestForm import de.westnordost.streetcomplete.quests.TextItem import de.westnordost.streetcomplete.quests.crossing.CrossingAnswer.* +import org.koin.android.ext.android.inject class AddCrossingForm : AListQuestForm() { + private val mapDataSource: MapDataWithEditsSource by inject() - override val items = listOf( + override val items get() = listOfNotNull( TextItem(YES, R.string.quest_crossing_yes), TextItem(NO, R.string.quest_crossing_no), - TextItem(PROHIBITED, R.string.quest_crossing_prohibited), + if (isOnSidewalkOrCrossing()) null else TextItem(PROHIBITED, R.string.quest_crossing_prohibited) ) + + /* PROHIBITED is neither available for sidewalks nor crossings (=separately mapped sidewalk infrastructure) + * because a "no" answer would require to also delete/adapt the crossing ways, rather than just + * tagging crossing=no on the vertex. + * See https://github.com/streetcomplete/StreetComplete/pull/2999#discussion_r681516203 + * and https://github.com/streetcomplete/StreetComplete/issues/5160 */ + + private fun isOnSidewalkOrCrossing(): Boolean { + val ways = mapDataSource.getWaysForNode(element.id) + return ways.any { + val footway = it.tags["footway"] + footway == "sidewalk" || footway == "crossing" + } + } } From 4a0d34293f8ab24b6b11f1eb3da30c34c7c802a6 Mon Sep 17 00:00:00 2001 From: Helium314 Date: Sat, 26 Aug 2023 07:35:36 +0200 Subject: [PATCH 2/4] ask user to leave a note when crossing with a sidewalk or crossing path is not possible --- .../streetcomplete/quests/AListQuestForm.kt | 2 ++ .../quests/crossing/AddCrossingForm.kt | 21 ++++++++++++++++--- app/src/main/res/values/strings.xml | 1 + 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/AListQuestForm.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/AListQuestForm.kt index 196e9bd321..44df54511f 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/quests/AListQuestForm.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/quests/AListQuestForm.kt @@ -19,6 +19,8 @@ abstract class AListQuestForm : AbstractOsmQuestForm() { private val radioButtonIds = HashMap>() + val checkedItem get() = radioButtonIds[binding.radioButtonGroup.checkedRadioButtonId] + override fun onViewCreated(view: View, savedInstanceState: Bundle?) { super.onViewCreated(view, savedInstanceState) for (item in items) { diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/crossing/AddCrossingForm.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/crossing/AddCrossingForm.kt index c1f8395577..dbc3ce42a4 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/quests/crossing/AddCrossingForm.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/quests/crossing/AddCrossingForm.kt @@ -1,5 +1,6 @@ package de.westnordost.streetcomplete.quests.crossing +import androidx.appcompat.app.AlertDialog import de.westnordost.streetcomplete.R import de.westnordost.streetcomplete.data.osm.edits.MapDataWithEditsSource import de.westnordost.streetcomplete.quests.AListQuestForm @@ -10,17 +11,31 @@ import org.koin.android.ext.android.inject class AddCrossingForm : AListQuestForm() { private val mapDataSource: MapDataWithEditsSource by inject() - override val items get() = listOfNotNull( + override val items get() = listOf( TextItem(YES, R.string.quest_crossing_yes), TextItem(NO, R.string.quest_crossing_no), - if (isOnSidewalkOrCrossing()) null else TextItem(PROHIBITED, R.string.quest_crossing_prohibited) + TextItem(PROHIBITED, R.string.quest_crossing_prohibited) ) - /* PROHIBITED is neither available for sidewalks nor crossings (=separately mapped sidewalk infrastructure) + /* PROHIBITED is neither possible for sidewalks nor crossings (=separately mapped sidewalk infrastructure) * because a "no" answer would require to also delete/adapt the crossing ways, rather than just * tagging crossing=no on the vertex. + * This situation needs to be solved in a different editor, so we ask the user to leave a note. * See https://github.com/streetcomplete/StreetComplete/pull/2999#discussion_r681516203 * and https://github.com/streetcomplete/StreetComplete/issues/5160 */ + override fun isFormComplete(): Boolean { + if (checkedItem?.value == PROHIBITED && isOnSidewalkOrCrossing()) { + AlertDialog.Builder(requireContext()) + .setMessage(R.string.quest_crossing_prohibited_but_on_sidewalk_or_crossing) + .setPositiveButton(R.string.quest_leave_new_note_yes) { _, _ -> composeNote() } + .setNegativeButton(android.R.string.cancel, null) + .show() + return false + } + return super.isFormComplete() + } + + override fun isRejectingClose() = super.isFormComplete() private fun isOnSidewalkOrCrossing(): Boolean { val ways = mapDataSource.getWaysForNode(element.id) diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 6bf71827db..4248cacb21 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -931,6 +931,7 @@ Before uploading your changes, the app checks with a <a href=\"https://www.we Yes (e.g. the curb is lowered here or there are markings) No, but crossing is possible No, crossing is prohibited or impossible + This position is on a sidewalk or crossing. Please leave a note to explain the situation. What kind of crossing is this? Controlled by traffic lights From 6866c63b5e55e7336e65b0690e6999adc1fdff3b Mon Sep 17 00:00:00 2001 From: Tobias Zwick Date: Fri, 15 Sep 2023 12:57:44 +0200 Subject: [PATCH 3/4] do not make isFormComplete have side-effects --- .../streetcomplete/quests/AListQuestForm.kt | 2 +- .../quests/crossing/AddCrossingForm.kt | 35 ++++++++++--------- 2 files changed, 19 insertions(+), 18 deletions(-) diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/AListQuestForm.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/AListQuestForm.kt index 44df54511f..ab3369d7ab 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/quests/AListQuestForm.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/quests/AListQuestForm.kt @@ -40,7 +40,7 @@ abstract class AListQuestForm : AbstractOsmQuestForm() { } override fun onClickOk() { - applyAnswer(radioButtonIds.getValue(binding.radioButtonGroup.checkedRadioButtonId).value) + applyAnswer(checkedItem!!.value) } override fun isFormComplete() = binding.radioButtonGroup.checkedRadioButtonId != -1 diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/crossing/AddCrossingForm.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/crossing/AddCrossingForm.kt index dbc3ce42a4..28bfd8a270 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/quests/crossing/AddCrossingForm.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/quests/crossing/AddCrossingForm.kt @@ -11,37 +11,38 @@ import org.koin.android.ext.android.inject class AddCrossingForm : AListQuestForm() { private val mapDataSource: MapDataWithEditsSource by inject() - override val items get() = listOf( + override val items = listOf( TextItem(YES, R.string.quest_crossing_yes), TextItem(NO, R.string.quest_crossing_no), - TextItem(PROHIBITED, R.string.quest_crossing_prohibited) + TextItem(PROHIBITED, R.string.quest_crossing_prohibited), ) - /* PROHIBITED is neither possible for sidewalks nor crossings (=separately mapped sidewalk infrastructure) - * because a "no" answer would require to also delete/adapt the crossing ways, rather than just - * tagging crossing=no on the vertex. - * This situation needs to be solved in a different editor, so we ask the user to leave a note. - * See https://github.com/streetcomplete/StreetComplete/pull/2999#discussion_r681516203 - * and https://github.com/streetcomplete/StreetComplete/issues/5160 */ - override fun isFormComplete(): Boolean { + /* PROHIBITED is not possible for sidewalks or crossings (=separately mapped sidewalk + infrastructure) because if the crossing does not exist, it would require to also + delete/adapt the crossing ways, rather than just tagging crossing=no on the vertex. + + This situation needs to be solved in a different editor, so we ask the user to leave a note. + See https://github.com/streetcomplete/StreetComplete/pull/2999#discussion_r681516203 + and https://github.com/streetcomplete/StreetComplete/issues/5160 + + NO on the other hand would be okay because crossing=informal would not require deleting + the crossing ways (I would say... it is in edge case...) + */ + override fun onClickOk() { if (checkedItem?.value == PROHIBITED && isOnSidewalkOrCrossing()) { AlertDialog.Builder(requireContext()) .setMessage(R.string.quest_crossing_prohibited_but_on_sidewalk_or_crossing) .setPositiveButton(R.string.quest_leave_new_note_yes) { _, _ -> composeNote() } .setNegativeButton(android.R.string.cancel, null) .show() - return false + } else { + super.onClickOk() } - return super.isFormComplete() } - override fun isRejectingClose() = super.isFormComplete() - - private fun isOnSidewalkOrCrossing(): Boolean { - val ways = mapDataSource.getWaysForNode(element.id) - return ways.any { + private fun isOnSidewalkOrCrossing(): Boolean = + mapDataSource.getWaysForNode(element.id).any { val footway = it.tags["footway"] footway == "sidewalk" || footway == "crossing" } - } } From 337c5779e204abfc8d752d67672b85f051a3260b Mon Sep 17 00:00:00 2001 From: Tobias Zwick Date: Fri, 15 Sep 2023 13:03:26 +0200 Subject: [PATCH 4/4] improve wording: For the user, it is irrelevant and confusing to read that "a position is on a sidewalk or crossing" --- .../streetcomplete/quests/crossing/AddCrossingForm.kt | 2 +- app/src/main/res/values/strings.xml | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/crossing/AddCrossingForm.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/crossing/AddCrossingForm.kt index 28bfd8a270..4a668e68e3 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/quests/crossing/AddCrossingForm.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/quests/crossing/AddCrossingForm.kt @@ -31,7 +31,7 @@ class AddCrossingForm : AListQuestForm() { override fun onClickOk() { if (checkedItem?.value == PROHIBITED && isOnSidewalkOrCrossing()) { AlertDialog.Builder(requireContext()) - .setMessage(R.string.quest_crossing_prohibited_but_on_sidewalk_or_crossing) + .setMessage(R.string.quest_leave_new_note_as_answer) .setPositiveButton(R.string.quest_leave_new_note_yes) { _, _ -> composeNote() } .setNegativeButton(android.R.string.cancel, null) .show() diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 4248cacb21..d984bd174a 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -131,6 +131,8 @@ The info you enter is directly added to OpenStreetMap in your name, without the "You can leave a public note for other mappers to resolve at this location, or hide this quest for yourself only" "Leave note" "Hide" + In this case, you need to leave a note in which you explain the situation. + "Other answers…" @@ -931,7 +933,6 @@ Before uploading your changes, the app checks with a <a href=\"https://www.we Yes (e.g. the curb is lowered here or there are markings) No, but crossing is possible No, crossing is prohibited or impossible - This position is on a sidewalk or crossing. Please leave a note to explain the situation. What kind of crossing is this? Controlled by traffic lights