Skip to content

Commit 3885c91

Browse files
committed
CMS-46481 Enhance dual-purpose attribute handling in toReactProps function
1 parent 495f4ef commit 3885c91

File tree

2 files changed

+77
-8
lines changed

2 files changed

+77
-8
lines changed
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
import React from 'react';
2+
import { describe, it, expect } from 'vitest';
3+
import '@testing-library/jest-dom';
4+
5+
import { toReactProps } from '../richText/lib.js';
6+
7+
describe('Dual-purpose attribute handling', () => {
8+
it('should handle border as HTML attribute for table elements', () => {
9+
const tableProps = toReactProps({ border: '1' }, 'table');
10+
expect(tableProps.border).toBe('1');
11+
expect(tableProps.style).toBeUndefined();
12+
});
13+
14+
it('should handle border as CSS property for non-table elements', () => {
15+
const divProps = toReactProps({ border: '1px solid black' }, 'div');
16+
expect(divProps.border).toBeUndefined();
17+
expect((divProps.style as any)?.border).toBe('1px solid black');
18+
});
19+
20+
it('should handle width as HTML attribute for img elements', () => {
21+
const imgProps = toReactProps({ width: '100' }, 'img');
22+
expect(imgProps.width).toBe('100');
23+
expect(imgProps.style).toBeUndefined();
24+
});
25+
26+
it('should handle width as CSS property for div elements', () => {
27+
const divProps = toReactProps({ width: '100px' }, 'div');
28+
expect(divProps.width).toBeUndefined();
29+
expect((divProps.style as any)?.width).toBe('100px');
30+
});
31+
32+
it('should handle height as HTML attribute for canvas elements', () => {
33+
const canvasProps = toReactProps({ height: '200' }, 'canvas');
34+
expect(canvasProps.height).toBe('200');
35+
expect(canvasProps.style).toBeUndefined();
36+
});
37+
38+
it('should handle height as CSS property for paragraph elements', () => {
39+
const pProps = toReactProps({ height: '50px' }, 'p');
40+
expect(pProps.height).toBeUndefined();
41+
expect((pProps.style as any)?.height).toBe('50px');
42+
});
43+
});

packages/optimizely-cms-sdk/src/react/richText/lib.ts

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -398,18 +398,44 @@ function kebabToCamelCase(str: string): string {
398398
return str.replace(/-([a-z])/g, (_, letter) => letter.toUpperCase());
399399
}
400400

401+
/**
402+
* Properties that can be either HTML attributes or CSS properties depending on context
403+
*/
404+
const DUAL_PURPOSE_PROPERTIES = new Set(['border', 'width', 'height']);
405+
406+
/**
407+
* Element types that should treat dual-purpose properties as HTML attributes
408+
*/
409+
const HTML_ATTRIBUTE_ELEMENTS = new Set(['table', 'img', 'input', 'canvas']);
410+
401411
/**
402412
* Converts framework-agnostic attributes to React props
403413
* Handles HTML attribute to React JSX attribute conversion and CSS properties
404414
*/
405-
function toReactProps(
406-
attributes: Record<string, unknown>
415+
export function toReactProps(
416+
attributes: Record<string, unknown>,
417+
elementType?: string
407418
): Record<string, unknown> {
408419
const reactProps: Record<string, unknown> = {};
409420
const styleProps: Record<string, string> = {};
410421

411422
for (const [key, value] of Object.entries(attributes)) {
412-
// Handle CSS properties - move them to style object
423+
// Handle dual-purpose properties based on element context
424+
if (DUAL_PURPOSE_PROPERTIES.has(key.toLowerCase())) {
425+
if (elementType && HTML_ATTRIBUTE_ELEMENTS.has(elementType)) {
426+
// Treat as HTML attribute for specific elements
427+
const reactKey = HTML_TO_REACT_ATTRS[key.toLowerCase()] || key;
428+
reactProps[reactKey] = value;
429+
continue;
430+
} else {
431+
// Treat as CSS property for other elements
432+
const camelKey = kebabToCamelCase(key);
433+
styleProps[camelKey] = String(value);
434+
continue;
435+
}
436+
}
437+
438+
// Handle other CSS properties - move them to style object
413439
if (CSS_PROPERTIES.has(key.toLowerCase())) {
414440
const camelKey = kebabToCamelCase(key);
415441
styleProps[camelKey] = String(value);
@@ -480,8 +506,8 @@ export function createHtmlComponent<T extends keyof JSX.IntrinsicElements>(
480506
config: HtmlComponentConfig = {}
481507
): ElementRenderer {
482508
const Component: ElementRenderer = ({ children, attributes, element }) => {
483-
// Convert to React props and merge with config
484-
const reactProps = toReactProps(attributes || {});
509+
// Convert to React props and merge with config, passing element type for context
510+
const reactProps = toReactProps(attributes || {}, tag as string);
485511
const mergedProps = {
486512
...reactProps,
487513
...config.attributes,
@@ -515,7 +541,7 @@ export function createLinkComponent<T extends keyof JSX.IntrinsicElements>(
515541
element,
516542
}) => {
517543
// Convert to React props and merge with config
518-
const reactProps = toReactProps(attributes || {});
544+
const reactProps = toReactProps(attributes || {}, tag as string);
519545

520546
// Type-safe access to link properties
521547
const linkProps = {
@@ -554,7 +580,7 @@ export function createImageComponent<T extends keyof JSX.IntrinsicElements>(
554580
element,
555581
}) => {
556582
// Convert to React props and merge with config
557-
const reactProps = toReactProps(attributes || {});
583+
const reactProps = toReactProps(attributes || {}, tag as string);
558584

559585
// Type-safe access to image properties
560586
const imageProps = {
@@ -596,7 +622,7 @@ export function createTableComponent<T extends keyof JSX.IntrinsicElements>(
596622
element,
597623
}) => {
598624
// Convert to React props and merge with config
599-
const reactProps = toReactProps(attributes || {});
625+
const reactProps = toReactProps(attributes || {}, tag as string);
600626

601627
const mergedProps = {
602628
...reactProps,

0 commit comments

Comments
 (0)