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

Update sources field #222

Merged
merged 7 commits into from
Aug 8, 2024

Conversation

pedromml
Copy link

@pedromml pedromml commented Jul 11, 2024

This PR aims to redesign the "Sources" field for the source= and source:*= keys.
The main change is the addition of extra rows on the Sources field that each map to a subkey, like source:name or source:url. In these rows, users will be able to assign values directly to the subkeys, something that was only possible before by manually adding these keys in the tag editor.
image

@pedromml pedromml linked an issue Jul 11, 2024 that may be closed by this pull request
@pedromml pedromml marked this pull request as draft July 11, 2024 20:15
@pedromml pedromml marked this pull request as ready for review July 18, 2024 19:24
data/core.yaml Outdated Show resolved Hide resolved
data/core.yaml Outdated
@@ -801,6 +801,11 @@ en:
field_source: add source
field_source_label: Source for {field_name}
field_source_placeholder: URL, newspaper article, book...
source:
main_input: General source
name: Name of the source
Copy link
Member

Choose a reason for hiding this comment

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

Since the subfields are already part of the Sources field, we don’t need to repeat “source” here. I don’t think users will confuse this with a field for the name of the feature.

On that note, we should rename the Sources field back to just “Source”, in anticipation of a different solution for tagging multiple sources. Otherwise, the user might think each line is for a different source. The placeholders would guard against that, but a placeholder doesn’t appear when the subfield has some text in it.

Copy link
Author

Choose a reason for hiding this comment

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

I agree that the field should be named "Source" now and I've been trying to find where the field name is defined, but I wasn't able to.
I found the preset definition in id-tagging-schema, but I believe this refers to the Sources field available after creating a changeset on OSM. It is possible that this preset is also used for the OHM Sources field because it is linked to the source key. Does it make sense to change it?

Copy link
Member

@1ec5 1ec5 Aug 8, 2024

Choose a reason for hiding this comment

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

We have a general mechanism for providing (or overriding) strings used for individual fields or presets:

iD/data/core.yaml

Lines 2528 to 2541 in 5cfff58

custom_presets:
fields:
end_date:
label: End Date
placeholder: YYYY-MM-DD
terms: abandoned, destruction, destroyed, demolition, demolished, closed, closure
license:
label: License
placeholder: CC0
terms: copyleft, copyright
presets:
type/chronology:
name: Chronology
terms: history, timeline, movement, evolution, change

If you search for custom_presets in the codebase, you can see where we prioritize these strings over the ones that come from id-tagging-schema.

The same preset is used in both situations. If you think it would be confusing to refer to just one “Source” in the save panel, we could add the same additional source fields you were planning to that panel, or we could split the field so that one field is universal to every selected feature, while the other only appears on the save panel. Two fields can correspond to the same raw key, as long as they have unique field IDs.

data/core.yaml Outdated Show resolved Hide resolved
data/core.yaml Outdated Show resolved Hide resolved
modules/ui/fields/sources.js Outdated Show resolved Hide resolved
modules/ui/fields/sources.js Show resolved Hide resolved
modules/ui/fields/sources.js Show resolved Hide resolved
Copy link
Member

@1ec5 1ec5 left a comment

Choose a reason for hiding this comment

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

This is ready to go. Renaming the field would be nice for consistency, but we can also do it as part of introducing the additional source fields.

@pedromml pedromml merged commit 846e2f2 into OpenHistoricalMap:staging Aug 8, 2024
3 checks passed
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 ui to allow users to change source:* keys
2 participants