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

Display object note above quest and overlay answers #5889

Merged
merged 12 commits into from
Oct 3, 2024

Conversation

kmpoppe
Copy link
Collaborator

@kmpoppe kmpoppe commented Sep 10, 2024

Resolves #5794.
Removed outdated screenshots ans behaviour, see #5889 (comment)
Marking this as a draft, looking for feedback.

If it exists, the `note` tag for an object is shown in the QuestHint.
@kmpoppe kmpoppe marked this pull request as draft September 10, 2024 09:07
Adding Bus Stop Name in the mix also.
@westnordost
Copy link
Member

This should be implemented generally for any quest type form and overlay form and should not use the hint thing but be displayed always. Otherwise, one would need to add the code you added to every single quest type where this might be important or turn out to be important later.

Regarding the wording, I think it should be made clear that this is a note from another user. Maybe something like

Another mapper wrote: "bla bla bla"

@kmpoppe

This comment was marked as outdated.

@mnalis
Copy link
Member

mnalis commented Sep 10, 2024

This should be implemented generally for any quest type form and overlay form and should not use the hint thing but be displayed always

I absolutely agree, it would be most useful that note should be displayed always when present (and as some quests use hint field, it should not be overwritten).

@westnordost
Copy link
Member

Does the AbstractOsmQuestForm know the element it's displaying?

Yes

@lgommans
Copy link

I like this implementation, thank you for making the proposal Kai!

The font looks small, which should make the maximum note length at 255 characters not be a problem. My phone screen is a bit less tall than yours (still too tall, I can't reach notifications one-handed, but alas) so I checked the screenshots on there and the text is easily readable at arm's length.

Adding quotes and perhaps italics as mentioned above, to make it clear that it's user-supplied text, sound good to me. I was missing something there as well, but thought that having a vertical bar on the left side for a blockquote, like GitHub's quote areas, would take more vertical space than necessary.

Added labels to fragment_quest_answer and made it display a note whenever a quest will be answered.
@kmpoppe
Copy link
Collaborator Author

kmpoppe commented Sep 10, 2024

How do we like this?

The note is now part of the fragment_quest_answer.xml and AbstractQuestForm which should make it visible whenever a quest should be answered. This does not include Overlays or fragments like creating a Note (Note API) or moving a node.

I would assume that people have seen the note before chosing any of these answers?

@westnordost
Copy link
Member

I like it! Or maybe "Another mapper noted:" is even better? Maybe put the user's text in quotation marks or like the GitHub quote areas as suggested above?

Also, seeing your screenshot, I guess it often happens that the note is completely unrelated to the question. How can we avoid confusing the user here?

UI-wise, I do wonder whether it would be better to display it in an extra bubble just like in @mnalis mockup in the issue ticket. Semantically, it would make sense - it is not part of the question but a note by a third party.

@kmpoppe
Copy link
Collaborator Author

kmpoppe commented Sep 11, 2024

Even though the screenshots not showing, I changed the wording to "Another person noted:", because I'm not too sure that the average SC user knows the term "mapper" and even considers themself as one.

This is a "normal" object:

This is an object with a relatively long note (183 characters):

And I have no idea what THIS is tbh, but these are 255 characters including line breaks:

All these look pretty good I think. The scrolling works to still show the quest pin.
Where else would we need the notes? Do they make sense in some or all quest overlays? I could think of the "Things" and "Places" maybe?

@westnordost
Copy link
Member

Yeah, I think notes should also be shown in the overlays (in all overlays).

app/src/main/res/layout/fragment_quest_answer.xml Outdated Show resolved Hide resolved
app/src/main/res/values/textStyles.xml Outdated Show resolved Hide resolved
@mnalis
Copy link
Member

mnalis commented Sep 11, 2024

What would be very useful (and I don't know if that is the case currently), is if the text from that note=* tag could be copied (to be pasted elsewhere).

Also, if there are http/https links, could they be made clickable?

For example, when showing/solving OSM Note quest, both are possible, and both have been proven very useful in practice! So perhaps code could be borrowed from there?

small_Screenshot_20240911_145034_StreetComplete

Removed Nested Scroll View (streetcomplete#5889 (comment)) and TextStyles (streetcomplete#5889 (comment)), made `note` text selectabled (copyable) as web links clickable (as per streetcomplete#5889 (comment)) and
@kmpoppe
Copy link
Collaborator Author

kmpoppe commented Sep 12, 2024

What would be very useful (and I don't know if that is the case currently), is if the text from that note=* tag could be copied (to be pasted elsewhere).

Also, if there are http/https links, could they be made clickable?

Both done in fa183c2

grafik

@westnordost
Copy link
Member

Awesome!

@kmpoppe kmpoppe marked this pull request as ready for review September 12, 2024 11:57
@kmpoppe
Copy link
Collaborator Author

kmpoppe commented Sep 12, 2024

Requested changes to Quest Module done, now Overlays also display notes:

Things Places

For simplicity's sake, only Things and Places shown here, all other Overlays work the same - the note bubble is ABOVE the Fragment that displays the task/information. I like it thus far.

The placement is changed dynamically, whether a hint (house number/name of the Thing) is displayed.

@kmpoppe
Copy link
Collaborator Author

kmpoppe commented Sep 12, 2024

@westnordost when transferring this to master, feel free to use squash and merge, I've got the checkin history in my fork but that doesn't need to be in master imho.

@kmpoppe
Copy link
Collaborator Author

kmpoppe commented Sep 12, 2024

@lgommans As you brought this up, would you like to make a comment on the looks of it?

@westnordost
Copy link
Member

To me it seems that it would be more consistent to have the house number / location label above the note. You wrote you liked it (that?) thus far. Why do you think it is better that way?

@kmpoppe
Copy link
Collaborator Author

kmpoppe commented Sep 12, 2024

grafik
Swapped in b32740f, you're totally right, looks more consistent that way around.
I'd really like some beta testing of that, do you think we could put this in a 59.0-beta2?

@kmpoppe kmpoppe changed the title Display object note in hint Display object note above quest and overlay answers Sep 12, 2024
@westnordost
Copy link
Member

I'd really like some beta testing of that, do you think we could put this in a 59.0-beta2?

I'd put this in the next major version (which will come out soon-ish anyway, v59 only took that much time because of MapLibre issues).

@kmpoppe
Copy link
Collaborator Author

kmpoppe commented Sep 12, 2024

I'd put this in the next major version

Sounds good to me :)

@lgommans
Copy link

@lgommans As you brought this up, would you like to make a comment on the looks of it?

I saw the email notifications come in today but only just now had a chance to catch up with the thread. Thanks for inviting my feedback :)

It looks great! Also the extra copying and link-parsing features, I did not even expect it to be so featureful! 🚀

The only thing I was just wondering about while eyeing the screenshots is the phrasing of "Another person noted". With notes being another OSM feature and also used in StreetComplete, that phrasing might be confusing for those who aren't used to looking at tags (also considering iD's habit of hiding those). One might wonder "who wrote this, why, how can I comment on it? Can I modify or remove it if it's outdated?"

Some options:

  • The heading could be a clickable link, but that is probably UI overload for the user
  • Tapping the bubble could show a pop up with an explanation, such as "This text is in the note tag on [this object](https://osm.org/node/321). This tag can be edited with an OSM editor such as [Vespucci](...) or [iD](...).", but that's another UI to design and I really appreciate all the work everyone involved has already put into this and I don't want to make more when I'm not even sure it's the best solution
  • A wording like "This object has a note attached:" could be used instead
  • Leave it as-is; it's not a significant problem anyway

What do you think?

@westnordost
Copy link
Member

westnordost commented Sep 12, 2024

None of the suggestions I think would make it less confusing, but more. "Noted" is used here in the sense of "remarked".

(But overall I personally find it better to use "noted" because it hints at - for advanced users who know the tag - that this might indeed be the note field.)

@lgommans
Copy link

Okay, then let's just leave it as it is :)

Thanks again for everyone's work on it!

@westnordost westnordost merged commit a75b637 into streetcomplete:master Oct 3, 2024
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.

Use note tag "to inform mappers about non-obvious information about an element"
4 participants