-
Notifications
You must be signed in to change notification settings - Fork 80
feat: add flex layout support to renderV2Message (DTCRCMERC-5374) #1367
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
9602ae4
9b3c120
c06fbe4
c57384f
a683159
4cd68a4
3b4d5ca
54b82ce
901d205
07df651
cbeb2f9
77046c1
c66135d
72e7634
73cac51
32d5840
5dfcb44
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,20 @@ | ||
| export const VARIANT = 'B'; | ||
| export const PORT = process.env.PORT || 8080; | ||
|
|
||
| export const FLEX_DEFAULTS = { | ||
| color: 'blue', | ||
| ratio: '1x1' | ||
| }; | ||
|
|
||
| // Maps flex style.color to logo asset variant keys (see flexLogoMutations + v5 flex defaults). | ||
| export const FLEX_COLOR_TO_LOGO_TEXT_COLOR = { | ||
| blue: 'white', | ||
| black: 'white', | ||
| white: 'black', | ||
| 'white-no-border': 'black', | ||
| gray: 'black', | ||
| grey: 'black', | ||
| monochrome: 'monochrome', | ||
| grayscale: 'grayscale', | ||
| greyscale: 'grayscale' | ||
| }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,107 @@ | ||
| /** @jsx h */ | ||
| /** @jsxFrag Fragment */ | ||
| import { h, Fragment } from 'preact'; | ||
|
|
||
| import { buildContentLabel } from './utils/buildContentLabel'; | ||
| import { renderBlock } from './utils/renderBlock'; | ||
| import { getLogoBrandClass, resolveLogoAssets } from './logos'; | ||
| import flexStyles from './flexStyles'; | ||
| import { FLEX_COLOR_TO_LOGO_TEXT_COLOR, FLEX_DEFAULTS } from './constants'; | ||
|
|
||
| function renderFlexLogo(logoBlock, flexColor) { | ||
| const textColor = FLEX_COLOR_TO_LOGO_TEXT_COLOR[flexColor] ?? 'white'; | ||
| const brandClass = getLogoBrandClass({ | ||
| logoName: logoBlock.name, | ||
| alternativeText: logoBlock.alternative_text | ||
| }); | ||
| const logoClassName = ['pp-flex__logo', brandClass].filter(Boolean).join(' '); | ||
| const assets = resolveLogoAssets({ | ||
| logoName: logoBlock.name, | ||
| effectiveLogoType: 'wordmark', | ||
| effectiveLogoPosition: 'left', | ||
| textColor | ||
| }); | ||
|
|
||
| if (assets) { | ||
| return assets.map(({ src, dimensions: [width, height] }, idx) => ( | ||
| // eslint-disable-next-line react/no-array-index-key | ||
| <span key={idx} className={logoClassName}> | ||
| <img src={src} alt="" role="presentation" width={width} height={height} /> | ||
| </span> | ||
| )); | ||
| } | ||
|
|
||
| return ( | ||
| <span className={['pp-flex__logo', 'pp-flex__logo--fallback', brandClass].filter(Boolean).join(' ')}> | ||
| {renderBlock(logoBlock)} | ||
| </span> | ||
| ); | ||
| } | ||
|
|
||
| export default function FlexMessage({ style, v2Content }) { | ||
| const color = style.color ?? FLEX_DEFAULTS.color; | ||
| const ratio = style.ratio ?? FLEX_DEFAULTS.ratio; | ||
|
|
||
| const mainItems = v2Content?.main_items ?? []; | ||
| const actionItems = v2Content?.action_items ?? []; | ||
| const disclaimerItems = v2Content?.disclaimer_items ?? []; | ||
|
|
||
| const logoBlock = mainItems.find(item => item.type === 'IMAGE'); | ||
| const mainBlocks = mainItems.filter(item => item.type !== 'IMAGE'); | ||
|
|
||
| const mainLabel = buildContentLabel(logoBlock ? [logoBlock, ...mainBlocks] : mainBlocks); | ||
| const actionLabel = buildContentLabel(actionItems); | ||
|
|
||
| return ( | ||
| <div | ||
| className={`pp-message pp-flex ${color} r-${ratio}`} | ||
| data-pp-style-layout="flex" | ||
| data-pp-style-color={color} | ||
| data-pp-style-ratio={ratio} | ||
| > | ||
| {/* eslint-disable react/no-danger */} | ||
| <style | ||
| dangerouslySetInnerHTML={{ | ||
| __html: flexStyles({ | ||
| fontFamily: style.text?.fontFamily, | ||
| fontSource: style.text?.fontSource, | ||
| ratio | ||
| }) | ||
| }} | ||
| /> | ||
| {/* eslint-enable react/no-danger */} | ||
| <div className="pp-flex__background" /> | ||
| <div className="pp-flex__content"> | ||
| {logoBlock ? ( | ||
| <div className="pp-flex__logo-container" aria-hidden="true"> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Finding Description[P1] Keep the logo alternative text accessible
Problematic Code Snippet(s)const mainLabel = buildContentLabel(mainBlocks);
<div className="pp-flex__logo-container" aria-hidden="true">
{renderFlexLogo(logoBlock, color)}
</div>Suggested Change(s)
Relevant Links
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 72e7634 Fixed here, nice catch |
||
| {renderFlexLogo(logoBlock, color)} | ||
| </div> | ||
| ) : null} | ||
| <div className="pp-flex__messaging"> | ||
| <div aria-label={mainLabel} className="pp-flex__main"> | ||
| {mainBlocks.map((item, idx) => ( | ||
| // eslint-disable-next-line react/no-array-index-key | ||
| <Fragment key={idx}>{renderBlock(item)}</Fragment> | ||
| ))} | ||
| </div> | ||
| {actionItems.length > 0 ? ( | ||
| <div aria-label={actionLabel} className="pp-flex__action"> | ||
| {actionItems.map((item, idx) => ( | ||
| // eslint-disable-next-line react/no-array-index-key | ||
| <Fragment key={idx}>{renderBlock(item)}</Fragment> | ||
| ))} | ||
| </div> | ||
| ) : null} | ||
| {disclaimerItems.length > 0 ? ( | ||
| <div className="pp-flex__disclaimer"> | ||
| {disclaimerItems.map((item, idx) => ( | ||
| // eslint-disable-next-line react/no-array-index-key | ||
| <Fragment key={idx}>{renderBlock(item)}</Fragment> | ||
| ))} | ||
| </div> | ||
| ) : null} | ||
| </div> | ||
| </div> | ||
| </div> | ||
| ); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Finding Description
[P1] Preserve ordered image blocks
The flex renderer finds one image, filters every image out of
main_items, and renders the selected image before the messaging container. A validTEXT, IMAGE, TEXTsequence therefore becomesIMAGE, TEXT, TEXT, and any second image is removed completely. This breaks the ordered typed-block contract instead of rendering CPS content directly.Problematic Code Snippet(s)
Suggested Change(s)
TEXT, IMAGE, TEXTand define explicit behavior for multiple images so no valid block is silently discarded.Relevant Links
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The find/filter behavior is intentional flex presentation, not a contract violation. Flex (like v5 and text left/top/right) extracts the brand logo into a dedicated logo container and renders the remaining blocks as messaging. Order-preserving in-stream
IMAGErendering is the inline path (buildLogoConfiguration + text layout), which has an explicitTEXT,IMAGE,TEXTtest. Flex has no logo.position option, and CPNW does not requestINLINE_LOGOfor flex - so the mid-streamTEXT,IMAGE,TEXTshape the finding assumes is not the flex content contract.CPS samples for this path use a single logo
IMAGE. Dropping additionalIMAGEblocks is theoretical vs real flex traffic, not evidence that flex must render an ordered multi-image stream.