Skip to content

feat: new Featured Grid component - #727

Draft
petheanraj-mitrah wants to merge 1 commit into
stagefrom
DEVSITE-2523
Draft

feat: new Featured Grid component#727
petheanraj-mitrah wants to merge 1 commit into
stagefrom
DEVSITE-2523

Conversation

@petheanraj-mitrah

Copy link
Copy Markdown
Collaborator

@aem-code-sync

aem-code-sync Bot commented Aug 12, 2026

Copy link
Copy Markdown

Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch and validate page speed.
In case there are problems, just click a checkbox below to rerun the respective action.

  • Re-run all PSI checks
  • Re-run failed PSI checks
  • Re-sync branch
Commits

@aem-code-sync

aem-code-sync Bot commented Aug 12, 2026

Copy link
Copy Markdown
Page Scores Audits Google
📱 /test/petheanraj/feature-grid PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI
🖥️ /test/petheanraj/feature-grid PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@github-actions

Copy link
Copy Markdown

❌ Test Results

Status: Some tests failed!

🔍 Click to view failed tests
📁 test/blocks/code/code.test.js:

❌ Code block > code > data-playground attributes from class
      AssertionError: expected null to equal 'code-session'
        at n.<anonymous> (test/blocks/code/code.test.js:29:63)


📁 test/blocks/columns/columns.test.js:

❌ Columns block > Columns > columns-container
      AssertionError: expected false to be true
      + expected - actual
      
      -false
      +true
      
      at n.<anonymous> (test/blocks/columns/columns.test.js:29:117)

❌ Columns block > Columns > buttons
      AssertionError: expected false to be true
      + expected - actual
      
      -false
      +true
      
      at test/blocks/columns/columns.test.js:100:86
      at NodeList.forEach (<anonymous>)
      at n.<anonymous> (test/blocks/columns/columns.test.js:95:50)


📁 test/blocks/embed/embed.test.js:

❌ Could not import your test module. Check the browser logs or open the browser in debug mode for more information.


📁 test/blocks/contributors/contributors.test.js:

❌ Contributors block > contributors > firstDiv
      AssertionError: expected null to exist
        at n.<anonymous> (test/blocks/contributors/contributors.test.js:32:28)

❌ Contributors block > contributors > remove divs without children
      AssertionError: expected false to be true
      + expected - actual
      
      -false
      +true
      
      at test/blocks/contributors/contributors.test.js:42:44
      at NodeList.forEach (<anonymous>)
      at n.<anonymous> (test/blocks/contributors/contributors.test.js:41:51)

❌ Contributors block > contributors > last update div
      AssertionError: expected '' to equal 'https://github.com/AdobeDocs/express-add-ons-docs/commits/main/src/pages/references/index.md'
      + expected - actual
      
      +https://github.com/AdobeDocs/express-add-ons-docs/commits/main/src/pages/references/index.md
      
      at n.<anonymous> (test/blocks/contributors/contributors.test.js:53:40)

❌ Contributors block > contributors > image list div
      AssertionError: expected null to exist
        at test/blocks/contributors/contributors.test.js:71:27
        at Array.forEach (<anonymous>)
        at n.<anonymous> (test/blocks/contributors/contributors.test.js:69:33)


📁 test/blocks/tab/tab-playground.test.js:

❌ Tab block playground metadata > extracts data-playground attributes via decoratePreformattedCode
      AssertionError: expected null to equal 'tab-session'
        at n.<anonymous> (test/blocks/tab/tab-playground.test.js:17:63)


📁 test/blocks/tab/tab.test.js:

❌ Tab block > Tab Button Structure > tab > button load structure
      AssertionError: expected 'tab-button active' to equal 'tab-button'
      + expected - actual
      
      -tab-button active
      +tab-button
      
      at n.<anonymous> (test/blocks/tab/tab.test.js:60:38)

❌ Tab block > Sub-tabs > sub-tab > attributes
      AssertionError: expected 'subTab1' to equal 'subTab3'
      + expected - actual
      
      -subTab1
      +subTab3
      
      at test/blocks/tab/tab.test.js:135:58
      at NodeList.forEach (<anonymous>)
      at n.<anonymous> (test/blocks/tab/tab.test.js:133:23)



Test Coverage Report

Overall Coverage Summary

Metric Percentage Coverage
Statements 57.3% 3923/6846
Branches 85.19% 564/662
Functions 52.4% 131/250
Lines 57.3% 3923/6846

Coverage by File/Directory

File Statements Branches Functions Lines
blocks/accordion 96.73% 100% 100% 96.73%
blocks/announcement 76.69% 60% 100% 76.69%
blocks/banner 94.28% 62.5% 100% 94.28%
blocks/cards 92.04% 90% 100% 92.04%
blocks/carousel 77.8% 82.35% 71.42% 77.8%
blocks/code 81.81% 60% 100% 81.81%
blocks/columns 62.59% 75% 100% 62.59%
blocks/contributors 84.23% 56% 100% 84.23%
blocks/edition 91.37% 57.14% 100% 91.37%
blocks/fragment 17.03% 100% 0% 17.03%
blocks/image-text 57.5% 55.55% 100% 57.5%
blocks/info 96.87% 100% 100% 96.87%
blocks/info-card 100% 100% 100% 100%
blocks/info-columns 75% 100% 100% 75%
blocks/list 59.09% 85.71% 100% 59.09%
blocks/mini-resource-card 98% 87.5% 100% 98%
blocks/product-card 78.57% 94.11% 100% 78.57%
blocks/profile-card 96.29% 100% 100% 96.29%
blocks/site-hero 93.93% 80% 100% 93.93%
blocks/summary 98.36% 88.88% 100% 98.36%
blocks/tab 93.25% 96.87% 100% 93.25%
blocks/table 100% 84.61% 100% 100%
blocks/text 50% 83.33% 50% 50%
blocks/title 84% 66.66% 100% 84%
components 43.52% 66.66% 53.84% 43.52%
scripts 49.17% 95.28% 43.08% 49.17%

Coverage report generated at 2026-08-12T11:07:49.012Z

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is a new component let's use CSS nesting here.
An LLM should be able to easily do this.

card.style.background = background;

const imageDiv = createTag('div', { class: 'feature-grid-card-image' });
const image = imageSlot?.querySelector('picture img, img');

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

querySelector('picture img, img') seems repetitive.
querySelector('img') should do the same.

@davids-ensemble davids-ensemble changed the title feat : feature grid component feat: new Featured Grid component Aug 12, 2026

@davids-ensemble davids-ensemble left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing inline padding around the grid on lower resolutions

Image

Comment on lines +56 to +58
const picWidth = image.naturalWidth > 0 ? String(image.naturalWidth) : '80';
imageDiv.appendChild(
createOptimizedPicture(image.src, image.alt, false, [{ width: picWidth }]),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No point in doing this naturalWidth stuff. We only display these at 40x40 size. Just pass '80' to optimize.

Unless I'm missing something.

@davids-ensemble davids-ensemble left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For accessibility let's have a ul > li instead of divs.
Each card becomes a list item.
Make sure to add a role="list" on the ul because Safari drops the implicit one when list markers are hidden.
You will of course have to modify the CSS a bit as well.

@davids-ensemble

Copy link
Copy Markdown
Collaborator

And here's two of the things that Claude would want you to fix which are actually worth the result.

  • [LOW] hlx_statics/blocks/feature-grid/feature-grid.js:75Dead defensive selector.
    querySelectorAll('p:not(.button-container)') — the .button-container class is
    only added by lib-helix.js's decorateButtons, not the lib-adobeio.js
    one imported here, so no <p> in this flow ever carries it. The :not(...) is a
    no-op and is already fully covered by the following !p.querySelector('a')
    guard. Remove to avoid implying a decoration path that never runs.
  • [LOW] hlx_statics/blocks/feature-grid/feature-grid.js:45Double button decoration.
    decorateButtons(block) (line 45) styles every <a> (adds label span, unwraps
    <p><strong><a>, assigns Spectrum classes); then styleCardButton (line 9)
    strips most of those classes and re-adds its own. The result is correct — the
    isStrong check survives via the spectrum-Button--accent fallback because
    decorateButtons already unwrapped the <strong> — but the first pass is
    wasted work and the coupling is fragile. Consider decorating buttons only
    inside the card loop.

@davids-ensemble

Copy link
Copy Markdown
Collaborator

Ignore the review comments for now.
We'll have to check with the product team on this one.
I'm moving the PR in draft until we hear back.

@davids-ensemble
davids-ensemble marked this pull request as draft August 12, 2026 22:06
@davids-ensemble davids-ensemble added the pending-product-discussion This can't continue until the product team makes a decision label Aug 12, 2026
@petheanraj-mitrah

Copy link
Copy Markdown
Collaborator Author

Ignore the review comments for now. We'll have to check with the product team on this one. I'm moving the PR in draft until we hear back.

Okay @davids-ensemble

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pending-product-discussion This can't continue until the product team makes a decision

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants