Added PrintShop - #61
Conversation
|
@rcrphillips You know a bit about these things, is this a fair representation of these articles or do we need to check with the Print Shop team to make sure we aren't missing anything here? |
|
There are also pieces which are a different content type altogether, which is 'picture'. So you've got immersive articles, interactives, cartoon/graphic/picture, all of which are different content types, that are tagged with the guardian print shop series. And DCR is only concerned with articles for now. I would say that it might not be worth including as it's own type, but it if it makes things a lot simpler for you, then fair enough. My hunch is that those are probably tagged with that series tag in order to do something automatically on the print shop site itself. @oliverlloyd hopefully that's clear as mud. |
|
I’m gonna add some mud (fight!). This is users getting cheeky and exploring the boundaries of what’s possible by playing with the switches. My hunch is that to follow that playfulness with a model change (I don’t really know what a Design type is!) is a wrong thing to do. We just have to (sorry) be able to support any article without main media and with a first element (yeah, not only an image) to be set to any weighting. This particular one (Immersive) should try to mimic the furniture looks of frontend only if Design thinks it’s what they want (because nobody ever designed it, or tested it, before users found this trick). Chasing users finding tricks within the model with model changes will get us to choke on our own tail, I suppose. With freedom comes… PITA. |
It basically is that (the escaping part, not the fool's gold bit - I hope!). The larger goal here is to support an Editorial Colour Palette. We want to be able to decide the look of something based on The challenge to this is, umm, well human nature and physics I guess. If a thing is possible in Composer, then given enough time (not very long) it will happen. The model that we currently have is actually quite good. There are still some things to bring into it but it probably (hand waving) encompases 99% of active content. There's a long tail with weird stuff in it but this was always going to be a thing. The problem with these loose couplings between Frontend and Composer (facilited with cleaners and conditional logic in the templates) is it doesn't scale and leads to the kinds of problems we want to solve with DCR. The downside with taking this it-must-meet-the-model position is that some content could 'break'. We're not there yet though, we support everything so far. But we have been quietly drawing some lines to stop people wandering off into the hinterlands and doing odd things - things they never have and probably never will, but things they could have done before. Stuff like, a paid advertisement labs piece under the News pillar or an Interview Opinion article. What were conventions before are now rules. It's true though that there's probably more stuff like this out there and trying to capture it in a model could be an attempt to herd cats but I'm hopeful that |
|
Thanks, very clear! Some loose thoughts based of shaky understanding and half-strongly held views I’m only half in agreement with myself:
So not much to say here apart from : why new names (first two)? (although Composer Otherwise, I’m all for more consistency, predictability and simplification for all concerned (devs, designers, users and… readers!). If that means limiting (some) options in Composer: 👍. Because
that. If we don’t want it to happen, we need to make it impossible. Your examples reveal my lack of specific knowledge, but:
I still think there is nothing special about this particular setup here with Print Shop. It just has no main media and first element is allowed to go wild, coz it’s Immersive too (so Immersive role is available for elements). Furn look comes from Immersive. Colours will depend on under which Pillar the Section that the first added tag dictates lands (so the usual) and the tone (again, usual). The only hard part from my naive POV is to make sure the CSS around furn and around no Main media and differently weighted first pic a) looks more or less like current frontend (only coz users expect that, if design decides differently – it should change) and b) doesn’t break. And that ads don’t overlap. But all that is true for all content, no? The only way to limit the possibility of this trick would be to disallow creation of content without Main media, I guess. All the rest will still be possible and I can’t see limiting that. Actually, getting rid of Design hints and freeing element roles will make such combinations more likely. I wrote too much. Again. |
|
[maybe instead of what I wrote, I should write this]
👍 Sensible and pragmatic. I should learn (bit late for me, though, so not gonna try too hard ;-). |
Correct, we're talking to Ben about the removal of some display hints.
Maps to DesignType: https://docs.google.com/document/d/1PLc-kGnO7aYJ-mkmsLctFK0etBNyHlWvHrS33hcLhtM/edit#heading=h.jkpehtc5tksn Tones don't exist in DCR - I explicitly wanted to remove any reference to a tone, either a tone fits within the Format type or maps functionality in components directly by a boolean. Tones were a major source of bugs in Frontend.
Partly agree, we should maybe have used the existing nomleclature of DisplayHint and DesignType, but I think they're close enough to not be a problem. Frankly though, I am of the strong opinion that if Format does not meet the requirements of a content type, we strongly review the content type first.
Things like this must be controlled in Composer. But, that said, I am not sure a Design for Print shop is the fix for lack of main media as @paperboyo says "We just have to (sorry) be able to support any article without main media" - Not every article without a main media is going to be a print shop article, so we need to handle lack of main media somewhere else. So this "To accomodate these differences we had to create a noMainMedia prop in DCR but this is a poor solution" would still be needed. But the difference in design and layout does hold up to needing a |
|
[not directly related to the PR]
Whoa! Radical! Half of me feels ecstatic we have less things, the other half would ask: how does a Sport editor apply a (Sport) Comment Tone to a piece, then? (in the same way and you mangle it on the way in and just don’t use it directly?). As far as presentation derived from tags goes, tags of type tone were always the least crazy (you can have just one, there is a finite list etc). |
|
I think if we go the prop route like |
@nitro-marky Could you elaborate on this? No main media is possible across all article types already, so it doesn't fit nicely in a |
@paperboyo What's a sport comment tone? (got an example?) Sounds like DesignType = Comment, Pillar = Sport ? |
If it already exists then it's not a problem, I'm just wary of trying to make an article type fit all situations by increasing the amount of props (which may not be necessary). Kind of similar to the Liskov principle (the duck and all that), just something to be aware of, if more props need to be added then abstraction/separation of concerns might need to be considered. |
Yep. Tones can be used on any content, from any section. I will repeat an example from above: https://www.theguardian.com/sport+tone/comment (just add this tag combiner to any section URL; and let’s just make DCR fronts accept any arbitrary CAPI query ;-) They were invented specifically to signal the editorial “type” of content: opinion piece, an interview, an analysis etc. (as opposed to tech “type” like video, article, gallery etc). Their functionality and their bearing on presentation have to be retained by DCR and AR, otherwise it would mean a major upset and I know nothing of any plans like that. And they do make sense. Ofc, that doesn’t mean you have to use them directly, you can parse them into your helpers as you please, goes without saying. |
I will note that DCR is serving 90% of the audience and supports 94% of article content now, so if something isn't right I'd hope we would have seen it by now! (Looking at that type, sport comment on Frontend (dcr=false) and DCR are largely identical) |
|
Just adding to this discussion that it is essential that the type of article is correctly model in our Why is it important?
As mentioned per @paperboyo, we already have the concept of
If we would like to deprecate Hope that makes sense. |
I think what builds a Format is represented in the CAPI Client |
|
I wanted to circle back to why I'm trying to add The wider goal that this PR supports is Editorial Palette. We want to be able make decisions about the design of an article, based on Format where Format is: interface Format {
theme: Pillar | Special;
design: Design;
display: Display;
}By using this definition we can write functions like this (which is proposed here): const getHeadlineColour = (format: Format): string => {
switch (format.display) {
case Display.Immersive:
return 'white';
case Display.Showcase:
case Display.Standard: {
switch (format.designType) {
case 'Review':
case 'Recipe':
case 'Feature':
return pillarPalette[format.pillar].dark;
case 'Interview':
return 'white';
default:
return 'black';
}
}
default:
return 'black';
}
};But that function fails on print shop content at the moment and it fails because the headline colour changes based on if there is main media present on the page or not and Next StepsI'm proposing we:
Then, assuming that Editorial are aware of this limitation and are in agreement that this aligns with how they expect these articles to work, we then ask tools to make Composer disallow main media when the print shop tag is present If the agreement is in place and the convention is already known, then the actual Composer limit is not essential. The agreement is key. We take what was a trick that someone found in Composer one day and transform it into a strictly typed design. Finally, if no agreement is found, and it come to light that Editorial have other requirements or expectations, then we refactor Types & DCR to manage this. |
|
Wow this is quite a discussion.
Unfortunately it's not that simple, and this gets to the heart of why we came up with a completely new type with new names.
Again, I'm afraid it's not that simple. Tone tags are a part of the information used to derive
and so on. Again, this is why we have new types with new names.
As @gtrufitt said this is originally where the
In the const format: Format = {
design: Design.Comment,
theme: Pillar.Sport,
display: Display.Standard,
};
Be careful referring to this field as
I agree @mchv. @rcrphillips has already facilitated a conversation between the Dotcom, Apps, CAPI and Tools teams about the possibility of |
|
@blishen @paperboyo @mchv @JamieB-gu @gtrufitt Some of us discussed this yesterday but we didn't come to a firm conclusion and I wanted to bring this up again as this decision feeds into a lot of the core work we are doing in DCR. My feeling here is that, by convention, Print Shop articles are a thing. We call them 'Print Shop' articles. But in Composer they are just a trick, they are what happens when you set display hint to I think that it is okay, valid even, for us to change the model that we use for DCR and AR to encapsulate this 'type' of article, even if it isn't represented by CAPI and even if we do nothing in Composer. We are already doing this in many other situations. So where an article has tag with id If we don't set this const getHeadlineColour = (format: Format): string => {
switch (format.display) {
case Display.Immersive:
return 'white';
case Display.Showcase:
case Display.Standard: {
switch (format.designType) {
case 'Review':
case 'Recipe':
case 'Feature':
return pillarPalette[format.pillar].dark;
case 'Interview':
return 'white';
default:
return 'black';
}
}
default:
return 'black';
}
};we will need to: const getHeadlineColour = (format: Format, noMainMedia: boolean): string => {
switch (format.display) {
case Display.Immersive:
if(noMainMedia) return 'black';
else return 'white';
case Display.Showcase:
case Display.Standard: {
switch (format.designType) {
case 'Review':
case 'Recipe':
case 'Feature':
return pillarPalette[format.pillar].dark;
case 'Interview':
return 'white';
default:
return 'black';
}
}
default:
return 'black';
}
};This is possible, perhaps inevitable, and seems innocent enough but it breaks the goals we - DCR, AR, Ben and Harry, etc - have been working towards for nearly a year now and my concern is once you cross this line in the sand the value of EditorialPalette will gradually reduce as more props are added to handle edge cases outside the model (or worse, just because future developers will read te code and learn that adding props is okay and assume this is the right way to do things). |
The only problem I can see is that we don’t know if there are other articles outside of this series with no Main media and a first element in body set to Immersive role… I asked CAPI, but it’s not clear if we are able to search for them. There is nothing stopping users to have no Main media on any article, subsequently there is nothing stopping them to choosing any available role for the first element in the body (incl. Immersive and Dessign Hint is Immersive too). |
|
We need to unblock this, so based on conversation and current behaviour in platforms, we're going to go ahead with this approach:
We will get final confirmation from central production that there are no other cases that we need to support outside of those identified. We can iterate on this, of course, and if we find it's the wrong decision then we will need to refactor but at this point this is purest approach that doesn't tie ourselves in knots for the future. |
This was my concern too - it's not currently
This sounds like a sensible solution to me - we only support this for print shop. I believe one consequence of this is that if an article is tagged "print shop" but does have a main media set, the main media will be ignored? I think this is the right behaviour, but just bringing it up in case it puzzles someone in Editorial @paperboyo. Excellent summary btw @gtrufitt, very well put! |
Actually, no, not right now. It will break the current design because the headline text will be black, not white, but there's no logic to ignore the main media if it exists. If you break the convention like this then you break the article. It's worth pointing out though that going forward where DCR is used for Preview (most places now) then things like this will be immediately evident. The risk is, as @paperboyo says, if there is legacy Immersive content with no main media which is what we want to learn from cp. |
|
@oliverlloyd will this be |
Or, them breaking the convention for this series as arbitrarily as they have adopted it? Or existing content in this series that is not using this trick (+photoEssay, anyone?)?. Or other ppl figuring out this trick? Or DCAR starting to render other content |
Yep 👍 Although, adding new types isn't breaking for DCR as far as I recall because we use switch statements and default out but we can't depend on that being the case everywhere |
For all new content, I'm less concerned because this trick will no longer work. From now on (once we merge and add these changes), if you remove main media from an immersive article it will look wrong in Preview (except for a few article types over the next few weeks while we finish the DCR migration which still show in the Frontend version in Preview) |
7 participants
What does this change?
Adds the
PrintShopDesign typeWhy?
There are a series of articles classified as Guardian Print Shop articles. Unlike other series, these articles have their own design which is not represented in the
Formatmodel.Print shop articles are
Immersivebut they have no main media, instead they use the first element in the body as the 'main' image, typically setting the weighting of this image toimmersive(but not always). Normally, the headline for anImmersivearticle is inverted and overlaid onto the main media but for Print Shop articles they expect the headline to sit above the first element, more akin to a normal article.To accomodate these differences we had to create a noMainMedia prop in DCR but this is a poor solution and does not allow us to use the Editorial Palette to decide the headline colours.
Design.PrintShopwould allow us to encapsulate these articles purely within the modelWhat about David Squires?
Naturally, there is an exception. Some print shop articles are different, having the article meta always below the headline and showing the article title in the left column. These are currently rendered as
ng-interactivesbut if the work was done to update Composer then we could support another deisgn,Design.CartoonShop?