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

Searchlinks in Additional metadata and more consistent Info panel UI #3871

Merged
merged 22 commits into from
Oct 18, 2022

Conversation

paperboyo
Copy link
Contributor

@paperboyo paperboyo commented Sep 28, 2022

What does this change?

Co-authored by @jonathonherbert
Co-authored by @andrew-nowak

Fixes #3849:
Links values for metadata fields visible under Show additional metadata section to searches bringing them in line with the rest of the Info panel. Please note that identifiers have no links (because they are either unique or defunct). Arrays and other structured fields do not work, but their display is already broken anyway. Both should one day be using our pills (like eg. people) for arrays and forms (like eg. Location) for structured fields. Also, there is no reason why Additional metadata UI could not be displayed and work for multiple selections…

image

For consistency , it also allows searching for multiple images with the same Title by linking from the Title field. Linking from Description makes either no or very little sense. I decided not to link from hour and/or day either. Not sure about Special Instructions, but decided on no.

Before After
image image

Finally, it brings some minor styling improvements which make the panel more consistent:

  1. Multiple values are always italicised (fixed Description and Date taken)
  2. makes Photoshoot value look like any other
  3. hides non-required fields when they are empty and user can’t edit (Location, Filename and People) (previously some were hidden, but these were not)
  4. adds Unknown placeholder for empty required Credit field and for empty (but important) Taken field
  5. It uses Title Case for three additional metadata fields (bylineTitle, suppliersReference and source) and for identifiers

Screenshots

No. Before After
1 image image
2 image image
3&4 image image
5 image image

How should a reviewer test this change?

Try to search by clicking on Title and values in Additional metadata. Look around for styling fixes shown above.

How can success be measured?

Editors can discover image groupings by easily searching on Title.
Editors can discover more ways of searching.
Less chaos in the info panel, no headings without values, more consistent styling.

Tested? Documented?

  • locally by committer
  • locally by Guardian reviewer
  • on the Guardian's TEST environment
  • relevant documentation added or amended (if needed)

@paperboyo paperboyo changed the title Searchlinks in Additional metadata and more consistent UI for Multiple values Searchlinks in Additional metadata and more consistent Infa panel UI Sep 30, 2022
@paperboyo paperboyo changed the title Searchlinks in Additional metadata and more consistent Infa panel UI Searchlinks in Additional metadata and more consistent Info panel UI Sep 30, 2022
…spaces, dots (and quotes necessary for spaced keys
this is better:
- cannot have spaces
- uses chips without fileMetadata. at the beginning
…so that clicking bylineTitle in Additional metadata actually works and does not require adding quotes around field manually
@paperboyo paperboyo marked this pull request as ready for review October 3, 2022 22:54
@paperboyo paperboyo requested a review from a team as a code owner October 3, 2022 22:54
@paperboyo paperboyo marked this pull request as draft October 4, 2022 14:04
@paperboyo paperboyo marked this pull request as ready for review October 5, 2022 09:49
@aracho1
Copy link
Contributor

aracho1 commented Oct 6, 2022

I was running this on TEST and noticed the following error in the console:
image

which is pointing to this line. I noticed that the values in the XMP Keywords field appear as an array of strings but they are not actually clickable, is this the expected behaviour?
image

If not, I wonder if the query for this field is failing potentially, as the other additional metadata fields seem to be working fine.

@paperboyo
Copy link
Contributor Author

paperboyo commented Oct 6, 2022

is this the expected behaviour?

Very good spot, @aracho1. This was hinted at in the PR description

Arrays and other structured fields do not work, but their display is already broken anyway

Arrays and other structured fields have currently no support to show properly. This PR adds “links” to them which do not work. Those fields would need to be consciously added via fieldAliases config (like XMP keywords is on our TEST). Current PROD has no configured fieldAliases at all.

It is a pity, but to add support to display them properly (and then fix the linking for them) is a job too clever for this contributor (I have tried). I think it is acceptable as is, since someone would need to configure them to be shown knowing there is no support. But if you think not, I may have a look at adding code that disables linking for arrays and structured fields. Again, much better would be to make them work, but here I would need help from someone…

@prout-bot
Copy link

Seen on auth, usage, cropper, collections, media-api, kahuna, image-loader, metadata-editor, thrall, leases (merged by @paperboyo 10 minutes and 50 seconds ago) Please check your changes!

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

Successfully merging this pull request may close these issues.

Auto-link metadata keys from additional metadata section
4 participants