Skip to content

Commit

Permalink
fix: Set conditional wrapping for big messages on PDF transcript's re…
Browse files Browse the repository at this point in the history
…act template (RocketChat#32311)
  • Loading branch information
KevLehman committed May 9, 2024
1 parent 3b3275f commit ad86761
Show file tree
Hide file tree
Showing 14 changed files with 865 additions and 52 deletions.
9 changes: 9 additions & 0 deletions .changeset/nasty-swans-compete.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
"@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 changes: 1 addition & 1 deletion ee/packages/pdf-worker/jest.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
},
Expand Down
5 changes: 5 additions & 0 deletions ee/packages/pdf-worker/jest.worker.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export default {
preset: 'ts-jest',
errorOnDeprecated: true,
modulePathIgnorePatterns: ['<rootDir>/dist/', '<rootDir>/src/strategies/', '<rootDir>/src/templates/'],
};
2 changes: 2 additions & 0 deletions ee/packages/pdf-worker/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
7 changes: 5 additions & 2 deletions ee/packages/pdf-worker/src/strategies/ChatTranscript.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
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';
Expand Down Expand Up @@ -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', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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' },
Expand All @@ -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' },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { Quotes } from './Quotes';
const styles = StyleSheet.create({
wrapper: {
marginBottom: 16,
paddingBottom: 16,
paddingHorizontal: 32,
},
message: {
Expand All @@ -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>
</>
);
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
});
Expand All @@ -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}
Expand Down
2 changes: 2 additions & 0 deletions ee/packages/pdf-worker/src/templates/ChatTranscript/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { View } from '@react-pdf/renderer';
import type * as MessageParser from '@rocket.chat/message-parser';

import InlineElements from '../elements/InlineElements';
Expand All @@ -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;
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { View } from '@react-pdf/renderer';
import type * as MessageParser from '@rocket.chat/message-parser';

import BigEmojiBlock from './blocks/BigEmojiBlock';
Expand All @@ -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)}
</>
);
Loading

0 comments on commit ad86761

Please sign in to comment.