grid
grid copied to clipboard
Add People to Info panel and Upload page
This follows the amazing https://github.com/guardian/grid/pull/3048.
- It adds People field on the Upload page in the similar, conditional, way as Copyright: it’s there only if the uploaded image already has
peopleInImage
field present (whether we want it conditional should be confirmed with PIcture Desk before this gets merged) - It adds People field to the Info panel
This should make editing People metadata more accessible, convenient, consistent and should raise the profile of this field.
This is the draft as I have hit the limit of my abilities: all having to do with the array nature of the data.
Current unresolved issues:
-
[ ] 1. Editing People on the Upload page doesn’t work (strangely, it works in the Info panel):
400: List((/peopleInImage,List(JsonValidationError(List(error.expected.jsarray),WrappedArray()))))
-
[x] 2. When only one person in one selected image is present in the field, Info panel shows it in square brackets:
-
[ ] 3. Even for one image, if there are multiple people in the field, Info panel shows what’s usually shown for multiple images unreconcilable data:
-
[ ] 4. The same as above is shown for multiple images having exactly the same one person (or the same collection of people) in their People fields
-
[ ] 5. When one (or more) image(s) have no People data, Unknown (click ✎ to… edit prompt isn’t shown
-
[x] 6. [preexisting issue] Hide the whole field when it’s empty and user cannot edit (see here) (in Viewer, fixed by https://github.com/guardian/grid/pull/3303)
Ideally, we would add reconciliation UI for multiple “tokens” here exactly as we have for labels, just keywords-like grey, (common ones could be removed from all selected images, those present only on some images, could be added to all etc.). We would use the same UI for (editing) keywords and other fields holding multiple values too (eg., per spec, XMP dc:creator
can contain an array, even though no suppliers are using it, really).


But even without this extra UI work, I hope pts. 2–6 can be fixed, so that they at least work correctly.
Tested?
- [x] locally and exhibits problems like described above
- [ ] on TEST
Was looking at 1., and it's possible to save in the correct form- but the array is prone to reordering after save. And fixing it is nontrivial.
it's possible to save in the correct form
Cool! Not sure why I said it didn’t work, will check, was some time ago (but I defo haven’t invented this error, I’m not that clever)
the array is prone to reordering after save. And fixing it is nontrivial.
All other points much more important than fixing reordering. We can defo live with the reordering. Ideally alphabetical, but random’s cool too if hard.
https://github.com/guardian/grid/compare/aw-mk-people-power I fixed the saving here, and rolled back my ordering fix (which didn't work)
I fixed the saving here
Yep! Confirmed it works, yay! (I think it may be the same for similarly conditional Copyright field: when you batch apply on the upload page, it, correctly, batch applies it also to the pics that had no people field before, so the effect of the change is not visible for them, because the UI won’t show this newly acquired field on those pics. Easiest would be not to make it conditional: a bit more clutter, but a signal that we care about people. That we are the people’s people ;-).
Both peopleInImage
and labels
are the same type (an array of strings). They both need to be editable on upload, preview and search pages. They both want to be batchable. They both want to be set aware (as in for multiple images, does it exist in the intersection or not).
I think there's an opportunity to re-work the metadata forms here:
- Create a generic "array of strings" form field component
- Adopt this new component for
peopleInImage
- Migrate labels to this new component
- (bigger task) make forms generic and configurable to avoid the copy pasting of massive chunks of html
@paperboyo and I have spiked 1 and hope to have a PR soon.
Create a generic "array of strings" form field component
Alas, this generic component will only benefit fields of type Iterable[String]
within ImageMetadata.
Labels sit under a different part of the image, namely within userMetadata
which is of type Edits
- we appear to convert Edits
into argo objects for the client and so the generic solution isn't easily applied to this field. Will see if the labeller component could be repurposed somehow.
After this pure goodness goes in, Show additional metadata
will be another place where arrays need to be handled. And there, we cannot assume the nature of the field upfront!
