Skip to content
Snippets Groups Projects
Unverified Commit ad867612 authored by Kevin Aleman's avatar Kevin Aleman Committed by GitHub
Browse files

fix: Set conditional wrapping for big messages on PDF transcript's react template (#32311)

parent 3b3275f6
No related branches found
No related tags found
No related merge requests found
Showing
with 865 additions and 52 deletions
---
"@rocket.chat/meteor": patch
"@rocket.chat/core-services": patch
"@rocket.chat/omnichannel-services": patch
"@rocket.chat/pdf-worker": patch
---
Fixed multiple issues with PDF generation logic when a quoted message was too big to fit in one single page. This was causing an internal infinite loop within the library (as it tried to make it fit, failing and then trying to fit on next page where the same happened thus causing a loop).
The library was not able to break down some nested views and thus was trying to fit the whole quote on one single page. Logic was updated to allow wrapping of the contents when messages are quoted (so they can span multiple lines) and removed a bunch of unnecesary views from the code.
......@@ -2,7 +2,7 @@ export default {
preset: 'ts-jest',
errorOnDeprecated: true,
testEnvironment: 'jsdom',
modulePathIgnorePatterns: ['<rootDir>/dist/'],
modulePathIgnorePatterns: ['<rootDir>/dist/', '<rootDir>/src/worker.spec.ts'],
moduleNameMapper: {
'\\.css$': 'identity-obj-proxy',
},
......
export default {
preset: 'ts-jest',
errorOnDeprecated: true,
modulePathIgnorePatterns: ['<rootDir>/dist/', '<rootDir>/src/strategies/', '<rootDir>/src/templates/'],
};
......@@ -22,6 +22,8 @@
"lint": "eslint --ext .js,.jsx,.ts,.tsx .",
"lint:fix": "eslint --ext .js,.jsx,.ts,.tsx . --fix",
"test": "jest",
"test:worker": "jest --config ./jest.worker.config.ts",
"testunit": "yarn run test && yarn run test:worker",
"build": "rm -rf dist && tsc -p tsconfig.json && cp -r src/public dist/public",
"dev": "tsc -p tsconfig.json --watch --preserveWatchOutput",
"storybook": "start-storybook -p 6006"
......
import moment from 'moment';
import moment from 'moment-timezone';
import '@testing-library/jest-dom';
import { invalidData, validData, newDayData, sameDayData, translationsData } from '../templates/ChatTranscript/ChatTranscript.fixtures';
......@@ -31,7 +31,10 @@ describe('Strategies/ChatTranscript', () => {
it('should creates a divider if message is from a new day', () => {
const result = chatTranscript.parseTemplateData(newDayData);
expect(result.messages[0]).toHaveProperty('divider');
expect(result.messages[1]).toHaveProperty('divider', moment(newDayData.messages[1].ts).format(newDayData.dateFormat));
expect(result.messages[1]).toHaveProperty(
'divider',
moment(newDayData.messages[1].ts).tz(newDayData.timezone).format(newDayData.dateFormat),
);
});
it('should not create a divider if message is from the same day', () => {
......
......@@ -31,6 +31,7 @@ export const newDayData = {
closedAt: '2022-11-21T00:00:00.000Z',
dateFormat: 'MMM D, YYYY',
timeAndDateFormat: 'MMM D, YYYY H:mm:ss',
timezone: 'UTC',
messages: [
{ ts: '2022-11-21T16:00:00.000Z', text: 'Hello' },
{ ts: '2022-11-22T16:00:00.000Z', text: 'How are you' },
......@@ -41,6 +42,7 @@ export const sameDayData = {
closedAt: '2022-11-21T00:00:00.000Z',
dateFormat: 'MMM D, YYYY',
timeAndDateFormat: 'MMM D, YYYY H:mm:ss',
timezone: 'UTC',
messages: [
{ ts: '2022-11-21T16:00:00.000Z', text: 'Hello' },
{ ts: '2022-11-21T16:00:00.000Z', text: 'How are you' },
......
......@@ -29,7 +29,7 @@ const styles = StyleSheet.create({
});
export const Files = ({ files, invalidMessage }: { files: PDFFile[]; invalidMessage: string }) => (
<View>
<View wrap={false}>
{files?.map((file, index) => (
<View style={styles.file} key={index}>
<Text>{file.name}</Text>
......
......@@ -11,6 +11,7 @@ import { Quotes } from './Quotes';
const styles = StyleSheet.create({
wrapper: {
marginBottom: 16,
paddingBottom: 16,
paddingHorizontal: 32,
},
message: {
......@@ -19,16 +20,21 @@ const styles = StyleSheet.create({
},
});
const messageLongerThanPage = (message: string) => message.length > 1200;
export const MessageList = ({ messages, invalidFileMessage }: { messages: ChatTranscriptData['messages']; invalidFileMessage: string }) => (
<View>
<>
{messages.map((message, index) => (
<View style={styles.wrapper} key={index} wrap={false}>
{message.divider && <Divider divider={message.divider} />}
<MessageHeader name={message.u.name || message.u.username} time={message.ts} />
<View style={styles.message}>{message.md ? <Markup tokens={message.md} /> : <Text>{message.msg}</Text>}</View>
{message.quotes && <Quotes quotes={message.quotes} />}
<View key={index} style={styles.wrapper}>
<View wrap={!!message.quotes || messageLongerThanPage(message.msg)}>
{message.divider && <Divider divider={message.divider} />}
<MessageHeader name={message.u.name || message.u.username} time={message.ts} />
<View style={styles.message}>{message.md ? <Markup tokens={message.md} /> : <Text>{message.msg}</Text>}</View>
{message.quotes && <Quotes quotes={message.quotes} />}
</View>
{message.files && <Files files={message.files} invalidMessage={invalidFileMessage} />}
</View>
))}
</View>
</>
);
......@@ -12,12 +12,13 @@ const styles = StyleSheet.create({
borderWidth: 1,
borderColor: colors.n250,
borderLeftColor: colors.n600,
padding: 16,
borderTopWidth: 1,
borderBottomWidth: 1,
paddingLeft: 16,
paddingRight: 16,
},
quoteMessage: {
marginTop: 6,
paddingTop: 6,
paddingBottom: 6,
fontSize: fontScales.p2.fontSize,
},
});
......@@ -29,11 +30,9 @@ const Quote = ({ quote, children, index }: { quote: QuoteType; children: JSX.Ele
marginTop: !index ? 4 : 16,
}}
>
<View>
<MessageHeader name={quote.name} time={quote.ts} light />
<View style={styles.quoteMessage}>
<Markup tokens={quote.md} />
</View>
<MessageHeader name={quote.name} time={quote.ts} light />
<View style={styles.quoteMessage}>
<Markup tokens={quote.md} />
</View>
{children}
......
......@@ -35,6 +35,8 @@ const styles = StyleSheet.create({
fontFamily: 'Inter',
lineHeight: 1.25,
color: colors.n800,
// ugh https://github.com/diegomura/react-pdf/issues/684
paddingBottom: 32,
},
wrapper: {
paddingHorizontal: 32,
......
import { View } from '@react-pdf/renderer';
import type * as MessageParser from '@rocket.chat/message-parser';
import InlineElements from '../elements/InlineElements';
......@@ -7,10 +6,6 @@ type ParagraphBlockProps = {
items: MessageParser.Inlines[];
};
const ParagraphBlock = ({ items }: ParagraphBlockProps) => (
<View>
<InlineElements children={items} />
</View>
);
const ParagraphBlock = ({ items }: ParagraphBlockProps) => <InlineElements children={items} />;
export default ParagraphBlock;
import { View } from '@react-pdf/renderer';
import type * as MessageParser from '@rocket.chat/message-parser';
import BigEmojiBlock from './blocks/BigEmojiBlock';
......@@ -13,30 +12,32 @@ type MarkupProps = {
};
export const Markup = ({ tokens }: MarkupProps) => (
<View>
{tokens.map((child, index) => {
switch (child.type) {
case 'PARAGRAPH':
return <ParagraphBlock key={index} items={child.value} />;
case 'HEADING':
return <HeadingBlock key={index} level={child.level} items={child.value} />;
case 'UNORDERED_LIST':
return <UnorderedListBlock key={index} items={child.value} />;
case 'ORDERED_LIST':
return <OrderedListBlock key={index} items={child.value} />;
case 'BIG_EMOJI':
return <BigEmojiBlock key={index} emoji={child.value} />;
case 'CODE':
return <CodeBlock key={index} lines={child.value} />;
default:
return null;
}
})}
</View>
<>
{tokens
.map((child, index) => {
switch (child.type) {
case 'PARAGRAPH':
return <ParagraphBlock key={index} items={child.value} />;
case 'HEADING':
return <HeadingBlock key={index} level={child.level} items={child.value} />;
case 'UNORDERED_LIST':
return <UnorderedListBlock key={index} items={child.value} />;
case 'ORDERED_LIST':
return <OrderedListBlock key={index} items={child.value} />;
case 'BIG_EMOJI':
return <BigEmojiBlock key={index} emoji={child.value} />;
case 'CODE':
return <CodeBlock key={index} lines={child.value} />;
default:
return null;
}
})
.filter(Boolean)}
</>
);
This diff is collapsed.
import fs from 'fs';
import { PdfWorker } from './index';
import {
bigConversationData,
dataWithASingleMessageButAReallyLongMessage,
dataWithMultipleMessagesAndABigMessage,
dataWithASingleMessageAndAnImage,
} from './worker.fixtures';
const streamToBuffer = async (stream: NodeJS.ReadableStream) => {
const chunks: (string | Buffer)[] = [];
for await (const chunk of stream) {
chunks.push(chunk);
}
return Buffer.concat(chunks as Buffer[]);
};
const pdfWorker = new PdfWorker('chat-transcript');
describe('PdfWorker', () => {
it('should fail to instantiate if no mode is provided', () => {
// @ts-expect-error - testing
expect(() => new PdfWorker('')).toThrow();
});
it('should fail to instantiate if mode is invalid', () => {
// @ts-expect-error - testing
expect(() => new PdfWorker('invalid')).toThrow();
});
it('should properly instantiate', () => {
const newWorker = new PdfWorker('chat-transcript');
expect(newWorker).toBeInstanceOf(PdfWorker);
expect(newWorker.mode).toBe('chat-transcript');
});
it('should generate a pdf transcript for a big bunch of messages', async () => {
const stream = await pdfWorker.renderToStream({ data: bigConversationData });
const buffer = await streamToBuffer(stream);
expect(buffer).toBeTruthy();
});
it('should generate a pdf transcript for a single message, but a really long message', async () => {
const stream = await pdfWorker.renderToStream({ data: dataWithASingleMessageButAReallyLongMessage });
const buffer = await streamToBuffer(stream);
expect(buffer).toBeTruthy();
});
it('should generate a pdf transcript of a single message with an image', async () => {
const stream = await pdfWorker.renderToStream({ data: dataWithASingleMessageAndAnImage });
const buffer = await streamToBuffer(stream);
fs.writeFileSync('test.pdf', buffer);
expect(buffer).toBeTruthy();
});
it('should generate a pdf transcript for multiple messages, one big message and 2 small messages', async () => {
const stream = await pdfWorker.renderToStream({ data: dataWithMultipleMessagesAndABigMessage });
const buffer = await streamToBuffer(stream);
expect(buffer).toBeTruthy();
});
describe('isMimeTypeValid', () => {
it('should return true if mimeType is valid', () => {
expect(pdfWorker.isMimeTypeValid('image/png')).toBe(true);
});
it('should return false if mimeType is not valid', () => {
expect(pdfWorker.isMimeTypeValid('image/svg')).toBe(false);
});
});
});
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment