Create CallToActionAtom component
#15342
Conversation
deedeeh
added
maintenance
deedeeh
force-pushed
the
dina/create-call-to-action-atom-component
branch
from
7423901 to
ce65fb6
Compare
deedeeh
force-pushed
the
dina/create-call-to-action-atom-component
branch
from
ce65fb6 to
94506e5
Compare
|
Hello 👋! When you're ready to run Chromatic, please apply the You will need to reapply the label each time you want to run Chromatic. |
| <div css={[grid.column.all]}>Footer</div> | ||
| </div> | ||
| </main> | ||
| <footer |
There was a problem hiding this comment.
I am not entirely sure if the footer or any of it's content should be just for Web. A confirmation on that would be great.
There was a problem hiding this comment.
I think most of these pages are going to be the same for apps and web. It's not a bad idea to just focus on web to begin with though.
I think the CTA probably doesn't count as a footer technically (sorry I realised the term "footer" was used casually in the ticket title!) and might be better as a component at the bottom of the main article. Keeping it in main section also means you get the grid definitions for free without having to redefine them.
| accentColour: string; | ||
| }; | ||
|
|
||
| export const CallToActionAtom = ({ |
There was a problem hiding this comment.
I know from the design catchup a FeatureCard was used but I believe we said we will have a placeholder for now until we get a final confirmation on the designs.
deedeeh
changed the title
CallToActionAtom component
cemms1
left a comment
cemms1
left a comment
There was a problem hiding this comment.
Really neat code! Just left a few thoughts on this, let me know if you want to chat it through
| return ( | ||
| <CallToActionAtom | ||
| ctaLinkURL="Link URL" | ||
| ctaBackgroundImage="Image URL" |
There was a problem hiding this comment.
Maybe we could use an actual image src here for better component visualisation? Can we copy from an existing article maybe?
There was a problem hiding this comment.
Same for other props ideally
| return ( | ||
| <div | ||
| css={css` | ||
| background-image: url(${ctaBackgroundImage}); |
There was a problem hiding this comment.
This is fine for now, but for the next iteration, we'll want to use a picture element with backup/default img element, similar to the way we do for front cards (https://github.com/guardian/dotcom-rendering/blob/f912e23753ac15a37af49ea0846d2ec256fdd4de/dotcom-rendering/src/components/CardPicture.tsx)
I know this is taken from frontend but it's more semantic to keep images explicitly inside image-based elements rather than as a CSS background
What we do for cards is overlay text and other elements on top of the picture element by using absolute/relative positioning, rather than setting a background on the text elements with an image
I don't think this necessarily holds up this PR as we can do that next, but we should note this somewhere on our to do list
| cssOverrides={css` | ||
| background-color: ${accentColour}; | ||
| `} |
There was a problem hiding this comment.
I think it's generally preferable to use theme properties to style buttons if at all possible
| <div css={[grid.column.all]}>Footer</div> | ||
| </div> | ||
| </main> | ||
| <footer |
There was a problem hiding this comment.
I think most of these pages are going to be the same for apps and web. It's not a bad idea to just focus on web to begin with though.
I think the CTA probably doesn't count as a footer technically (sorry I realised the term "footer" was used casually in the ticket title!) and might be better as a component at the bottom of the main article. Keeping it in main section also means you get the grid definitions for free without having to redefine them.
2 participants
What does this change?
This PR creates the Call To Action Atom component which is included in the Hosted Article and the Hosted Video contents. At the moment we have just 1 layout,
HostedArticleLayout, for hosted articles and videos but we will separate them in a later PR.This is a very basic component with placeholders until we have the approved styling from the designer and with the actual data once we investigate how we can fetch the atom data for hosted content.
The changes are:
CallToActionAtomcomponentCallToActionAtomHostedArticleLayoutWhy?
Hosted Content migration to DCAR
Screenshots
Mobile
Desktop