From d1c36b27f2f9f87acce5647b47ce373b2a1fb8f6 Mon Sep 17 00:00:00 2001 From: purnimagarg1 <139125209+purnimagarg1@users.noreply.github.com> Date: Wed, 27 Aug 2025 22:02:24 +0530 Subject: [PATCH] improvement(ui): save underline and font-size from rich text editor and improve styles (#14525) Co-authored-by: Chris Collins --- .../components/Editor/__tests__/utils.test.ts | 239 ++++++++++++++++++ .../Editor/extensions/htmlToMarkdown.tsx | 33 +++ .../Editor/toolbar/FontSizeSelect.tsx | 16 +- .../components/Editor/toolbar/Toolbar.tsx | 28 +- .../components/Editor/utils.ts | 8 + 5 files changed, 308 insertions(+), 16 deletions(-) create mode 100644 datahub-web-react/src/alchemy-components/components/Editor/__tests__/utils.test.ts diff --git a/datahub-web-react/src/alchemy-components/components/Editor/__tests__/utils.test.ts b/datahub-web-react/src/alchemy-components/components/Editor/__tests__/utils.test.ts new file mode 100644 index 0000000000..8af4598a6f --- /dev/null +++ b/datahub-web-react/src/alchemy-components/components/Editor/__tests__/utils.test.ts @@ -0,0 +1,239 @@ +import DOMPurify from 'dompurify'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { ptToPx, pxToPt, sanitizeRichText } from '@components/components/Editor/utils'; + +// Mock DOMPurify +vi.mock('dompurify', () => ({ + default: { + sanitize: vi.fn(), + }, +})); + +const mockDOMPurify = DOMPurify as any as { sanitize: ReturnType }; + +describe('Editor Utils', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + describe('sanitizeRichText', () => { + it('should return empty string for falsy content', () => { + expect(sanitizeRichText('')).toBe(''); + expect(sanitizeRichText(null as any)).toBe(''); + expect(sanitizeRichText(undefined as any)).toBe(''); + }); + + it('should encode and decode HTML entities correctly', () => { + const input = ''; + const encodedInput = '<script>alert("test")</script>'; + + // Mock DOMPurify to return the encoded input (simulating sanitization) + mockDOMPurify.sanitize.mockReturnValue(encodedInput); + + const result = sanitizeRichText(input); + + // Verify DOMPurify was called with encoded input + expect(mockDOMPurify.sanitize).toHaveBeenCalledWith(encodedInput); + + // Should decode back to original after sanitization + expect(result).toBe(''); + }); + + it('should handle mixed HTML content', () => { + const input = '
Hello world
'; + const encodedInput = '<div>Hello <span>world</span></div>'; + + mockDOMPurify.sanitize.mockReturnValue(encodedInput); + + const result = sanitizeRichText(input); + + expect(mockDOMPurify.sanitize).toHaveBeenCalledWith(encodedInput); + expect(result).toBe('
Hello world
'); + }); + + it('should handle content with only < characters', () => { + const input = 'Price < 100'; + const encodedInput = 'Price < 100'; + + mockDOMPurify.sanitize.mockReturnValue(encodedInput); + + const result = sanitizeRichText(input); + + expect(result).toBe('Price < 100'); + }); + + it('should handle content with only > characters', () => { + const input = 'Score > 50'; + const encodedInput = 'Score > 50'; + + mockDOMPurify.sanitize.mockReturnValue(encodedInput); + + const result = sanitizeRichText(input); + + expect(result).toBe('Score > 50'); + }); + + it('should handle content with no HTML characters', () => { + const input = 'Hello world'; + + mockDOMPurify.sanitize.mockReturnValue(input); + + const result = sanitizeRichText(input); + + expect(mockDOMPurify.sanitize).toHaveBeenCalledWith(input); + expect(result).toBe('Hello world'); + }); + + it('should handle complex nested HTML with attributes', () => { + const input = '
Content bold
'; + + // Simulate DOMPurify removing malicious attributes but keeping safe ones + const sanitizedEncoded = '<div class="test">Content <b>bold</b></div>'; + mockDOMPurify.sanitize.mockReturnValue(sanitizedEncoded); + + const result = sanitizeRichText(input); + + expect(result).toBe('
Content bold
'); + }); + + it('should handle already encoded entities', () => { + const input = '<script>'; + const encodedInput = '&lt;script&gt;'; + + mockDOMPurify.sanitize.mockReturnValue(encodedInput); + + const result = sanitizeRichText(input); + + // Should handle double encoding properly + expect(result).toBe('&lt;script&gt;'); + }); + }); + + describe('ptToPx', () => { + it('should convert points to pixels correctly', () => { + // Standard conversion: pt * 96 / 72 + expect(ptToPx(72)).toBe(96); // 72pt = 96px (1 inch) + expect(ptToPx(36)).toBe(48); // 36pt = 48px (0.5 inch) + expect(ptToPx(144)).toBe(192); // 144pt = 192px (2 inches) + }); + + it('should handle decimal points', () => { + expect(ptToPx(12.5)).toBe(17); // Math.round((12.5 * 96) / 72) = 17 + expect(ptToPx(10.75)).toBe(14); // Math.round((10.75 * 96) / 72) = 14 + }); + + it('should handle zero', () => { + expect(ptToPx(0)).toBe(0); + }); + + it('should handle negative values', () => { + expect(ptToPx(-12)).toBe(-16); // Math.round((-12 * 96) / 72) = -16 + expect(ptToPx(-36)).toBe(-48); + }); + + it('should round to nearest integer', () => { + expect(ptToPx(1)).toBe(1); // Math.round((1 * 96) / 72) = 1.33... rounds to 1 + expect(ptToPx(2)).toBe(3); // Math.round((2 * 96) / 72) = 2.67... rounds to 3 + }); + + it('should handle very small values', () => { + expect(ptToPx(0.1)).toBe(0); // Math.round((0.1 * 96) / 72) = 0.13... rounds to 0 + expect(ptToPx(0.5)).toBe(1); // Math.round((0.5 * 96) / 72) = 0.67... rounds to 1 + }); + + it('should handle large values', () => { + expect(ptToPx(1000)).toBe(1333); // Math.round((1000 * 96) / 72) = 1333 + expect(ptToPx(7200)).toBe(9600); // Math.round((7200 * 96) / 72) = 9600 + }); + }); + + describe('pxToPt', () => { + it('should convert pixels to points correctly', () => { + // Standard conversion: px * 72 / 96 + expect(pxToPt(96)).toBe(72); // 96px = 72pt (1 inch) + expect(pxToPt(48)).toBe(36); // 48px = 36pt (0.5 inch) + expect(pxToPt(192)).toBe(144); // 192px = 144pt (2 inches) + }); + + it('should handle decimal pixels', () => { + expect(pxToPt(16.5)).toBe(12); // Math.round((16.5 * 72) / 96) = 12.375 rounds to 12 + expect(pxToPt(14.25)).toBe(11); // Math.round((14.25 * 72) / 96) = 10.6875 rounds to 11 + }); + + it('should handle zero', () => { + expect(pxToPt(0)).toBe(0); + }); + + it('should handle negative values', () => { + expect(pxToPt(-16)).toBe(-12); // Math.round((-16 * 72) / 96) = -12 + expect(pxToPt(-48)).toBe(-36); + }); + + it('should round to nearest integer', () => { + expect(pxToPt(1)).toBe(1); // Math.round((1 * 72) / 96) = 0.75 rounds to 1 + expect(pxToPt(2)).toBe(2); // Math.round((2 * 72) / 96) = 1.5 rounds to 2 + }); + + it('should handle very small values', () => { + expect(pxToPt(0.1)).toBe(0); // Math.round((0.1 * 72) / 96) = 0.075 rounds to 0 + expect(pxToPt(0.8)).toBe(1); // Math.round((0.8 * 72) / 96) = 0.6 rounds to 1 + }); + + it('should handle large values', () => { + expect(pxToPt(1333)).toBe(1000); // Math.round((1333 * 72) / 96) = 999.75 rounds to 1000 + expect(pxToPt(9600)).toBe(7200); // Math.round((9600 * 72) / 96) = 7200 + }); + }); + + describe('Round-trip conversions', () => { + it('should handle round-trip pt->px->pt conversions', () => { + const originalPt = 12; + const px = ptToPx(originalPt); + const backToPt = pxToPt(px); + + // Should be close due to rounding, but may not be exact + expect(Math.abs(backToPt - originalPt)).toBeLessThanOrEqual(1); + }); + + it('should handle round-trip px->pt->px conversions', () => { + const originalPx = 16; + const pt = pxToPt(originalPx); + const backToPx = ptToPx(pt); + + // Should be close due to rounding, but may not be exact + expect(Math.abs(backToPx - originalPx)).toBeLessThanOrEqual(1); + }); + + it('should maintain perfect round-trip for standard DPI values', () => { + // 72pt = 96px exactly, so round trip should be perfect + expect(pxToPt(ptToPx(72))).toBe(72); + expect(ptToPx(pxToPt(96))).toBe(96); + + // 36pt = 48px exactly + expect(pxToPt(ptToPx(36))).toBe(36); + expect(ptToPx(pxToPt(48))).toBe(48); + }); + }); + + describe('Edge cases and integration', () => { + it('should handle type conversion edge cases', () => { + // Test with numbers that could cause floating point precision issues + expect(ptToPx(12.3456789)).toBe(16); // Should round properly + expect(pxToPt(16.7890123)).toBe(13); // Should round properly + }); + + it('should maintain consistent behavior with mathematical operations', () => { + const pt1 = 10; + const pt2 = 20; + const px1 = ptToPx(pt1); + const px2 = ptToPx(pt2); + + // Addition should be approximately preserved + const sumPt = pt1 + pt2; + const sumPx = px1 + px2; + + expect(Math.abs(ptToPx(sumPt) - sumPx)).toBeLessThanOrEqual(1); + }); + }); +}); diff --git a/datahub-web-react/src/alchemy-components/components/Editor/extensions/htmlToMarkdown.tsx b/datahub-web-react/src/alchemy-components/components/Editor/extensions/htmlToMarkdown.tsx index 86022a43da..18f7c86cd2 100644 --- a/datahub-web-react/src/alchemy-components/components/Editor/extensions/htmlToMarkdown.tsx +++ b/datahub-web-react/src/alchemy-components/components/Editor/extensions/htmlToMarkdown.tsx @@ -3,6 +3,7 @@ import _TurndownService from 'turndown'; import { gfm } from 'turndown-plugin-gfm'; import { DATAHUB_MENTION_ATTRS } from '@components/components/Editor/extensions/mentions/DataHubMentionsExtension'; +import { ptToPx } from '@components/components/Editor/utils'; const TurndownService = defaultImport(_TurndownService); @@ -105,6 +106,38 @@ const turndownService = new TurndownService({ return `[${node.textContent}](${urn})`; }, + }) + /* Add support for underline */ + .addRule('underline', { + filter: (node) => { + const nodeName = node.nodeName?.toUpperCase(); + return ( + nodeName === 'U' || + (nodeName === 'SPAN' && + node instanceof HTMLElement && + typeof node.style.textDecoration === 'string' && + node.style.textDecoration.toLowerCase().includes('underline')) + ); + }, + replacement: (content) => `${content}`, + }) + /* Add support for handling font size change */ + .addRule('fontSize', { + filter: (node) => + node instanceof HTMLElement && node.nodeName?.toUpperCase() === 'SPAN' && !!node.style.fontSize, + replacement: (content, node) => { + const elem = node as HTMLElement; + let size = elem.style.fontSize.trim(); + if (!size) return content; + + // Convert pt to px + if (size.endsWith('pt')) { + const pts = parseFloat(size); + size = `${ptToPx(pts)}px`; + } + + return `${content}`; + }, }); /** diff --git a/datahub-web-react/src/alchemy-components/components/Editor/toolbar/FontSizeSelect.tsx b/datahub-web-react/src/alchemy-components/components/Editor/toolbar/FontSizeSelect.tsx index fc2390dfc3..74a53573e5 100644 --- a/datahub-web-react/src/alchemy-components/components/Editor/toolbar/FontSizeSelect.tsx +++ b/datahub-web-react/src/alchemy-components/components/Editor/toolbar/FontSizeSelect.tsx @@ -1,15 +1,25 @@ -import { useActive, useCommands } from '@remirror/react'; +import { useCommands, useHelpers } from '@remirror/react'; import React from 'react'; +import { ptToPx } from '@components/components/Editor/utils'; import { SimpleSelect } from '@components/components/Select'; const FONT_SIZES = ['12px', '14px', '16px', '18px', '24px', '32px']; export const FontSizeSelect = () => { const commands = useCommands(); - const active = useActive(); - const currentSize = FONT_SIZES.find((size) => active.fontSize({ size })) || '14px'; + const getFontSize = useHelpers().getFontSizeForSelection; + + const currentSize = (() => { + const sizeTuple = getFontSize()?.[0]; + if (!sizeTuple) return '14px'; + const [value, unit] = sizeTuple; + if (value === 0) return '14px'; + if (unit === 'px') return `${value}px`; + if (unit === 'pt') return `${ptToPx(value)}px`; + return `${value}${unit}`; + })(); const options = FONT_SIZES.map((size) => ({ value: size, diff --git a/datahub-web-react/src/alchemy-components/components/Editor/toolbar/Toolbar.tsx b/datahub-web-react/src/alchemy-components/components/Editor/toolbar/Toolbar.tsx index 188c9f21cd..3c4cb545ec 100644 --- a/datahub-web-react/src/alchemy-components/components/Editor/toolbar/Toolbar.tsx +++ b/datahub-web-react/src/alchemy-components/components/Editor/toolbar/Toolbar.tsx @@ -37,11 +37,13 @@ const Container = styled.div` justify-content: space-between; align-items: center; box-shadow: 0 4px 6px -4px rgba(0, 0, 0, 0.1); + gap: 8px; + width: fit-content; `; const CustomDivider = styled(Divider)` height: 100%; - margin: 0 6px; + margin: 0; `; interface Props { @@ -58,63 +60,63 @@ export const Toolbar = ({ styles }: Props) => { } + icon={} style={{ marginRight: 2 }} commandName="toggleBold" active={active.bold()} onClick={() => commands.toggleBold()} /> } + icon={} style={{ marginRight: 2 }} commandName="toggleItalic" active={active.italic()} onClick={() => commands.toggleItalic()} /> } + icon={} style={{ marginRight: 2 }} commandName="toggleUnderline" active={active.underline()} onClick={() => commands.toggleUnderline()} /> } + icon={} commandName="toggleStrike" active={active.strike()} onClick={() => commands.toggleStrike()} /> - + } + icon={} commandName="toggleBulletList" active={active.bulletList()} onClick={() => commands.toggleBulletList()} /> } + icon={} commandName="toggleOrderedList" active={active.orderedList()} onClick={() => commands.toggleOrderedList()} /> - + } + icon={} commandName="toggleCode" active={active.code()} onClick={() => commands.toggleCode()} /> } + icon={} commandName="toggleCodeBlock" active={active.codeBlock()} onClick={() => commands.toggleCodeBlock()} /> - + } + icon={} commandName="createTable" onClick={() => commands.createTable()} disabled={active.table()} /* Disables nested tables */ diff --git a/datahub-web-react/src/alchemy-components/components/Editor/utils.ts b/datahub-web-react/src/alchemy-components/components/Editor/utils.ts index 8bbc0a9222..9fdcb0496a 100644 --- a/datahub-web-react/src/alchemy-components/components/Editor/utils.ts +++ b/datahub-web-react/src/alchemy-components/components/Editor/utils.ts @@ -5,3 +5,11 @@ export const sanitizeRichText = (content: string) => { const encoded = content.replace(//g, '>'); return DOMPurify.sanitize(encoded).replace(/</g, '<').replace(/>/g, '>'); }; + +export function ptToPx(pt: number): number { + return Math.round((pt * 96) / 72); +} + +export function pxToPt(px: number): number { + return Math.round((px * 72) / 96); +}