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 hint label to note text #5440

Merged
merged 3 commits into from
Jan 13, 2024
Merged

Conversation

Helium314
Copy link
Collaborator

fixes #5398

This changes the note text as discussed in #5398.
I chose to use < and > for the link, as I suspect that links are parsed by different consumers (osm.org, notesReview, and probably more that I don't think of) and ideally none of them should produce broken links.

Though I'm not sure how nice this is in all cases, e.g. with level:
Unable to answer "Are these opening hours still correct?" for on level 2: Mobile Service (Mobile Phone Shop) https://...

@westnordost
Copy link
Member

Well, to be absolutely sure, I guess one could also use spaces. E.g. ( https://www.dot.at ). Looks fine, too. But, your decision. LGTM

@FloEdelmann
Copy link
Member

Though I'm not sure how nice this is in all cases, e.g. with level:
Unable to answer "Are these opening hours still correct?" for on level 2: Mobile Service (Mobile Phone Shop) ...

Another option would be Unable to answer "$question $hint" for $element, e.g. Unable to answer "Are these opening hours still correct? on level 2: Mobile Service (Mobile Phone Shop)" for osm.org/…

@Helium314
Copy link
Collaborator Author

Thanks, I think that looks better and it avoids the parentheses.

@Helium314
Copy link
Collaborator Author

Oh, that brings me to the next issue: It doesn't work for In context of "$overlayTitle" overlay for $element, e.g. In context of "Street Parking Service Road" overlay for http...
Maybe

  • In context of overlay "$overlayTitle $hintTitle" for $element
    In context of overlay "Street Parking Service Road" for http...
  • In context of overlay "$overlayTitle ($hintTitle)" for $element
    In context of overlay "Street Parking (Service Road)" for http...
  • In context of "$overlayTitle" overlay ($hintTitle) for $element
    In context of "Street Parking" overlay (Service Road) for http...

I'd go for the second one if there is no objection or better idea.

@FloEdelmann
Copy link
Member

I guess there is no easy way to append the hint text after the element URL? That would be consistent and clean:

  • Unable to answer "$question" for $element ($hintTitle)
    Unable to answer "Are these opening hours still correct?" for http… (on level 2: Mobile Service (Mobile Phone Shop))
  • In context of "$overlayTitle" overlay for $element ($hintTitle)
    In context of "Street Parking" overlay for http… (Service Road)

@westnordost
Copy link
Member

Maybe without the "for" but instead with a "-"?

val leaveNoteContext = if (hintLabel.isNullOrBlank()) {
"Unable to answer \"$questTitle\""
} else {
"Unable to answer \"$questTitle – $hintLabel\""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrong placement of "?

Should it really be

Unable to answer "Are these opening hours still correct? - on level 2: Mobile Service (Mobile Phone Shop)"

and not rather

Unable to answer "Are these opening hours still correct?" - on level 2: Mobile Service (Mobile Phone Shop)

?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really have a preference here, if you like like the second variant more I'll change it.

Copy link
Member

@westnordost westnordost Jan 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I like the second option more, or actually, the full string would be

Unable to answer "Are these opening hours still correct?" - on level 2: Mobile Service (Mobile Phone Shop) for https://osm.org/node/....

wouldn't it? Actually, my suggestion was to not use "for" at all, but use "-", i.e.

Unable to answer "Are these opening hours still correct?" - on level 2: Mobile Service (Mobile Phone Shop) - https://osm.org/node/....

(Because the "for" feels out of place there, "phone shop for element", eh?

@westnordost westnordost merged commit 0d2d173 into streetcomplete:master Jan 13, 2024
@westnordost westnordost deleted the note_text branch January 13, 2024 18:26
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.

Add more context to notes
3 participants