Unify Image Viewer's and Image Browser's info panels

[adrielulanovsky]

What does this change?

Following the discussion on #3448, this attempts to unify the code and UI for the metadata section of the info panels on image viewer and image browser.

Some details about PR:

• For handling arrays of strings in multiple selection (image browser) we need a generic component that abstracts the logic used, for example, in the Labels field (showing elements in the array that are in all selections differently than the ones that are only in some, allowing to remove from all and add to all, etc.). This will be done in a future PR, so for now I decided, when multiple selecting, not to show array fields that are shown in single selection or image viewer (e.g. People field, collections field). I'm leaving the ones that currently exist (e.g. Labels) like they are now, so we don't lose existing functionality.

• It was decided not to show the Crops field at all in the image browser.

How can success be measured?

• Existing functionality in the image viewer and image browser should still work, while using only one template and controller for both panels.

• When selecting a single image in Image Browser, all fields that are shown in the image viewer (except Show Crops) should also be shown in the info panel.

Screenshots

Image browser (single image selected):

Image browser (multiple images selected):

Image viewer:

Who should look at this?

Tested? Documented?

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

[paperboyo]

This is great! Most bugs with those panels are not specific to this work: multiple redundant scrollbars, whole image moving around, editing hints (Multiple bylines…) sometimes in italic, sometimes not or these gremlins.

It’s great that you’ve described future steps around tokens! I think most can be modelled like Labels, with just some colour/CSS tweaks. Maybe People will present some difficulty if we would like to also allow editing of existing names…

This is what I could find specific to this PR:

1. We do not show leases for users without edit_metadata in Viewer (this is questionable, because leases are not shown, nor their effect, but nevertheless this is how it is). In Browser, we show some useless (current + inactive) text which lies about the counts (unlike Usage rights count). If anything, it’s better to copy current Viewer, than current Browser:
   Current PROD Viewer:
   
   This PR (and current Browser):
   
2. Special instructions is editable, so should behave like other fields (should show Multiple intructions (click ✎ to edit all)) and not disappear:
   

Will add more if I find anything. This looks very good. Just a note that there is this abandoned PR with slightly related work…

[adrielulanovsky]

Hi @paperboyo. I fixed issue number 2.

Regarding issue number 1, do you want me to hide the Leases panel when there are multiple images selected? (and perhaps tackle fixing the count issue in a different PR? this problem wasn't introduced in this PR, right?). I noticed that the image viewer has the following condition for showing leases: permission to edit metadata OR number of leases > 0. Do we want to maintain that condition?

[paperboyo]

I fixed issue number 2

Yay! Thanks!

do you want me to hide the Leases panel when there are multiple images selected?

Yeah, I think so (but obviously only for unpermissioned users!). current leases + inactive leases text is useless without the actual counts and ability to add a lease! Yes, the problem precedes this PR and yes, it would be great to have actual dynamic counts that do not break the CSS, haha – then we could show them.

image viewer has the following condition for showing leases: permission to edit metadata OR number of leases > 0. Do we want to maintain that condition?

I think it was @andrew-nowak who spotted that we have a bug with the second part of the condition, so that none of the leases ever show for non-permissioned users. We have users complained that if unpermissioned users are asking permissioned users to grant a lease they have no way of knowing this has actually happened apart from the fact that the Cannot crop changes to Crop.

This can be sorted in two ways:

1. either don’t display the leases, but create a special state for unpermissioned users clearly indicating a leased image is fine to use
2. fix the condition and display active leases to unpermissioned users in both Viewer and Browser (but obviously read-only). bonus for hiding syndication leases as these are useless for them

I think option 2 is much easier to implement (just fixing a bug), but I’m famous for thinking something I won’t actually be fixing myself is easy to fix :-).

[paperboyo]

[ignore, not directly relevant to this PR work]

they have no way of knowing this has actually happened

Just for the sake of completeness, and for my own record, because I’m testing this as I go, this isn’t strictly true. The other way to know something has been leased for an unpermissioned user is the lack of these additional banners

• No rights imagery without a lease:
  
• Chargeable category without a lease:
  
• Restricted imagery without a lease (no mention of needing a lease, sadly):
  

So if we ever think of solution 1, those banner better look different.

[paperboyo]

Testing, (Date) Taken isn’t shown:
PROD:

TEST:

And if super-easy, I think I like the more subdued missing values from PROD Viewer better (Copyright: unknown) than the same intensity ones from TEST, what do you think?

[adrielulanovsky]

Testing, (Date) Taken isn’t shown: PROD: TEST:

And if super-easy, I think I like the more subdued missing values from PROD Viewer better (Copyright: unknown) than the same intensity ones from TEST, what do you think?

@paperboyo Fixed

[paperboyo]

Almost there!

1. Description text got greyed out:

   

2. Could we maintain filename wrapping behaviour from current PROD Viewer? Applies to at least Description and Filename. Filename shown here:

   PROD (long filename without spaces):

   

   PROD (long filename with spaces):

   

   TEST (long filename without spaces):

   

   TEST (long filename without spaces):

   

3. [only if needed after sorting pt. 2] Could we keep the behaviour of this little guy which allows selecting the whole of truncated filename with one click? (otherwise it’s impossible to copy the whole of truncated filename) It should only apply to filename field (otherwise it would be impossible to select part of eg. Description etc):
   

4. Could we keep the tooltip showing the whole of any truncated field?
   

5. In Browser, any non-array, non-editable, always-present fields (eg. Uploaded, Uploader, Filename) should be shown as long as the values are the same for all selected images. As soon as the values differ, the whole field should disappear. It is of no use to say eg. Filename: Multiple filenames (since it’s impossible to edit it either). Currently, Filename stays, while eg. Uploader disappears even if it’s the same for all selected photos.

[adrielulanovsky]

Almost there!

1. Description text got greyed out:
   ![image](https://user-images.githubusercontent.com/6032869/137587284-2cacd7fb-bdfc-44f3-94c9-d9c9b48e6bff.png)

2. Could we maintain filename wrapping behaviour from current PROD Viewer? Applies to **at least** Description and Filename. Filename shown here:
   PROD (long filename without spaces):
   ![image](https://user-images.githubusercontent.com/6032869/137587710-0306da73-1a79-477b-83da-70b8ef904155.png)
   PROD (long filename with spaces):
   ![image](https://user-images.githubusercontent.com/6032869/137587758-703ef9ba-7ae0-445d-9a85-5f43da951fee.png)
   TEST (long filename without spaces):
   ![image](https://user-images.githubusercontent.com/6032869/137587776-0dc99114-3578-45c2-af73-afe7a43a0a04.png)
   TEST (long filename without spaces):
   ![image](https://user-images.githubusercontent.com/6032869/137587794-d2f31a23-e257-4fb1-800d-09214341c290.png)

3. [only if needed after sorting pt. 2] Could we keep the behaviour of [this little guy](https://github.com/guardian/grid/pull/2637) which allows selecting **the whole** of truncated filename with one click? (otherwise it’s impossible to copy the whole of truncated filename) It should only apply to filename field (otherwise it would be impossible to select part of eg. Description etc):
   ![sdfsdf](https://user-images.githubusercontent.com/6032869/137588162-39200fd2-37e1-4a86-af11-631ae95f4931.gif)

4. Could we keep the tooltip showing the whole of any truncated field?
   ![image](https://user-images.githubusercontent.com/6032869/137588193-9f2a1cb4-2709-4869-8e16-4f39361c16c2.png)

5. In Browser, any non-array, non-editable, always-present fields (eg. Uploaded, Uploader, Filename) should be shown as long as the values are the same for all selected images. As soon as the values differ, the whole field should disappear. It is of no use to say eg. `Filename: Multiple filenames` (since it’s impossible to edit it either). Currently, Filename stays, while eg. Uploader disappears even if it’s the same for all selected photos.

Issues 1 to 5 should be fixed now. Thanks for the feedback

[prout-bot]

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

[prout-bot]

Seen on auth (created by @adrielulanovsky and merged by @paperboyo 10 minutes and 58 seconds ago) Please check your changes!

[prout-bot]

Seen on collections (created by @adrielulanovsky and merged by @paperboyo 11 minutes and 8 seconds ago) Please check your changes!