Conversation
| } | ||
|
|
||
| // Custom pp (PayPal monogram) is always eligible - bypass SDK check | ||
| if (source === "pp") { |
There was a problem hiding this comment.
This bypass lives in isFundingEligible(), which is shared by both buttons and marks. Since "pp" is a marks-only concept, it shouldn't be here. If someone calls paypal.Buttons({ fundingSource: "pp" }), this bypass would return true and the button render would fail downstream because there's no Logo or Label in the pp config.
Remove this block entirely. Move the check into component.jsx's isEligible() instead, gated on the rebrand experiment. Two changes in component.jsx:
1. Hoist isRebrandEnabled from inside render() (line 157) to the top of the Marks() function, right after the hasShippingCallback declaration (line 99):
const hasShippingCallback = Boolean(
onShippingChange || onShippingAddressChange || onShippingOptionsChange
);
const isRebrandEnabled = experiment?.isPaypalRebrandEnabled;Then remove line 157 (const isRebrandEnabled = experiment?.isPaypalRebrandEnabled; inside render()). The hoisted variable is used by both isEligible() and render().
2. Add the "pp" check in isEligible() (after the if (!fundingSource) block at line 127):
const isEligible = () => {
if (!fundingSource) {
return true;
}
if (fundingSource === "pp") {
return Boolean(isRebrandEnabled);
}
return isFundingEligible(fundingSource, { ... });
};This keeps "pp" knowledge in the marks layer and prevents a crash in the standalone template (template.jsx) when rebrand is OFF. That template uses getFundingConfig() which doesn't include "pp", so it would throw Can not find funding config for pp.
| env, | ||
| }: MarkOptions): ChildNodeType { | ||
| const fundingConfig = getFundingConfig()[fundingSource]; | ||
| let fundingConfig; |
There was a problem hiding this comment.
This branching between getFundingConfig() and getMarksFundingConfig() adds complexity that isn't needed. getMarksFundingConfig() is a superset of getFundingConfig() (it spreads getFundingConfig() and adds pp), so it works for ALL funding sources. Replace this entire block with:
const fundingConfig = getMarksFundingConfig()[fundingSource];
if (!fundingConfig) {
throw new Error(`Can not find funding config for ${fundingSource}`);
}
const { Logo } = fundingConfig;
const MarkLogo = fundingConfig.Mark;And update the import at line 14 to remove getFundingConfig:
import { getMarksFundingConfig } from "../funding";Single lookup, single null check, no branching.
| ); | ||
| } | ||
|
|
||
| export function getPPConfig(): FundingSourceConfig { |
There was a problem hiding this comment.
"pp" is only used to render an alternative mark for the PayPal funding source. It's not a real funding source, so it doesn't need DEFAULT_FUNDING_CONFIG, flows, layouts, colors, or the FundingSourceConfig return type. Those are all button rendering properties that have no effect here.
Strip this down to just the Mark component:
/* @flow */
/** @jsx node */
import { node, type ChildType } from "@krakenjs/jsx-pragmatic/src";
import {
PPRebrandLogoExternalImage,
PPRebrandLogoInlineSVG,
} from "@paypal/sdk-logos/src";
import { LOGO_COLOR } from "@paypal/sdk-logos/src/constants";
// Alternative mark for the PayPal funding source (monogram icon).
// Not a real funding source - only used in rebrand marks rendering.
function Mark(): ChildType {
return __WEB__ ? (
<PPRebrandLogoExternalImage logoColor={LOGO_COLOR.BLUE} />
) : (
<PPRebrandLogoInlineSVG />
);
}
export function getPPConfig() {
return {
Mark,
};
}The imports for BUTTON_COLOR, BUTTON_LAYOUT, BUTTON_FLOW, DEFAULT_FUNDING_CONFIG, and FundingSourceConfig can all be removed. I verified this locally - zero Flow errors and the template only hits the <MarkLogo> branch for "pp" since Mark is defined, so Logo is never called.
| return inlineMemoize(getMarksFundingConfig, () => { | ||
| return { | ||
| ...getFundingConfig(), | ||
| pp: getPPConfig(), |
There was a problem hiding this comment.
"pp" is used as a raw string in 4 files (config.js, funding.js, templateRebrand.jsx, component.jsx). Every other funding source uses FUNDING.* constants from @paypal/sdk-constants but for this unique use case we don't need to add it there.
Add a constant to src/constants/button.js:
// Alternative mark for the PayPal funding source (monogram icon).
// Not a real funding source - only used in rebrand marks rendering.
export const MARKS_FUNDING = {
PP: ("pp": "pp"),
};It's already re-exported via src/constants/index.js (export * from "./button"), so all consumers can import it:
import { MARKS_FUNDING } from "../constants";Then replace all raw "pp" references with MARKS_FUNDING.PP.
| function Mark(): ChildType { | ||
| return __WEB__ ? ( | ||
| // $FlowFixMe - JSX pragma compatibility | ||
| <PPRebrandLogoExternalImage logoColor={LOGO_COLOR.BLUE} /> |
There was a problem hiding this comment.
I looks like other Marks only use the External image and don't have the logic to swap out on first and second render.
lets remove the logic and only return the <PPRebrandLogoExternalImage logoColor={LOGO_COLOR.BLUE} /> like the credit mark is
Lets remove the flow fix me here as well. These doesn't seem to be an issue related to his PR and all other logos are having this issue and the build still passed
The buttons redesign requires a new PayPal monogram mark in paypal-checkout-components (PCC). Currently, marks are selected based on the funding source property.
https://paypal.atlassian.net/browse/DTPPCPSDK-6244