Skip to content

fix(BpkInsetBannerSponsored): remove per-item icons from bottom sheet, make title optional#4498

Open
Daniel Villanueva (villanuevadani) wants to merge 3 commits into
mainfrom
fix/QUAR-1531-remove-inset-banner-bottom-sheet-icons
Open

fix(BpkInsetBannerSponsored): remove per-item icons from bottom sheet, make title optional#4498
Daniel Villanueva (villanuevadani) wants to merge 3 commits into
mainfrom
fix/QUAR-1531-remove-inset-banner-bottom-sheet-icons

Conversation

@villanuevadani
Copy link
Copy Markdown
Contributor

Summary

  • Removes the hardcoded ViewIcon (item 0) and InfoIcon (subsequent items) that were rendered inside each bottom-sheet section of BpkInsetBannerSponsored V2. The DSA Legal Names spec calls for icon-free section content across all surfaces.
  • Makes bottomSheetContent[].title optional (title?: string) so a title-less first section (e.g. the DSA intro body) is a valid shape.
  • Replaces key={item.title} with a stable key={item.title ?? \section-${index}`}` to avoid React key collisions when a title is absent.
  • The CTA trigger (i) icon that opens the bottom sheet is unchanged — this PR only affects the per-item icons inside the sheet.
  • Deletes the &--bottom-sheet-icon SCSS rule since the wrapper it styled is gone.
  • Updates tests to assert icon absence and adds a DSA-shape story (title-less intro + two titled sections).

Visual change

Each bottom-sheet item previously rendered a leading icon (ViewIcon for the first, InfoIcon for the rest). After this PR, items render text only. This is a visible change for all consumers.

Known consumers

Repo File Impact
carhire-homepage libs/cars-results/features/src/Adverts/InlinePlusAd/InlinePlusAd.tsx Currently on a local V3 fork (QUAR-1531 work). PR B of that work restores V2 and uses the new title-optional shape. Aligns with DSA Legal Names epic.
web-platform libs/flights-booking-panel/.../SponsoredPricingItem.tsx Passes a single bottom-sheet item — today renders ViewIcon. After this PR it shows title + description only. Cross-team heads-up sent to Sponsored Airlines team.

Checklist

  • Tests updated and passing (npm test -- src/bpk-component-inset-banner)
  • Snapshots updated for iconless variants
  • Story added: WithDsaStyleBottomSheetExampleV2Light
  • common-types.tstitle widened to optional
  • SCSS — &--bottom-sheet-icon rule removed
  • Lint clean

Per the DSA Legal Names spec, bottom-sheet content sections should render
as plain text only — no ViewIcon or InfoIcon per item. This is a clean
removal for all consumers; the CTA trigger (i) icon that opens the sheet
is unchanged.

Also makes bottomSheetContent[].title optional so title-less intro
sections (e.g. the DSA intro body paragraph) can be rendered without
a heading.

Known consumers:
- carhire-homepage InlinePlusAd: benefits directly from icon removal and
  optional title.
- web-platform SponsoredPricingItem: currently shows ViewIcon (index 0);
  after this change it will show title + description only. Confirm with
  Sponsored Airlines team before merging.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 19, 2026 14:24
@villanuevadani Daniel Villanueva (villanuevadani) added the patch Patch production bug label May 19, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates BpkInsetBannerSponsored (V2) to comply with the DSA Legal Names spec by removing per-section icons from the bottom sheet content and allowing bottom-sheet sections to omit a title (to support an intro/body-only first section).

Changes:

  • Removed the hardcoded per-item icons in the bottom sheet content and deleted the now-unused SCSS rule.
  • Made bottomSheetContent[].title optional and updated rendering to only show the title block when present.
  • Updated tests and added a Storybook example covering the “DSA shape” (title-less first item + titled sections).

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
packages/backpack-web/src/bpk-component-inset-banner/src/BpkInsetBannerV2/common-types.ts Widens the bottom sheet item shape by making title optional.
packages/backpack-web/src/bpk-component-inset-banner/src/BpkInsetBannerV2/BpkInsetBannerSponsored.tsx Removes per-item bottom sheet icons and conditionally renders section titles; updates React keys.
packages/backpack-web/src/bpk-component-inset-banner/src/BpkInsetBannerV2/BpkInsetBannerSponsored.stories.tsx Adds a DSA-style story and updates existing copy to reflect icon removal.
packages/backpack-web/src/bpk-component-inset-banner/src/BpkInsetBannerV2/BpkInsetBannerSponsored.module.scss Removes styling for the deleted bottom sheet icon wrapper.
packages/backpack-web/src/bpk-component-inset-banner/src/BpkInsetBannerV2/BpkInsetBannerSponsored-test.tsx Updates assertions for icon absence and adds coverage for title-less first section rendering.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

{callToAction.bottomSheetContent.map((item, index) => (
<div
key={item.title}
key={item.title ?? `section-${index}`}
@skyscanner-backpack-bot
Copy link
Copy Markdown

Visit https://backpack.github.io/storybook-prs/4498 to see this build running in a browser.

…ng with DSA spec

- heading4 → heading5 for section titles (matches InlineAd DSA bottom sheet)
- title margin-bottom: md → sm (tighter gap to description)
- section padding-bottom: xl → lg (matches inter-section rhythm)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@skyscanner-backpack-bot
Copy link
Copy Markdown

skyscanner-backpack-bot Bot commented May 19, 2026

Warnings
⚠️

Package source files (e.g. packages/package-name/src/Component.js) were updated, but snapshots weren't. Have you checked that the tests still pass?

Browser support

If this is a visual change, make sure you've tested it in multiple browsers.

Generated by 🚫 dangerJS against 66d8529

@skyscanner-backpack-bot
Copy link
Copy Markdown

Visit https://backpack.github.io/storybook-prs/4498 to see this build running in a browser.

…A spec

- Remove explicit top padding from bottom sheet (was base, default is none)
- Reduce inter-section padding-bottom: lg → base

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@skyscanner-backpack-bot
Copy link
Copy Markdown

Visit https://backpack.github.io/storybook-prs/4498 to see this build running in a browser.

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

Labels

patch Patch production bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants