[PE-45] refactor: page export components#5773
Conversation
WalkthroughThe changes in this pull request involve significant updates to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (5)
web/helpers/file.helper.ts (1)
11-16: Good URL validation, but consider edge cases.The addition of URL object creation for validation is a robust approach. However, be aware that some valid image URLs might not pass this strict validation. Consider adding a fallback or using a more lenient URL validation method if you expect to handle non-standard URLs.
You might want to consider using a regular expression for URL validation as a fallback. Here's an example of how you could modify the code:
// Try to create a URL object to validate the URL try { new URL(url); } catch (error) { - throw new Error("Invalid URL format"); + // Fallback to regex validation + const urlRegex = /^(https?:\/\/)?([\da-z\.-]+)\.([a-z\.]{2,6})([\/\w \.-]*)*\/?$/; + if (!urlRegex.test(url)) { + throw new Error("Invalid URL format"); + } }This change allows for more flexibility while still maintaining strong validation.
web/helpers/editor.helper.ts (2)
Line range hint
1-136: Enhancements to HTML content processing look good, with some considerations.The changes to
replaceCustomComponentsFromHTMLContentimprove the export functionality:
- Checkbox handling enhances visual representation.
- Removal of
issue-embed-componentelements is consistent with markdown processing.- Removal of null colors from table elements improves output quality.
However, consider the following:
- Replacing checkboxes with non-interactive div elements might affect user experience if the exported content is intended to be interactive.
- Removing
issue-embed-componentelements could lead to loss of information. Consider adding a placeholder or reference to the removed content.To maintain content integrity, consider implementing a configuration option to control the removal of interactive elements or embedded components during export.
137-138: Improved origin URL handling, with a minor suggestion.The change to check for the existence of the
windowobject before accessing its properties is a good improvement. It prevents potential errors in environments wherewindowmay not be available, such as during server-side rendering.Consider using optional chaining for a more concise and equally safe approach:
const originUrl = window?.location?.origin || "";This achieves the same result with a slightly more compact syntax.
web/core/components/pages/modals/export-page-modal.tsx (2)
112-115: Improved filename sanitization, consider adding a trim step.The new implementation robustly handles non-alphanumeric characters and consecutive hyphens, resulting in cleaner filenames. Well done!
Consider adding a
.trim()step to remove any leading or trailing hyphens:const fileName = pageTitle ?.toLowerCase() ?.replace(/[^a-z0-9-_]/g, "-") - .replace(/-+/g, "-"); + .replace(/-+/g, "-") + .replace(/^-|-$/g, "");This ensures that the filename doesn't start or end with a hyphen.
124-133: Well-structured download function, consider adding error handling.The
initiateDownloadfunction effectively centralizes the download logic, improving code reusability and maintainability. The use ofURL.revokeObjectURLis a good practice for memory management.Consider adding a try-catch block to handle potential errors during the download process:
const initiateDownload = (blob: Blob, filename: string) => { + try { const url = URL.createObjectURL(blob); const link = document.createElement("a"); link.href = url; link.download = filename; link.click(); setTimeout(() => { URL.revokeObjectURL(url); }, 1000); + } catch (error) { + console.error("Error initiating download:", error); + setToast({ + type: TOAST_TYPE.ERROR, + title: "Error!", + message: "Failed to initiate download. Please try again.", + }); + } };This addition will help catch and handle any unexpected errors during the download process.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
- web/core/components/pages/modals/export-page-modal.tsx (3 hunks)
- web/core/constants/editor.ts (0 hunks)
- web/helpers/editor.helper.ts (1 hunks)
- web/helpers/file.helper.ts (1 hunks)
💤 Files with no reviewable changes (1)
- web/core/constants/editor.ts
🧰 Additional context used
🔇 Additional comments (4)
web/helpers/file.helper.ts (3)
7-9: Good addition of input validation.The added check for the
urlparameter's existence and type is a good practice. It helps catch potential errors early and provides a clear error message.
Line range hint
18-42: Existing implementation looks good.The unchanged part of the function, which handles fetching the image and converting it to base64, is well-implemented. It uses Promises appropriately and has good error handling for both the fetch operation and the FileReader.
Line range hint
1-42: Overall, good improvements to input validation.The changes to the
getBase64Imagefunction enhance its robustness by adding thorough input validation. The added checks for theurlparameter's existence, type, and format will help catch potential errors early in the function execution. These improvements align well with the PR objective of refactoring and improving code maintainability.To further improve the function, consider the suggestion about URL validation to handle potential edge cases with non-standard URLs.
web/core/components/pages/modals/export-page-modal.tsx (1)
145-145: Improved export functions with centralized download logic.The use of the
initiateDownloadfunction in bothhandleExportAsPDFandhandleExportAsMarkdownsimplifies these functions and reduces code duplication. The file names now benefit from the improved sanitization logic.These changes enhance the overall code structure and maintainability.
Also applies to: 160-160
Summary by CodeRabbit
New Features
Bug Fixes
Chores