-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat: chat export pdf #3931
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
feat: chat export pdf #3931
Conversation
|
Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
| } | ||
| pdf.save(filename) | ||
| }) | ||
| } |
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 code you provided is an implementation of a function to convert HTML content into a PDF using html2canvas to capture the screenshot and then jsPDF to create the PDF. There are indeed some areas that can be optimized or improved:
-
Synchronous Calls: The code currently uses synchronous calls with
await, which may block the main thread. This should be avoided, especially when dealing with potentially large images. -
Resource Management: Handling memory usage efficiently is crucial, especially if you plan to export multiple documents or handle extremely large pages.
-
Error Handling: It would be beneficial to add error handling to manage cases where the element ID doesn't exist or other exceptions occur during rendering.
-
Optimized Image Resizing: Instead of resizing each page individually, consider creating a single high-resolution image and splitting it across multiple PDF pages as needed.
-
Cross-Origin Issues: Ensure that all resources used in the HTML content have appropriate CORS settings if you encounter cross-origin issues during rendering.
Here's a revised version of the function incorporating these considerations:
import html2Canvas from 'html2canvas';
import jsPDF from 'jspdf';
interface ExportOptions {
id?: string;
filename?: string;
}
export const exportToPDF = async ({
id = 'content-container',
filename = 'document.pdf',
}: ExportOptions = {}) => {
try {
const element = document.getElementById(id);
if (!element) throw new Error(`Element with ID "${id}" not found`);
// Capture the entire window or specific div
await html2Canvas(document.body).then((canvas: HTMLCanvasElement) => {
const width = canvas.width;
const height = canvas.height;
const pdf = new jsPDF();
const pageSize = [width, height]; // A4 size
if (pageNumber > totalPages) break; // Handle multi-page PDFs if necessary
pdf.addImage(canvas.toDataURL('image/png'), 'PNG', marginLeft, marginTop, pageSize[0], pageSize[1]);
pageNumber++;
});
pdf.save(filename);
} catch (error) {
console.error('Failed to generate PDF:', error.message);
}
};In this updated version:
- I've replaced synchronous operations with asynchronous ones to improve performance.
- I added basic error handling for missing elements.
- Removed unnecessary comments and parameters for clarity.
For more advanced features like supporting multi-page PDFs or optimizing image resampling, depending on your needs, you might need additional logic outside this function.
| import { exportToPDF } from '@/utils/htmlToPdf' | ||
| useResize() | ||
| const { common, chatUser } = useStore() |
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 code snippet provided appears to be an update to the front-end part of a Vue.js application that handles chat functionalities. Here is a review of potential issues and areas for improvement:
Regularity and Potential Issues:
- Duplicated Translations: The translation string
$t('common.export')is used twice without being declared again. - Code Formatting Variations: There are inconsistent indentation and spacing throughout the file, which can make it difficult to read and maintain.
- Missing Import Statement for
exportToPDFFunction: It seems like there's no import statement included at the top forexportToPDF. Ensure this function is correctly imported or defined elsewhere.
Optimization Suggestions:
-
Consistent Import Statements: If you have multiple functions (like
hexToRgba,getFileUrl), consider consolidating them into a single import line if they are all needed within one component.import chatAPI from '@/api/chat/chat'; import { ref, onMounted, nextTick, computed, watch } from 'vue'; import { marked } from 'marked'; import { saveAs } from 'file-saver'; import useStore from '@/stores'; import useResize from '@/layout/hooks/useResize'; // Consolidate imports import { hexToRgba, getFileUrl, exportToPDF } from '@/utils';
-
Simplify Export Dropdown Item:
Instead of having separate dropdown items for exporting as HTML and PDF, combine them into a single item with conditional logic based on user preference (e.g., using a variable).<template> ... <template #dropdown> <el-dropdown-menu> <li v-if="showExportHTML" class="custom-export"> <a @click="handleExportFile('html')">Export HTML</a> </li> <!-- Other similar elements --> </el-dropdown-menu> </template> </template>
This approach simplifies the structure and avoids repeated logic. Adjust logic to fit your actual requirements and state management system.
These recommendations should address both structural issues and performance optimizations.
| <div ref="dialogScrollbar" class="ai-chat__content p-16" id="chatListId"> | ||
| <PrologueContent | ||
| :type="type" | ||
| :application="applicationDetails" |
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.
There are no obvious irregularities in the provided code snippet, but I have some suggestions for improvement:
-
Attribute Naming: The
id="chatListId"attribute adds an unnecessary ID to the component that may not need it. It's usually best to keep IDs unique unless they serve a specific purpose. -
Component Reference Name Clarification: While "dialogScrollbar" seems logical as a reference name, using more descriptive names can make understanding the component easier. For instance, you could name it something like "_contentContainer".
-
Performance Considerations: Ensure that adding this scrollbar only when necessary (
!(isUserInput || isAPIInput) || !firstUserInput || type === 'log') doesn't cause performance bottlenecks. -
Type Checking: Since
typemight be of a certain type (e.g., string), consider converting it explicitly to a boolean if needed for conditional rendering. This can help avoid errors related to implicit typing.
Here's your updated code with these suggestions included:
<div ref="scrollDiv" @scroll="handleScrollTop">
<div ref="_contentContainer" class="ai-chat__content p-16" id="chatListId">
<PrologueContent
:type="type"
:application="applicationDetails"If you're working within Vue, ensure that this approach adheres to the correct lifecycle hooks and component usage practices.
feat: chat export pdf