Skip to content

feat(CHOD-11867): Add salla-cart-summary-card component with responsive#899

Draft
abdomursal wants to merge 1 commit intomasterfrom
feature/CHOD-11867-cart-summary-card
Draft

feat(CHOD-11867): Add salla-cart-summary-card component with responsive#899
abdomursal wants to merge 1 commit intomasterfrom
feature/CHOD-11867-cart-summary-card

Conversation

@abdomursal
Copy link
Copy Markdown
Contributor

@abdomursal abdomursal commented May 4, 2026

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

  • feature

What is the current behaviour? (You can also link to an open issue here)

What is the new behaviour? (You can also link to the ticket here)

Does this PR introduce a breaking change?

Screenshots (If appropriate)

Greptile Summary

This PR refactors the cart page sidebar by replacing the hand-crafted summary HTML block with the new <salla-cart-summary-card> web component and extracts salla-cart-coupons into its own styled card above the free-shipping bar.

Confidence Score: 4/5

Safe to merge with minor suggestions addressed.

Only P2 findings remain after dropping hook feedback; P2s alone yield a 4/5.

src/views/pages/cart.twig — verify options_total signalling and mobile padding intent.

Important Files Changed

Filename Overview
src/views/pages/cart.twig Replaces the static summary card HTML with the new salla-cart-summary-card web component and moves salla-cart-coupons to a standalone card; minor concerns around options_total signalling and mobile padding.
public/app.css Minified/compiled CSS bundle — contains generated styles for the new salla-cart-summary-card component; expected build artifact.

Fix All in Claude Code Fix All in Cursor

Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
src/views/pages/cart.twig:59
**`pb-32` mobile bottom padding is very large**

The bottom padding on mobile was changed from `pb-6` (24 px) to `pb-32` (128 px). If this is to accommodate a fixed/sticky checkout button rendered by the new `salla-cart-summary-card` component, consider tying the padding to the component's presence or using a more targeted selector — a hard-coded 128 px spacer will apply even when the cart is empty and may produce unexpected blank space on certain viewports.

### Issue 2 of 2
src/views/pages/cart.twig:221-230
**`options_total` passed without an options-presence signal**

The old template conditionally rendered the options total row only when `cart.options|length > 0`. The new `salla-cart-summary-card` component receives `options_total: cart.options_total` but no count or existence flag. If the component decides visibility based solely on whether `options_total` is truthy/non-zero, carts where options exist but sum to zero cost may silently hide the row. Consider passing `options_count: cart.options|length` alongside `options_total` so the component can replicate the same conditional the old template used.

Reviews (1): Last reviewed commit: "feat(CHOD-11867): update cart summary ca..." | Re-trigger Greptile

Greptile also left 1 inline comment on this PR.

@sallainternalbot sallainternalbot Bot marked this pull request as draft May 4, 2026 09:55
@codacy-production
Copy link
Copy Markdown
Contributor

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

AI Reviewer: run a review on demand. To trigger the first review automatically, go to your organization or repository integration settings. AI can make mistakes. Always validate suggestions.

Run reviewer

TIP This summary will be updated as you push new changes.

Comment thread src/views/pages/cart.twig
Comment on lines +221 to +230
<salla-cart-summary-card initial-cart="{{ {
count: cart.count,
total: cart.total,
sub_total: cart.sub_total,
total_discount: cart.total_discount,
tax_amount: cart.tax_amount,
options_total: cart.options_total,
real_shipping_cost: cart.real_shipping_cost,
has_shipping: cart.has_shipping
}|json_encode }}"></salla-cart-summary-card>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 options_total passed without an options-presence signal

The old template conditionally rendered the options total row only when cart.options|length > 0. The new salla-cart-summary-card component receives options_total: cart.options_total but no count or existence flag. If the component decides visibility based solely on whether options_total is truthy/non-zero, carts where options exist but sum to zero cost may silently hide the row. Consider passing options_count: cart.options|length alongside options_total so the component can replicate the same conditional the old template used.

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/views/pages/cart.twig
Line: 221-230

Comment:
**`options_total` passed without an options-presence signal**

The old template conditionally rendered the options total row only when `cart.options|length > 0`. The new `salla-cart-summary-card` component receives `options_total: cart.options_total` but no count or existence flag. If the component decides visibility based solely on whether `options_total` is truthy/non-zero, carts where options exist but sum to zero cost may silently hide the row. Consider passing `options_count: cart.options|length` alongside `options_total` so the component can replicate the same conditional the old template used.

How can I resolve this? If you propose a fix, please make it concise.

Fix in Claude Code Fix in Cursor

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant