greenpeace / planet4-presentation-layer

Styling and markup for various pages of Master theme for Planet4
https://greenpeace.github.io/planet4-presentation-layer/
1 stars 2 forks source link

ACT questions on integration with backend #67

Closed mcshanea closed 6 years ago

mcshanea commented 6 years ago

1) As we see it, the background is applied to the header block , and not to the whole page. But this means that if the header block itself is smaller (because there is not such a big text) or bigger, then the background will not look good. Shouldn't it be applied to the whole page instead? 2) The image itself has the gradient effect. Is the gradient effect going to be needed to be made by the editors? (Please note that "act" is a page, and we need to have unified functionality for all pages. So, the background needs to be able to be overwritten by the editors, and mutliple images can be used). If the background is applied only to the header block, then we have a problem regarding the background in other pages. Else, the css would need to apply the gradient effect

koyan commented 6 years ago

Question by me (please answer @ me)

mikeselander commented 6 years ago

Acknowledging these questions and will answer them tomorrow mid-PST.

mikeselander commented 6 years ago

(Please note that "act" is a page, and we need to have unified functionality for all pages. So, the background needs to be able to be overwritten by the editors, and mutliple images can be used).

@koyan can you please elaborate on this requirement and how it will work in the backend? How will we be applying multiple background images to a single page? This affects how we style this.

koyan commented 6 years ago

As per @mcshanea original instructions all the pages that we defined would be served as "Wordpress pages" (aka, most of the things you design for, except mainly posts and 404). These included act, take-action, issue, evergreen, homepage etc. The instructions we were working on was that what would make a page be different from an other one, would be what blocks an editor chose to put in it. And since we have multiples of each page (for example, there will be many different "take action" pages, and each one will have a different background, or each office that runs a planet4 site may decide to have a different background at any given moment on their ACT page, based on what the office is working at that moment). Based on that, we have introduced a field in wordpress on the backend, labeled "overide background". I can pass the src (and srcset) from whatever image is selected to the front end. How the frontend team decides to implement this, is up to you .

Not knowing better (and until such a moment that the front end team suggested a better way), until now we were implementing it with a css overide (check: https://github.com/greenpeace/planet4-master-theme/blob/master/templates/page.twig ). If you suggest a different way, we will implement it. But the points are two: a) The background image needs to be able to overwritten by the editors b) The background image needs to be attached to the page itself, and not the header block (because there are pages that do not have a header block).

mcshanea commented 6 years ago

On b) I can't find an example where this is the case i.e. all pages with a background image also have a header, and the background image takes the underlying space that the header provides (therefore it suggests a symbiosis). Am I missing one?

koyan commented 6 years ago

@mcshanea Take action page: https://drive.google.com/drive/u/0/folders/0B-hrpY3C01WJbE5fbGpGQkxNanM

mcshanea commented 6 years ago

There's still a header, it's just a shorter one with only the tags and the title. So the question might be can the background image bleed across other blocks (because the second is a separate block and sits on top). If that's not possible maybe it needs to be lifted out as you suggest.

koyan commented 6 years ago

If the product owner has no problem with it, I have no problem with it.

mikeselander commented 6 years ago

@koyan I think that I have a grasp on this now and we should be OK on both points.

The background image should not be attached to the size of the header itself.

We're OK on this point - the background image element is in the page header element, but it's height is only tied to the width of the page, and not the height of the page header. This can be proven with the following Gif below - I remove all of the header elements (thus collapsing the header), but the background image size stays the same.

load-more-loading

Not knowing better (and until such a moment that the front end team suggested a better way), until now we were implementing it with a css overide (check: greenpeace/planet4-master-theme:templates/page.twig@master ).

If you look at the new markup then you'll see I am loading an img tag in instead of using a CSS background image. Tis should reduce the work that you guys have to do in loading a bunch of different inline styles. Do let me know if you have any questions on this.

The image itself has the gradient effect. Is the gradient effect going to be needed to be made by the editors? (Please note that "act" is a page, and we need to have unified functionality for all pages. So, the background needs to be able to be overwritten by the editors, and mutliple images can be used).

This is written such that the CSS will handle the gradient at the bottom of the image. This will have to be driven by inline CSS for now but I'm working on an easier way of handling these color overrides across the whole site.

Let me know if you have any more questions on this!

koyan commented 6 years ago

If you look at the new markup then you'll see I am loading an img tag in instead of using a CSS background image.

Thanks @mikeselander , that makes more sense. To confirm, you are talking about these lines, correct? https://github.com/greenpeace/planet4-presentation-layer/blob/master/act.html#L728-L730

(we have already applied this markup)

mikeselander commented 6 years ago

@koyan yup - that is it!

mikeselander commented 6 years ago

I'm going to go ahead and close this issue out for now - let us know if there's any more questions @koyan