WIP [LUNA-3186]: Make marker prop optional in BpkPriceRange (backwards compatible option) by Jack-Waller · Pull Request #4131 · Skyscanner/backpack

Jack Waller (Jack-Waller) · 2026-01-21T18:47:27Z

Summary

Makes the marker prop optional in the BpkPriceRange component. When the marker prop is omitted, the component displays only the three-tier price range bars (low/medium/high segments) without any marker, dot, or badge.

This lets us display price bands without a specific price indicator, whilst maintaining full backward compatibility with existing usage.

Changes

Core Component Updates

✅ Made marker prop optional in the Props type definition
- When present, the functionality of this component remains the same
- When absent, the price indicator tooltip positioned on the price range disappears
✅ Added conditional logic to safely handle undefined marker throughout the component
✅ Updated conditional rendering to only show marker/dot when both marker and type exist
✅ Made dotClassName conditional on type being defined

Examples

Price range with marker provided

`showPriceIndicator = true` (default)

With `showPriceIndicator = false`

Price range with marker missing

`showPriceIndicator = true` (default)

With `showPriceIndicator = false` (Luna don't need this, but this is introduced by this PR)

Test Coverage

✅ Added 4 new unit tests (11 tests total, all passing):
- Test for rendering without marker with labels
- Test for rendering without marker without labels
- Test for marker value changes (ensures re-rendering works correctly)
- Test for min/max changes (ensures useCallback memoisation works correctly)
✅ Added accessibility test for no-marker scenario

Examples and Stories

✅ Created NoMarkerWithLabelsExample - Shows bars with segment price labels
✅ Created NoMarkerWithoutLabelsExample - Shows only bars (cleanest UI)
✅ Added new stories to Storybook for visual regression testing

Documentation

✅ Updated README with:
- Example usage without marker
- Comprehensive props table
- Marker type definition
- Clarified optional vs required props

Verification

✅ TypeScript: No errors (only pre-existing weak warning about UMD global variable)
✅ Tests: All 11 tests passing
✅ Build: Successful compilation
✅ Linting: All checks passed
✅ Visual verification: Tested in Storybook with Chrome DevTools MCP
- Verified no-marker scenarios render correctly
- Verified existing marker behaviour unchanged
- Verified marker position updates correctly when values change

Accessibility

Ability to navigate using a keyboard only
Zoom functionality:
- Functional and readable at 200% text magnification
- Reflows correctly up to 400% zoom
Screen reader compatible (no interactive elements affected)

Breaking Changes

None. This is a purely additive change with full backward compatibility.

References

Jira: LUNA-3186
Component: BpkPriceRange
Affected teams: Luna (Flights), Hotels
Alternative: [LUNA-3186]: Make marker prop optional in BpkPriceRange (breaking change option) #4133

🤖 Generated with Claude Code

- Updated Props type to make marker optional - Added conditional rendering logic for marker and dot - Moved indicatorPercent calculation into useEffect for better performance - Used useCallback to memoize calcPercentage function - Added comprehensive test coverage (11 tests total) - Created NoMarkerWithLabels and NoMarkerWithoutLabels example components - Updated documentation with usage examples and props table - Verified functionality with Chrome DevTools and Storybook Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

skyscanner-backpack-bot · 2026-01-21T18:54:45Z

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

skyscanner-backpack-bot · 2026-01-21T18:56:29Z

	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 4069bd9

Removed useCallback in favour of simpler arrow function approach. All tests continue to pass. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

skyscanner-backpack-bot · 2026-01-21T19:06:42Z

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

Moved all conditional logic into dotClassName computation instead of duplicating checks at render time. This creates a single source of truth and makes the JSX more readable. Before: Check type for className, then marker && type && !showPriceIndicator for render After: Check all conditions for className, then just dotClassName for render All 11 tests continue to pass. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

skyscanner-backpack-bot · 2026-01-21T19:20:11Z

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

skyscanner-backpack-bot · 2026-01-21T19:39:48Z

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

Jack Waller (Jack-Waller) · 2026-01-21T19:02:38Z

  const [prefilledWidth, setPrefilledWidth] = useState(0);
  const calcPercentage = (current: number) =>
    (clamp(current, min, max) - min) / (max - min);
-  const indicatorPercent = calcPercentage(marker.percentage);


I moved this into the effect where it's used, as it's now conditional on if the marker prop is present.

Jack Waller (Jack-Waller) · 2026-01-21T19:57:09Z

-  if (marker.percentage < segments.low.percentage) {
+  if (marker && marker.percentage < segments.low.percentage) {
    type = MARKER_TYPES.LOW;
-  } else if (marker.percentage > segments.high.percentage) {
+  } else if (marker && marker.percentage > segments.high.percentage) {
    type = MARKER_TYPES.HIGH;
  } else {
    type = MARKER_TYPES.MEDIUM;


Defaulting to MEDUIM when the marker isn't provided. I add guards for displaying markers below to prevent this from ever showing. Keeping this as a strictly defined type makes the logic further below simpler.

Jack Waller (Jack-Waller) · 2026-01-21T19:58:25Z

+  const shouldShowMarker = !!marker && showPriceIndicator;
+  const shouldShowDot = !!marker && !showPriceIndicator;


My thinking was to make the meaning of these conditions explicit, rather than complicate the logic below.

Copilot

Pull request overview

This PR makes the marker prop optional in the BpkPriceRange component, enabling display of price range bars without a specific price indicator while maintaining full backward compatibility.

Changes:

Made marker prop optional in TypeScript type definition
Added conditional logic to handle undefined marker values throughout the component
Added comprehensive test coverage including accessibility tests for the no-marker scenario

Reviewed changes

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

Show a summary per file

File	Description
packages/bpk-component-price-range/src/BpkPriceRange.tsx	Updated Props type to make marker optional, added conditional checks for marker usage, and introduced helper variables for conditional rendering
packages/bpk-component-price-range/src/BpkPriceRange-test.tsx	Added 4 new unit tests covering rendering without marker, marker value changes, and min/max recalculation
packages/bpk-component-price-range/src/accessibility-test.tsx	Added accessibility test for no-marker scenario
packages/bpk-component-price-range/README.md	Added documentation for optional marker usage and comprehensive props table
examples/bpk-component-price-range/examples.tsx	Created two new example components demonstrating no-marker usage with and without labels
examples/bpk-component-price-range/stories.tsx	Exported new example components as Storybook stories

Comments suppressed due to low confidence (2)

packages/bpk-component-price-range/src/BpkPriceRange.tsx:72

The variable type is used later in dotClassName even when marker is undefined. This will result in type being assigned MARKER_TYPES.MEDIUM when marker is undefined, which is incorrect. Consider making type optional and only assigning it when marker exists, or handle the undefined case explicitly in the dotClassName logic.

  let type: MarkerType;
  if (marker && marker.percentage < segments.low.percentage) {
    type = MARKER_TYPES.LOW;
  } else if (marker && marker.percentage > segments.high.percentage) {
    type = MARKER_TYPES.HIGH;
  } else {
    type = MARKER_TYPES.MEDIUM;
  }

packages/bpk-component-price-range/src/BpkPriceRange.tsx:123

The dotClassName uses type variable which may be MARKER_TYPES.MEDIUM even when marker is undefined. This could cause incorrect styling. Since dotClassName is only used when shouldShowDot is true (which requires marker), consider moving this className calculation inside a conditional block or only after validating marker exists.

  const dotClassName = getClassName(
    `bpk-price-range__line--${type}`,
    'bpk-price-range__line--dot',
  );

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

skyscanner-backpack-bot · 2026-01-21T20:02:21Z

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

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

skyscanner-backpack-bot · 2026-01-21T20:10:45Z

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

…ice-range

skyscanner-backpack-bot · 2026-01-22T14:37:32Z

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

…ake-marker-optional-in-price-range' into feature/LUNA-3186-make-marker-optional-in-price-range

Jack Waller (Jack-Waller) · 2026-01-22T14:41:31Z

  max?: number;
-  showPriceIndicator?: boolean;
-  marker: Marker;
+  showPriceIndicator?: boolean; // Should be named 'showPriceLabels'.


The meaning of this prop has changed with this PR. As marker is now optional, the presence of any form of marker which notes the position of the current price is determined by the presence or absence of the marker prop.

This showPriceIndicator prop controls two things:

The presence or absence of prices below the change in colour of the bar segments

Example of price below the segments:

Example of no price below the segments

Weather the price marker tooltip is a dot positioned on the range, or a price tooltip displaying the price of the marker

Example of dot

Example of price marker

Unfortunately, re-naming this prop is a breaking change. We're in the process of raising this issue with Backpack, explaining this problem & forming a plan to re-name this variable (and potentially the name of the marker prop too). In the meantime, we've left this code comment to try and make this clearer.

Related discussion

skyscanner-backpack-bot · 2026-01-22T14:53:01Z

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

Michal Sekulski (michmaml) · 2026-01-22T14:58:30Z

+  if (marker && marker.percentage < segments.low.percentage) {
    type = MARKER_TYPES.LOW;
-  } else if (marker.percentage > segments.high.percentage) {
+  } else if (marker && marker.percentage > segments.high.percentage) {


marker && marker.percentage can be written as marker?.percentage.

See line 102 :-)

I tried that 😅 I get a typescript error. I think it's because it could evaluate to undefined > number if marker is undefined:

Rob Waddell (robaw) · 2026-01-22T15:34:05Z

  min = 0,
  segments,
-  showPriceIndicator = true,
+  showPriceIndicator = true, // Should be named 'showPriceLabels'.


Are we keeping these comments? What's the reason behind having a preference for a different name?

not sure we should keep this comment but here's rationale behind preference #4131 (comment)

My thinking was yes, as we think that the showPriceIndicator name is a bit confusing. Changing the name is a breaking change though, which we thought was unnecessarily complex for what we're trying to do for price bands.

The thinking was that this comment could suggest that this prop actually is in control of showing price labels, rather than the indicator itself (hence the preference for a different name).

Rob and I had a quick call to discuss this: my initial assumption against a breaking change would slow us down was false: we'll have to bump the version of backpack anyway. We're now leaning towards making the breaking change, provided if we think that it's necessary to make this name change. Rob's having a think about this, and we'll circle back to it tomorrow.

Jack Waller (Jack-Waller) · 2026-01-29T10:30:05Z

Closing this, as we've decided to go with a different option (#4158)

Jack Waller (Jack-Waller) added the minor Non breaking change label Jan 21, 2026

refactor: simplify calcPercentage to regular arrow function

e69c31d

Removed useCallback in favour of simpler arrow function approach. All tests continue to pass. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

Jack Waller (Jack-Waller) added 2 commits January 21, 2026 19:30

refactor: make dotClassName condition explicit

7063f41

refactor: run prettier

1fa6252

refactor: extract semantic booleans

2d67932

Jack Waller (Jack-Waller) commented Jan 21, 2026

View reviewed changes

Jack Waller (Jack-Waller) marked this pull request as ready for review January 21, 2026 19:59

Copilot AI review requested due to automatic review settings January 21, 2026 19:59

Copilot AI reviewed Jan 21, 2026

View reviewed changes

Comment thread packages/bpk-component-price-range/src/BpkPriceRange.tsx Outdated

fix: use marker percentage explicitly

3e83fdd

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Jack Waller (Jack-Waller) and others added 2 commits January 22, 2026 14:23

chore: clarify confusing prop naming

1d783eb

Merge branch 'main' into feature/LUNA-3186-make-marker-optional-in-pr…

9247317

…ice-range

Jack Waller (Jack-Waller) added 2 commits January 22, 2026 14:45

docs(README): clarifying showPriceIndicator prop

fa9aa51

Merge remote-tracking branch 'refs/remotes/origin/feature/LUNA-3186-m…

4069bd9

…ake-marker-optional-in-price-range' into feature/LUNA-3186-make-marker-optional-in-price-range

Jack Waller (Jack-Waller) commented Jan 22, 2026

View reviewed changes

Michal Sekulski (michmaml) reviewed Jan 22, 2026

View reviewed changes

Rob Waddell (robaw) reviewed Jan 22, 2026

View reviewed changes

Comment thread packages/bpk-component-price-range/src/BpkPriceRange.tsx

Jack Waller (Jack-Waller) mentioned this pull request Jan 22, 2026

[LUNA-3186]: Make marker prop optional in BpkPriceRange (breaking change option) #4133

Closed

12 tasks

Jack Waller (Jack-Waller) changed the title ~~[LUNA-3186]: Make marker prop optional in BpkPriceRange~~ [LUNA-3186]: Make marker prop optional in BpkPriceRange (backwards compatible option) Jan 22, 2026

Jack Waller (Jack-Waller) marked this pull request as draft January 27, 2026 14:28

Jack Waller (Jack-Waller) changed the title ~~[LUNA-3186]: Make marker prop optional in BpkPriceRange (backwards compatible option)~~ WIP [LUNA-3186]: Make marker prop optional in BpkPriceRange (backwards compatible option) Jan 27, 2026

Jack Waller (Jack-Waller) closed this Jan 29, 2026

Jack Waller (Jack-Waller) deleted the feature/LUNA-3186-make-marker-optional-in-price-range branch April 22, 2026 11:08

		const shouldShowMarker = !!marker && showPriceIndicator;
		const shouldShowDot = !!marker && !showPriceIndicator;

Conversation

Jack Waller (Jack-Waller) commented Jan 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Core Component Updates

Examples

Price range with marker provided

showPriceIndicator = true (default)

With showPriceIndicator = false

Price range with marker missing

showPriceIndicator = true (default)

With showPriceIndicator = false (Luna don't need this, but this is introduced by this PR)

Test Coverage

Examples and Stories

Documentation

Verification

Accessibility

Breaking Changes

References

Uh oh!

skyscanner-backpack-bot Bot commented Jan 21, 2026

Uh oh!

skyscanner-backpack-bot Bot commented Jan 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Browser support

Uh oh!

skyscanner-backpack-bot Bot commented Jan 21, 2026

Uh oh!

skyscanner-backpack-bot Bot commented Jan 21, 2026

Uh oh!

skyscanner-backpack-bot Bot commented Jan 21, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

skyscanner-backpack-bot Bot commented Jan 21, 2026

Uh oh!

skyscanner-backpack-bot Bot commented Jan 21, 2026

Uh oh!

skyscanner-backpack-bot Bot commented Jan 22, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

skyscanner-backpack-bot Bot commented Jan 22, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Jack Waller (Jack-Waller) Jan 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Jack Waller (Jack-Waller) Jan 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Jack Waller (Jack-Waller) commented Jan 29, 2026

Uh oh!

Reviewers

Assignees

Jack Waller (Jack-Waller) commented Jan 21, 2026 •

edited

Loading

`showPriceIndicator = true` (default)

With `showPriceIndicator = false`

`showPriceIndicator = true` (default)

With `showPriceIndicator = false` (Luna don't need this, but this is introduced by this PR)

skyscanner-backpack-bot Bot commented Jan 21, 2026 •

edited

Loading

Jack Waller (Jack-Waller) Jan 22, 2026 •

edited

Loading

Jack Waller (Jack-Waller) Jan 22, 2026 •

edited

Loading