EMULSIF-304: Banner component.#152
Conversation
callinmullaney
left a comment
There was a problem hiding this comment.
@robherba This PR matches the design but I have some feedback for you:
-
Lets try and avoid committing images (png, gif, jpg) directly to the repository and use a free service instead. https://picsum.photos/ is what we've used for Compound and seemed to work well. Here is an example of how we crafted a URL which defines the image size. I don't think you need to make this a responsive image so a standard
<img src />should be fine: https://github.com/emulsify-ds/compound/blob/main/components/02-molecules/text-with-media/text-with-media.yml#L4C16-L4C37 -
See code comments on
banner.twig -
Lets combine the
bem()andadd_attributes()functions. For example:
{% set banner__attributes = {
'data-banner-alignment': banner__alignment|default('left'),
'class': bem(banner__base_class, banner__modifiers, banner__blockname)
} %}
<div {{ add_attributes(banner__attributes) }}> ... </div>
- Avoid using CSS background images. Lets use the image component instead. Pull it it below all the banner content/text. Then use
position:absolute,z-index, andobject-fitstyles. This makes wiring a lot easier and allows us to use responsive images.
…ulsif-304--banner-component
✅ Deploy Preview for emulsify-ui-kit ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Hey @callinmullaney, the changes have been applied! |
|
➤ Josue commented: Hi @Callin Mullaney reviewing the backlog and PRs, we noticed you left feedback to Roberto, he already applied the feedback so tagging you to follow up the review, hopefully for this quarter maybe the contrib day (Q3 2025) |
…ulsif-304--banner-component
|
Hi @callinmullaney! When you return, could you help me with an approval here, please? |
|
🎉 This PR is included in version 1.8.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Summary
Adds an banner component.
How to review this pull request