-
Notifications
You must be signed in to change notification settings - Fork 1
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
slick 5:4 and 4:5 crop sharing from composer to fronts tool #312
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
twrichards
force-pushed
the
fronts-integration
branch
5 times, most recently
from
October 8, 2024 13:48
967c3cd
to
404e6ea
Compare
twrichards
force-pushed
the
fronts-integration
branch
6 times, most recently
from
October 14, 2024 10:17
1cd536b
to
afbff5a
Compare
This was referenced Oct 15, 2024
twrichards
changed the title
fronts integration
slick 5:4 and 4:5 crop sharing from composer to fronts tool
Oct 15, 2024
twrichards
force-pushed
the
fronts-integration
branch
4 times, most recently
from
October 15, 2024 10:22
7f1349b
to
61ebc82
Compare
twrichards
force-pushed
the
fronts-integration
branch
2 times, most recently
from
October 15, 2024 15:00
b4cd10d
to
66db3fa
Compare
twrichards
changed the base branch from
main
to
expose-bootstrapping-lambda-name-as-CfnOutput
October 15, 2024 15:57
twrichards
force-pushed
the
fronts-integration
branch
from
October 15, 2024 16:21
66db3fa
to
3e2fc4c
Compare
Base automatically changed from
expose-bootstrapping-lambda-name-as-CfnOutput
to
main
October 17, 2024 08:32
twrichards
force-pushed
the
fronts-integration
branch
3 times, most recently
from
October 17, 2024 12:50
be47b48
to
f0d56fb
Compare
…ts integration to avoid polluting users' `My Pinboards` lists
…eded and take up too much space
…flag `alternateCropSuggesting`
…he expanded fronts card)
…`, `ALTERNATE_CROP_SUGGESTING_FEATURE_TURNED_OFF`, `ALTERNATE_CROP_SUGGESTED` and `ALTERNATE_CROP_DRAGGED`
… `FrontsPinboardArticleButton` can request matching crops without having to get ALL the items for that pinboard and filter out the majority that it doesn't need
twrichards
force-pushed
the
fronts-integration
branch
from
November 23, 2024 11:08
bb03b86
to
f74e79f
Compare
twrichards
added a commit
to guardian/facia-tool
that referenced
this pull request
Nov 25, 2024
…s stage specific default) following release of guardian/pinboard#312
twrichards
added a commit
to guardian/facia-tool
that referenced
this pull request
Nov 25, 2024
This was referenced Nov 25, 2024
twrichards
added a commit
to guardian/facia-tool
that referenced
this pull request
Nov 25, 2024
andrew-nowak
approved these changes
Nov 25, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we've had a walkthrough so I've seen it working, logic seems fine but I've not done a super-deep-dive into it. Just a couple of comments which could be addressed in a followup
great work!
Co-authored-by: Andrew Nowak <[email protected]>
Co-authored-by: Andrew Nowak <[email protected]>
Seen on PROD (merged by @twrichards 3 minutes and 13 seconds ago) Please check your changes! |
twrichards
added a commit
to guardian/facia-tool
that referenced
this pull request
Nov 25, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pre-requisite PRs (9)
urlPath
as data attribute) facia-tool#1695asset-handle
elements (used by Pinboard) grid#4350show-cropping-gutters-switch
(still GNM only) grid#4348alternateCropSuggesting
Feature SwitchThis PR makes use of the feature switch mechanism added in #321, via a new feature switch
alternateCropSuggesting
(set via a query param on any page of the host tool). All the below changes (Composer and all Fronts Integration) will only display with the feature switch on. Doing it this way allows users to be trained over a few weeks and will see the new functionality from whenever they once visit a fronts/composer URL with?pinboardFeatureFlag_alternateCropSuggesting=true
onwards (as the feature flag is persisted to their user in the DB).Composer changes
Thanks to https://github.com/guardian/flexible-content/pull/4984 and https://github.com/guardian/flexible-content/pull/4999 we have a new
<pinboard-suggest-alternate-crops>
element which sits beneath the 'Alt text' of the trail/thumbnail in the Furniture panel. Pinboard detects this element (as with other tools integrations) and populates it using React portal (and shadow DOM) to display a heading and two new crop ratios, with a button and count for each...postMessage
to share the crop to the pinboard, sending and expanding automatically so the user can see what has happenedasset-handle
elements (used by Pinboard) grid#4350) within the 'pre-selected pinboard' for the composer piece in question (this required some refactors to push some stuff up to global application state).Hovering over the count displays a smallish tooltip showing small thumbnails for the crops of that ratio...
...clicking on those thumbnails will expand the pinboard, scroll to the item and flash to highlight, so the user can see any supporting contextual messages if any have been sent).
Pinboard also adds translucent gutters down the L&R sides of the trail to help remind users what will be clipped off with the automatic 5:3 -> 5:4 crops (centring the image)...
...these complement the gutters users will have seen in the grid when they were cropping for main/trail (thanks to https://github.com/guardian/flexible-content/pull/5049 composer will only send the
shouldShowCropGuttersIfApplicable
query param to Grid IF pinboard has taken over the<pinboard-suggest-alternate-crops>
- i.e. grid's cropping gutters only show for users with the feature switch on - indirectly though, since pinboard doesn't run in iframed grid, only standalone grid - so grid cannot check the feature switch itself).Fronts integration
This PR adds Fronts integration (previously there wasn't any pinboard in fronts tool).
Since Pinboard uses workflow items for pinboards (pinboardId is just a workflow stub id) the only bit of connecting information between a card in fronts and the pinboard is via the
urlPath
field, we needed to allow searching for many url paths in one go, returning a list of workflow ids - cue https://github.com/guardian/workflow/pull/1119.Thanks to...
urlPath
as data attribute) facia-tool#1695urlPath
toFeedItem
, rather than usingliveUrl
facia-tool#1705...we now have a
<pinboard-article-button>
on the various cards in fronts tool (typically in the hover menu), each with adata-url-path
attribute (see previous para). Pinboard detects these elements (as with other tools integrations) and populates each using React portals (and shadow DOM) to display for a given pinboard a...... allowing users to see at a glance which cards have pinboard discussions and suitable assets to use as overrides on the cards. All the following are when hovering...
'Latest' feed
Clipboard
Container (collapsed)
... clicking the yellow buttons opens Pinboard to the relevant pinboard.
Then for the expanded cards, in a container, we get thumbnails for the crops of the appropriate ratio, in the GIF below this is a 5:4 container...
...notice how clicking on the suggested crop takes you to the relevant image in the chat (to see if there's any useful context), the user can then drag the thumbnail to replace the trail image (alas fronts tool is quite slow doing this).