You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
[#363] Added a 'card' banner type that shows the featured image uncropped. - #364
BEFORE (every type with a featured image)
┌──────────────────────── .ct-banner__inner ───────────────────────┬──────────────────────┐
│ .container │ __featured-image__ │
│ title, intro (col-m-6) │ wrapper, absolute, │
│ │ 40% wide, full height│
│ │ banner_featured │
│ │ 4:3 focal crop │
│ │ + object-fit: cover │
│ │ display:none < 768px │
└──────────────────────────────────────────────────────────────────┴──────────────────────┘
AFTER (type = card)
┌──────────────────────────── .ct-banner__inner ───────────────────────────────┐
│ .container > .row.ct-banner__card-layout (align-items: center) │
│ ┌──────── col-m-7 ────────────────┐ ┌──────── col-m-5 ────────────────┐ │
│ │ breadcrumb, title, intro │ │ .ct-banner__card │ │
│ │ │ │ img via banner_card │ │
│ │ │ │ scale to fit 960x960, WebP │ │
│ │ │ │ own ratio, radius + shadow │ │
│ └─────────────────────────────────┘ └─────────────────────────────────┘ │
│ < 768px: the card stacks below the intro, full container width │
└──────────────────────────────────────────────────────────────────────────────┘
default / large / intro: markup and banner_featured unchanged
Checklist before requesting a review
Subject includes ticket number as [#123] Verb in past tense.
Ticket number #123 added to description
Added context in Changed section
Self-reviewed code and commented in commented complex areas.
Added tests for fix/feature.
Relevant tests run and passed locally.
Changed
Editors can now pick card as the banner type on project, blog and civictheme_page nodes and on the civictheme_banner block. A card banner renders the featured image inside .container as the second grid column, through the new banner_card image style, at the image's own aspect ratio on desktop and on phones.
The existing banner pushes the featured image through banner_featured (a 1090x818 focal-point crop) and then object-fit: cover into a box 40% wide and as tall as the banner, and hides that box below 768px. When the image is a designed graphic rather than a photo, such as the Behat Screenshot and Git Artifact work pages, the logo and the code are cut off on desktop and nothing shows on mobile.
After merge, nothing changes on screen until an editor switches a page to card. Pages on default, large and intro keep their markup and banner_featured. The 1 behaviour fix outside card: when a node says inherit, the theme now passes the banner block's type instead of the literal inherit (which Twig rendered as default). The only banner block is default, so no page renders differently today.
Config
New image.style.banner_card: image_scale 960x960 with upscale off, then image_convert to WebP. No crop effect.
card|Card added to the allowed values of field.storage.node.field_c_n_banner_type and field.storage.block_content.field_c_b_banner_type. Adding a value is allowed on a field with data, so drush cim applies it without a deploy hook.
Preprocess (includes/banner.inc)
_drevops_preprocess_block__civictheme_banner() resolves the effective type once: the node's value, or the block's when the node says inherit.
_drevops_banner_apply_image_styles() takes that type and resolves the featured image through banner_card for card, banner_featured otherwise. Both style names stay literal arguments so ImageStyleConfigTest finds them when it scans the theme.
Component (components/03-organisms/banner/)
banner.twig whitelists card. For card it wraps the text rows in col-xxs-12 col-m-7 and renders the image in a sibling col-xxs-12 col-m-5, and skips the absolute __featured-image__wrapper. The 4 repeated featured_image ? col-m-6 ternaries became 1 column_class variable.
banner.scss adds .ct-banner-type--card: the row is vertically centred from the m breakpoint, the card gets a top margin when stacked, and the image is display: block; width: 100%; height: auto with $ct-banner-card-border-radius and $ct-banner-card-shadow from variables.components.scss.
banner.component.yml and the Storybook type control list card.
The image atom already fills width and height through do_image_dimensions(), which applies the style's transform, so the card reserves its space before loading (for example 960x549 for a 1344x768 source).
Tests
ImageStyleConfigTest::testCardImageScalesWithoutCrop() asserts the 960x960 bounds, no upscale and exactly image_scale then image_convert. The existing exported and WebP checks pick up banner_card from the theme scan (15 tests).
New banner_card.feature: card is offered on the node and block forms; a card project renders the image inside the content grid through banner_card as WebP with no side wrapper; a default project still renders the side wrapper through banner_featured.
Docs: docs/performance.md lists banner_card and describes when the card type uses it.
After deploy, switch the Behat Screenshot and Git Artifact work pages to card in the editor. This PR doesn't change any content.
Screenshots
Local build with the Behat Screenshot work page switched to card, desktop at 1280px:
Phone at 390px, with the card below the intro:
Summary by CodeRabbit
New Features
Added a Card banner option for project pages and banner blocks.
Card banners display text alongside a featured image, with responsive layout, rounded corners, and a shadow.
Card images are optimized for display at up to 960 × 960 pixels while preserving their original proportions.
Documentation
Updated performance guidance with details about Card banner image sizing and layout.
No actionable comments were generated in the recent review.
Walkthrough
The banner supports a new card type with a dedicated 960×960 WebP image style. Preprocessing selects the effective banner type, and the template renders card banners with a separate image column. Configuration, styles, tests, and performance documentation cover the new type.
Changes: Card Banner
Layer / File(s)
Summary
Card type and image style config/default/field.storage.block_content.field_c_b_banner_type.yml, config/default/field.storage.node.field_c_n_banner_type.yml, config/default/image.style.banner_card.yml, web/themes/custom/drevops/includes/banner.inc
Both banner type fields allow card. The new image style scales images to 960×960 without upscaling, then converts them to WebP. Preprocessing uses the node type unless it is INHERIT, then uses the block type. It selects banner_card for card banners and banner_featured otherwise.
The template renders the card image in a separate column and uses full-width text columns for the card layout. Component styles add responsive card spacing, image sizing, rounded corners, and a shadow. Component metadata and stories list card as a banner type.
Card banner validation and documentation web/modules/custom/do_base/tests/src/Unit/ImageStyleConfigTest.php, tests/behat/features/banner_card.feature, docs/performance.md
Unit tests check the image style dimensions, effects, and upscale setting. Behat scenarios check type availability and card and default banner output. Performance documentation describes the style and type selection.
sequenceDiagram
participant BannerPreprocess as banner.inc
participant ImageStyle as banner_card
participant BannerTemplate as banner.twig
participant Browser
BannerPreprocess->>ImageStyle: Select style for effective card type
ImageStyle->>BannerTemplate: Supply styled featured image
BannerTemplate->>Browser: Render card layout and image
Loading
Suggested reviewers:nsineok
Merge Risk: Minimal · up to 3f736
The card banner change appears mergeable after normal checks; no unresolved issue was identified.
Pre-merge checks: 5
Check name
Status
Explanation
Description Check
Check skipped - CodeRabbit’s high-level summary is enabled.
Title check
The title names the main change: a new selectable 'card' banner type that displays the featured image uncropped. The changeset adds the card type to field storage, the banner Twig/SCSS/JS components, …
Linked Issues check
Issue #363 acceptance criteria are met by the changes between merge base 4b789dc and head 3f73679.
(a) Selectable card type [#363]: The config diff adds card|Card to field.storage.node.field_c_n_bann…
Out of Scope Changes check
All changed files relate to the #363 card banner objective: - Config: field storage allowed values for node and block banner type; new image.style.banner_card. - banner.inc: effective type (node value…
Docstring Coverage
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Full details: Title check
Explanation
The title names the main change: a new selectable 'card' banner type that displays the featured image uncropped. The changeset adds the card type to field storage, the banner Twig/SCSS/JS components, the banner_card image style (resize-only 960x960, WebP), and the banner.inc helper logic. The tests and docs also match. The title also cites issue #363, which the linked issue confirms. The PR objectives mention PR #364, but the title's issue reference is consistent with the linked issue, so this does not affect the verdict.
Full details: Linked Issues check
Explanation
Issue #363 acceptance criteria are met by the changes between merge base 4b789dc and head 3f73679.
(a) Selectable card type [#363]: The config diff adds card|Card to field.storage.node.field_c_n_banner_type and field.storage.block_content.field_c_b_banner_type (per the change summary, not re-read in this review). banner.twig whitelists 'card' in the type check.
(b) Whole image shown, uncropped, desktop and mobile [#363]: For type card with a featured image, banner.twig sets is_card and renders the image in a col-xxs-12 col-m-5 column with no absolute __featured-image__wrapper. The wrapper is rendered only when 'featured_image and not is_card'. banner.scss sets width: 100%, height: auto and display: block on .ct-banner__card-image, with no object-fit or crop. The column stacks below the intro under the m breakpoint, so it is not hidden on mobile.
(c) WebP, at most 960px, through banner_card [#363]: banner.inc passes 'banner_card' to civictheme_media_image_get_variables() when $type === 'card'. The banner_card style is a 960x960 image_scale with upscale off, then image_convert to WebP (per the change summary and ImageStyleConfigTest::testCardImageScalesWithoutCrop, not re-read in this review).
(d) default, large and intro unchanged [#363]: For these types column_class evaluates to the same classes as before (col-xxs-12 col-m-6 with a featured image, col-xxs-12 without). The featured-image wrapper still renders and still uses banner_featured. The inherit change in banner.inc resolves to the block type, which is default in the only current banner block, so no current page changes. This change is required for the card effective-type rule in #363.
Residual uncertainty: #363 asks that the card <img> carry the derivative width and height to reserve layout space. The reviewed diff does not change the dimension handling. The PR description states that do_image_dimensions() supplies these attributes. This was not verified in the reviewed code. It is not a listed acceptance criterion.
Full details: Out of Scope Changes check
Explanation
All changed files relate to the #363 card banner objective:
Config: field storage allowed values for node and block banner type; new image.style.banner_card.
banner.inc: effective type (node value, or block type when the node says inherit) and selection of banner_card for card. This is required by #363.
banner.twig: card whitelist, column_class, card column markup, and the wrapper guard. These implement the desktop and mobile card layout.
banner.scss and variables.components.scss: card layout, image, radius and shadow tokens. #363 asks for rounded corners and a shadow.
banner.component.yml and banner.stories.js: list card as a type value.
ImageStyleConfigTest and banner_card.feature: tests for the new style and card rendering. #363 requests these tests.
docs/performance.md: documents banner_card. #363 requests this update.
No unrelated functional change was found in the reviewed diffs. The changes to config, tests, and docs were assessed from the change summary and were not individually re-read.
Full details: Docstring Coverage
Explanation
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 2 files. (10 skipped: 10 unsupported.)
Finishing Touches
Generate docstrings
Commit to this branch
Create a new PR
Generate unit tests (beta)
Commit to this branch
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Recent review info
Run configuration
Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: Organization UI
Review profile: ASSERTIVE
Plan: Team
Run ID: 23f2da9a-8eb2-4c24-b05e-6fe2e2f0d956
Commits
Reviewing files that changed from the base of the PR and between 4b789dc and 3f73679.
Included review availability: This review used your included allowance. 1 included review remains after this review. Your included PR review attempts over the past 7 days set your current allowance at 8 reviews per hour.
This file contains hidden or 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
A1Board worker 1AUTOMERGEPull request has been approved and set to automerge
1 participant
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.
AlexSkrypnyk
added
A1
Closes #363
Before / After
Checklist before requesting a review
[#123] Verb in past tense.#123added to descriptionChangedsectionChanged
Editors can now pick
cardas the banner type onproject,blogandcivictheme_pagenodes and on thecivictheme_bannerblock. Acardbanner renders the featured image inside.containeras the second grid column, through the newbanner_cardimage style, at the image's own aspect ratio on desktop and on phones.The existing banner pushes the featured image through
banner_featured(a 1090x818 focal-point crop) and thenobject-fit: coverinto a box 40% wide and as tall as the banner, and hides that box below 768px. When the image is a designed graphic rather than a photo, such as the Behat Screenshot and Git Artifact work pages, the logo and the code are cut off on desktop and nothing shows on mobile.After merge, nothing changes on screen until an editor switches a page to
card. Pages ondefault,largeandintrokeep their markup andbanner_featured. The 1 behaviour fix outsidecard: when a node saysinherit, the theme now passes the banner block's type instead of the literalinherit(which Twig rendered asdefault). The only banner block isdefault, so no page renders differently today.image.style.banner_card:image_scale960x960 with upscale off, thenimage_convertto WebP. No crop effect.card|Cardadded to the allowed values offield.storage.node.field_c_n_banner_typeandfield.storage.block_content.field_c_b_banner_type. Adding a value is allowed on a field with data, sodrush cimapplies it without a deploy hook.includes/banner.inc)_drevops_preprocess_block__civictheme_banner()resolves the effective type once: the node's value, or the block's when the node saysinherit._drevops_banner_apply_image_styles()takes that type and resolves the featured image throughbanner_cardforcard,banner_featuredotherwise. Both style names stay literal arguments soImageStyleConfigTestfinds them when it scans the theme.components/03-organisms/banner/)banner.twigwhitelistscard. Forcardit wraps the text rows incol-xxs-12 col-m-7and renders the image in a siblingcol-xxs-12 col-m-5, and skips the absolute__featured-image__wrapper. The 4 repeatedfeatured_image ? col-m-6ternaries became 1column_classvariable.banner.scssadds.ct-banner-type--card: the row is vertically centred from thembreakpoint, the card gets a top margin when stacked, and the image isdisplay: block; width: 100%; height: autowith$ct-banner-card-border-radiusand$ct-banner-card-shadowfromvariables.components.scss.banner.component.ymland the Storybooktypecontrol listcard.widthandheightthroughdo_image_dimensions(), which applies the style's transform, so the card reserves its space before loading (for example960x549for a 1344x768 source).ImageStyleConfigTest::testCardImageScalesWithoutCrop()asserts the 960x960 bounds, no upscale and exactlyimage_scalethenimage_convert. The existing exported and WebP checks pick upbanner_cardfrom the theme scan (15 tests).banner_card.feature:cardis offered on the node and block forms; acardproject renders the image inside the content grid throughbanner_cardas WebP with no side wrapper; adefaultproject still renders the side wrapper throughbanner_featured.docs/performance.mdlistsbanner_cardand describes when thecardtype uses it.After deploy, switch the Behat Screenshot and Git Artifact work pages to
cardin the editor. This PR doesn't change any content.Screenshots
Local build with the Behat Screenshot work page switched to
card, desktop at 1280px:Phone at 390px, with the card below the intro:
Summary by CodeRabbit