grid
grid copied to clipboard
guardian
IMAGEDAM-1666: Photosales icon is visible when syndication usage is present
What does this change?
Note: this PR should be reviewed after #4267 as it builds on the changes included in that PR
The relevant files for this PR are:
- kahuna/public/js/components/gr-icon/gr-icon.css
- kahuna/public/js/components/gr-icon/gr-icon.js
- kahuna/public/js/components/gr-icon/icons/sent-to-photosales.svg
- kahuna/public/js/preview/image.html
- kahuna/public/js/preview/image.js
- kahuna/public/js/services/image/usages.js
As an archivist user of BBC Images, after I have sent an image to BBC Photo Sales I want to see a visual indication that this has taken place. Therefore, after an image has been selected and sent to Photo Sales a 'sales' icon should appear in the thumbnail of that image.
The Photosales functionality is BBC specific, hence it is disabled by default via the showSendToPhotoSales feature flag.
How should a reviewer test this change?
As a user with elevated permissions and with the showSendToPhotoSales flag set to true:
A reviewer can test this change by selecting an image and indicating that they want to send it to BBC Photo Sales. After this action has completed, a 'SALES' icon should appear in the thumnbail of that image, as demonstrated in the following screenshot:
How can success be measured?
This testing can be considered successful if after following the above steps, the 'SALES' icon appears on the selected image, as demonstrated in the previous screenshot. Additionally, images without a syndication usage should not have this icon present.
Who should look at this?
Tested? Documented?
- [x] locally by committer
- [ ] locally by Guardian reviewer
- [ ] on the Guardian's TEST environment
- [ ] relevant documentation added or amended (if needed)
Just a note that we already do have icons associated with different syndication statuses:
Now, I have no idea how much of BBC syndication code and functionality overlaps Guardian’s. Or would it be helpful if it did. Just wanted to mention in case useful.
I have tested this locally and it all works as expected and the code looks good.
Just want to second @paperboyo's very valid comment that we already have an icon slot available for syndication status and it would be great to be able to configure that slot rather than having an increasing amount of Guardian-specific and BBC-specific code.
Perhaps a conversation for another time, but something we should definitely consider when migrating this codebase to React.
IMO, making icons configurable would be a much more sustainable approach.
Thanks for the review Rebecca! Apologies for just getting round to responding to this now, I was on a training course earlier this week and this PR slipped my mind until now!
I agree that making icons configurable would be a good change and certainly something to look into for the future.
Would someone from the Guardian be able to merge this PR now it's approved? :)
Seen on auth, collections, media-api (created by @Conalb97 and merged by @paperboyo 8 minutes and 39 seconds ago) Please check your changes!
Seen on thrall, usage, kahuna (created by @Conalb97 and merged by @paperboyo 8 minutes and 49 seconds ago) Please check your changes!
Seen on metadata-editor, cropper (created by @Conalb97 and merged by @paperboyo 8 minutes and 49 seconds ago) Please check your changes!
Seen on thrall, usage, kahuna (created by @Conalb97 and merged by @paperboyo 8 minutes and 53 seconds ago) Please check your changes!
Seen on image-loader, leases (created by @Conalb97 and merged by @paperboyo 9 minutes and 2 seconds ago) Please check your changes!
Seen on image-loader, leases (created by @Conalb97 and merged by @paperboyo 9 minutes and 2 seconds ago) Please check your changes!