Skip to content

Collect guidepost/map metadata (ref,ele,name)#477

Merged
Helium314 merged 8 commits intoHelium314:modifiedfrom
qugebert:guidepost_metadata
Dec 25, 2023
Merged

Collect guidepost/map metadata (ref,ele,name)#477
Helium314 merged 8 commits intoHelium314:modifiedfrom
qugebert:guidepost_metadata

Conversation

@qugebert
Copy link
Copy Markdown

@qugebert qugebert commented Oct 29, 2023 •
edited
Loading

closes #476

Three additional quests to collect name,ref and elevation of information=guidepost or information=map.

guidepost_screenshots

@qugebert qugebert marked this pull request as draft November 18, 2023 11:05
@mcliquid
Copy link
Copy Markdown

mcliquid commented Nov 20, 2023

@qugebert What's missing / How could I support you here?

@qugebert
Copy link
Copy Markdown
Author

qugebert commented Nov 20, 2023

@qugebert What's missing / How could I support you here?
Sorry, i just forgot this PR, and yesterday i noticed there have been many concerns against this so i first need to get an idea which parts of this still would be accepted and what i need to change.

@Helium314
Copy link
Copy Markdown
Owner

Helium314 commented Nov 20, 2023 •
edited
Loading

Some short summary of what I remember after quickly going over the discussion:

  • an ele quest should be fine, but needs a clear warning about not using GPS, and only entering plausible data (as per the warning in the wiki)
  • name is probably fine too, though on the other hand users can also modify the place name quest. Should still clarify what a name for a guidepost is, as there are many without a sign like https://wiki.openstreetmap.org/wiki/File:Signpost.jpg, but with a lot of other Text people may use as a name.
  • For ref it should be clarified what goes in there. Looking at random places I found nearby routes, UTM coordinates, distances (like track km) and what appears to be refs for the guidepost itself. Usually this differs by region, probably because of how they are signed, and possibly also because personal style of individual very active mappers.

@Helium314
Copy link
Copy Markdown
Owner

Helium314 commented Dec 21, 2023

Considering streetcomplete#5412, this PR should be done before the mentioned changes.
Do you want to continue here, or do you need anything from me?

The "technical" part seems to be mostly ok, except for the ele:signed part and the inclusion of maps (very low usage of those tags, not documented).

What's left is clarifications about what a guidepost ref or name is / should (not) contain, and the warning about ele.

@qugebert
Copy link
Copy Markdown
Author

qugebert commented Dec 22, 2023

Ok, so how should i treat the can't answer - case for the elevation then?

@Helium314
Copy link
Copy Markdown
Owner

Helium314 commented Dec 22, 2023

Just remove the option. SCEE quests sometimes are not answerable, which is mentioned both on first start and in the default EE disabled message.
This is not a great solution, but SCEE should not add such tags. If ele:signed is already set, the quest is not asked anyway due to !~"ele:.*" (which the user can still set in tag editor).

@qugebert
Modify Ele Quest
fb3539d 
-  Remove otherAnswers (ele:signed=no)
- Additional text "don
t enter gps ele"
- Accept only numeric entries
- Exclude maps
@qugebert
Copy link
Copy Markdown
Author

qugebert commented Dec 22, 2023

Ok, i modified the ele quest.
Any idea, how to describe the difference between ref and name?
ref:signed and name:signed=no is ok, as it is still etablished?

@Helium314
Copy link
Copy Markdown
Owner

Helium314 commented Dec 22, 2023

ref:signed and name:signed=no is ok, as it is still etablished?

Both are already set by SC, so it's definitely fine

@qugebert
Copy link
Copy Markdown
Author

qugebert commented Dec 22, 2023

Ok, i modified the form, so that there is a hint at the ref and name quest.

But someone else should look over the strings, i'm not good with englisch language.

@qugebert qugebert marked this pull request as ready for review December 22, 2023 12:43
Helium314
Helium314 reviewed Dec 23, 2023
View reviewed changes
Comment thread app/src/main/java/de/westnordost/streetcomplete/quests/guidepost/AddGuidepostRef.kt Outdated
@Helium314
Copy link
Copy Markdown
Owner

Helium314 commented Dec 23, 2023

Maybe the element selection could also exclude guidepost=simple, and possibly include guidepost (there are some occurrences where guidepost is tagged without information=guidepost)

Helium314
Helium314 reviewed Dec 23, 2023
View reviewed changes
Comment thread app/src/main/res/values/strings_ee.xml Outdated
Helium314
Helium314 reviewed Dec 23, 2023
View reviewed changes
Comment thread app/src/main/res/values/strings_ee.xml Outdated
Helium314
Helium314 reviewed Dec 23, 2023
View reviewed changes
Comment thread app/src/main/res/values/strings_ee.xml Outdated
Helium314
Helium314 reviewed Dec 23, 2023
View reviewed changes
Comment thread app/src/main/res/values/strings_ee.xml Outdated
@qugebert
Copy link
Copy Markdown
Author

qugebert commented Dec 23, 2023

Maybe the element selection could also exclude guidepost=simple, and possibly include guidepost (there are some occurrences where guidepost is tagged without information=guidepost)

I changed this, is the query ok now?
Sorry, I have no possibillity to compile/test this the next days.

@Helium314
Copy link
Copy Markdown
Owner

Helium314 commented Dec 23, 2023

Looks ok now, I'll do some actual testing in the next week or so

@Helium314
Copy link
Copy Markdown
Owner

Helium314 commented Dec 24, 2023

Had a short test and looks good.

Name input should start with uppercase (should work when using InputType.TYPE_TEXT_FLAG_CAP_SENTENCES)
ele input could allow for decimals, but I'll leave the decision to you.

The icons could use some tuning:

  • I find the name a bit hard to read due to small size
  • Guidepost using the address icon looks a little weird, I prefer the style of the guidepost sports quest
  • ID is used instead of ref as text in other quest icons (hydrant ID icon looks nice and reusable)

But if you don't want to adjust the icons it's fine too.

qugebert and others added 2 commits December 24, 2023 12:58
@qugebert
change inputType
df8e76c 
ele: allow decimals
name should start with uppercase
@Helium314
Merge branch 'modified' into guidepost_metadata
1feb926
@Helium314 Helium314 merged commit 88817c4 into Helium314:modified Dec 25, 2023
@qugebert
Copy link
Copy Markdown
Author

qugebert commented Dec 29, 2023

@Helium314 Sorry, I only had the time to take care of the icons now and graphic design is not really my forte, so i tried to reuse parts of existing icons.
i'm still not really satisfied but i think it's at least better than the previous set.
Screenshot_20231229-114359
Need i make another PR for this now?

@Helium314
Copy link
Copy Markdown
Owner

Helium314 commented Dec 29, 2023

Thanks, this looks really good!
Please open another PR, as this is already merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Helium314 Helium314 Helium314 left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

New Quest: Guidepost Metadata (ref,ele,name)

3 participants

@qugebert @mcliquid @Helium314