Fix completion-screen override handling (blank button + uninterpolated {tokens}) - #157
Fix completion-screen override handling (blank button + uninterpolated {tokens})#157gcutrini wants to merge 2 commits into
Conversation
Two problems on the completion screen when marketing overrides are set:
- A button/title override passed as a present-but-undefined prop slipped
through the `!isEmptyString(...)` guard, because isEmptyString only
treated strings as empty. The order-complete button then rendered
blank. isEmptyString now treats null/undefined as empty, so the button
and title overrides fall back to their translated defaults; the
redundant `typeof !== 'undefined'` checks are removed.
- An override paragraph was inserted verbatim, so {attendee}/{adv}/{button}
tokens in custom copy printed literally. Add an interpolate() helper and
run the override paragraphs through it with the same values the built-in
strings use.
Adds unit tests for isEmptyString and interpolate, plus regression tests
for the blank button and the paragraph interpolation.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| // Replaces {token} placeholders in a template string with values from `vars`. | ||
| // Used so marketing-override copy supports the same {attendee}/{adv}/{button} | ||
| // tokens the built-in i18n strings do. Unknown tokens are left untouched. | ||
| export const interpolate = (template, vars = {}) => { |
There was a problem hiding this comment.
maybe you can avoid looping over the template over and over again with something like this:
const TOKEN = /\{(\w+)\}/g;
export const interpolate = (template, vars = {}) => {
if (typeof template !== 'string') return template;
return template.replace(TOKEN, (match, key) =>
key in vars ? String(vars[key]) : match
);
};```
There was a problem hiding this comment.
Done in 8acd6e6, thanks. Single regex pass now.
It also fixes a bug: with the old loop, a value containing a {token} got expanded by the next pass. Added a test for it.
Replace the per-variable split/join loop with one regex pass. A value
that contains a {token} is now inserted as written, not expanded again.
|
@smarcet all my open PRs have the ref: header now, each one pointing to its ClickUp ticket. |
3 participants
ref: https://app.clickup.com/t/86bbm1pzq
Problem
On the purchase-complete screen, marketing overrides were mishandled two ways:
passed the
!isEmptyString(...)guard (isEmptyString only treated strings asempty), so the order-complete button rendered with no text.
{attendee}/{adv}/{button}tokens in custom copy printed literally, while thebuilt-in i18n strings interpolated them.
Fix
isEmptyStringnow treatsnull/undefinedas empty, so every override guard(both buttons + title) falls back to its default. Redundant
typeofchecks removed.interpolate()helper; override paragraphs run through it with the same{attendee}/{adv}/{button}values the defaults use.Tests
isEmptyString(null/undefined/whitespace) andinterpolate.{button}in anoverride paragraph resolves.