Feat/file uploader message #69
Conversation
WalkthroughThis pull request introduces updates to the Changes
Possibly related PRs
Suggested reviewers
Poem
🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
web/src/utils/index.ts (2)
13-19: Add JSDoc documentation for the Role type.Adding documentation will help other developers understand the purpose and structure of this type.
+/** + * Represents a role with its associated file upload restrictions. + * @property {string} name - The name of the role + * @property {Object} restriction - The upload restrictions for this role + * @property {number} restriction.maxSize - Maximum allowed file size in bytes + * @property {string[]} restriction.allowedMimeTypes - List of allowed MIME types + */ type Role = { name: string; restriction: { maxSize: number; allowedMimeTypes: string[]; }; };
21-35: Enhance the robustness and readability of the file uploader message generator.The function could benefit from several improvements:
- Handle edge cases for MIME types
- Use constants for size conversion
- Improve message formatting
+const BYTES_PER_MB = 1024 * 1024; + export const getFileUploaderMsg = (role: Roles, roleRestrictions?: Role[]) => { if (!roleRestrictions) return; const restrictions = roleRestrictions.find((supportedRoles) => Roles[supportedRoles.name] === role); if (!restrictions) return; + if (!restrictions.restriction.allowedMimeTypes.length) return; + const formatMimeType = (type: string) => { + if (!type.includes('/')) return type; + const [prefix, suffix] = type.split('/'); + return suffix === '*' ? prefix : suffix; + }; + const formattedTypes = restrictions.restriction.allowedMimeTypes + .map(formatMimeType) + .filter(Boolean) + .join(', '); + const maxSizeMB = (restrictions.restriction.maxSize / BYTES_PER_MB).toFixed(2); - return `Allowed file types: [ ${restrictions.restriction.allowedMimeTypes - .map((type) => { - const [prefix, suffix] = type.split("/"); - if (!suffix) return prefix ?? null; - - return suffix === "*" ? prefix : suffix; - }) - .join(", ")} ], Max allowed size: ${(restrictions.restriction.maxSize / (1024 * 1024)).toFixed(2)} MB.`; + return `Allowed file types: [${formattedTypes}]\nMax allowed size: ${maxSizeMB} MB`; };web/src/pages/SubmitItem/ItemField/FieldInput/FileInput.tsx (1)
Line range hint
33-44: Improve error handling in file upload.The error handling could be enhanced:
- Remove console.log
- Add more specific error messages
const handleFileUpload = (file: File) => { infoToast("Uploading to IPFS..."); uploadFile(file, Roles.CurateItemFile) .then(async (fileURI) => { if (!fileURI) return; successToast("Uploaded successfully!"); handleWrite(fileURI); }) .catch((err) => { - console.log(err); - errorToast(`Upload failed: ${err?.message}`); + const errorMessage = err instanceof Error + ? err.message + : "An unexpected error occurred"; + errorToast(`File upload failed: ${errorMessage}`); }); };web/src/pages/SubmitItem/ItemField/FieldInput/ImageInput.tsx (1)
Line range hint
34-46: Remove unnecessary .finally() call.The empty .finally() block can be removed.
uploadFile(file, Roles.CurateItemImage) .then(async (fileURI) => { if (!fileURI) return; successToast("Uploaded successfully!"); handleWrite(fileURI); }) .catch((err) => { console.log(err); errorToast(`Upload failed: ${err?.message}`); - }) - .finally(); + });web/src/pages/SubmitList/ListParameters/LogoUpload.tsx (2)
Line range hint
65-82: Enhance image validation robustness.The aspect ratio validation could be improved by:
- Adding size validation
- Adding minimum dimensions check
- Adding maximum dimensions check
const handleLoad = (file: File) => { const reader = new FileReader(); reader.onload = (event) => { const image = new Image(); image.onload = () => { + const MIN_DIMENSIONS = 200; + const MAX_DIMENSIONS = 2000; + if (image.width !== image.height) { errorToast("Image aspect ratio must be 1:1"); return; } + if (image.width < MIN_DIMENSIONS || image.height < MIN_DIMENSIONS) { + errorToast(`Image dimensions must be at least ${MIN_DIMENSIONS}x${MIN_DIMENSIONS} pixels`); + return; + } + if (image.width > MAX_DIMENSIONS || image.height > MAX_DIMENSIONS) { + errorToast(`Image dimensions must not exceed ${MAX_DIMENSIONS}x${MAX_DIMENSIONS} pixels`); + return; + } handleFileUpload(file); }; image.src = event.target?.result as string; }; reader.readAsDataURL(file); };
87-90: Simplify message concatenation using template literals.The message concatenation can be simplified and made more readable.
- msg={ - "Upload a logo to represent your list. The logo should be a 1:1 aspect ratio image with transparent background, in SVG, or PNG.\n" + - (getFileUploaderMsg(Roles.Logo, roleRestrictions) ?? "") - } + msg={`Upload a logo to represent your list. The logo should be a 1:1 aspect ratio image with transparent background, in SVG, or PNG. +${getFileUploaderMsg(Roles.Logo, roleRestrictions) ?? ""}`}web/src/components/ActionButton/Modal/EvidenceUpload.tsx (1)
112-116: Consider improving message formatting.While the functionality is correct, the message concatenation could be more maintainable:
- msg={ - "Additionally, you can add an external file in PDF.\n" + - (getFileUploaderMsg(Roles.Evidence, roleRestrictions) ?? "") - } + msg={[ + "Additionally, you can add an external file in PDF.", + getFileUploaderMsg(Roles.Evidence, roleRestrictions) + ].filter(Boolean).join("\n")}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (7)
web/package.json(1 hunks)web/src/components/ActionButton/Modal/EvidenceUpload.tsx(4 hunks)web/src/pages/SubmitItem/ItemField/FieldInput/FileInput.tsx(2 hunks)web/src/pages/SubmitItem/ItemField/FieldInput/ImageInput.tsx(3 hunks)web/src/pages/SubmitList/ListParameters/LogoUpload.tsx(3 hunks)web/src/pages/SubmitList/ListParameters/Policy.tsx(3 hunks)web/src/utils/index.ts(2 hunks)
🔇 Additional comments (9)
web/src/pages/SubmitItem/ItemField/FieldInput/FileInput.tsx (1)
18-21: LGTM! Styling improvements for better message readability.The white-space and text-align properties ensure proper formatting of multi-line messages.
web/src/pages/SubmitList/ListParameters/Policy.tsx (4)
13-14: LGTM! New imports enhance file upload functionality.The addition of
getFileUploaderMsganduseIsDesktopaligns with the PR's objective of improving file upload messaging and device-specific styling.
36-43: Verify the significant margin increase.The margin-bottom has been increased from
responsiveSize(52, 32)toresponsiveSize(150, 72), which is a substantial change. Please ensure this doesn't create excessive whitespace or affect the layout negatively.The new styles for the
smallelement improve text readability withwhite-space: pre-lineandtext-align: start.
48-50: LGTM! Enhanced component with role restrictions and device detection.The addition of
roleRestrictionsandisDesktophook provides necessary context for dynamic messaging and responsive styling.
84-88: LGTM! Improved file uploader with dynamic messaging.The StyledFileUploader now:
- Adapts its variant based on the device type
- Includes role-specific messaging using
getFileUploaderMsgweb/src/components/ActionButton/Modal/EvidenceUpload.tsx (3)
8-9: LGTM! Consistent import additions.The imports match the pattern established in Policy.tsx, maintaining consistency across components.
40-47: Verify consistent margin changes across components.The margin-bottom increase matches Policy.tsx. The text styling for small elements is also consistent.
66-68: LGTM! Consistent hook usage.The component follows the same pattern as Policy.tsx for role restrictions and device detection.
web/package.json (1)
73-73: Verify @kleros/kleros-app upgrade impact.The upgrade from ^2.0.1 to ^2.0.2 is a patch version bump. However, please verify:
- Changelog for any breaking changes
- Compatibility with other dependencies
- Impact on the new file upload message functionality
✅ Verification successful
✓ Safe to proceed with @kleros/kleros-app update
The upgrade from 2.0.1 to 2.0.2 is a safe patch version update with minimal risk. The package is used consistently for authentication, file uploads, and notifications through standard hooks and enums, which are stable interfaces unlikely to be affected by patch-level changes.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check the changelog and git history for breaking changes gh api repos/kleros/kleros-app/releases/tags/v2.0.2 --jq '.body' || echo "Release notes not found" # Check for potential compatibility issues in the codebase rg -l "@kleros/kleros-app" | xargs rg "import.*from.*@kleros/kleros-app"Length of output: 1664
Script:
#!/bin/bash # Check npm registry for package details and changes npm view @kleros/[email protected] version description npm view @kleros/[email protected] version description # Check the actual published dates to understand the timeline npm view @kleros/kleros-app time --json | jq '."2.0.1", ."2.0.2"'Length of output: 437
✅ Deploy Preview for curate-v2 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Harman-singh-waraich
dismissed stale reviews from kemuru and coderabbitai[bot]
via
9e39555
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
web/src/pages/SubmitList/ListParameters/LogoUpload.tsx (1)
86-90: Consider enhancing error handling and user guidance.While the implementation looks good, consider these improvements:
- The null coalescing operator handles missing restrictions, but we could provide more informative fallback text.
- The aspect ratio requirement is mentioned in the message but not enforced until the error callback. Consider moving this validation earlier in the process.
msg={ "Upload a logo to represent your list. The logo should be a 1:1 aspect ratio image with transparent background.\n" + - (getFileUploaderMsg(Roles.Logo, roleRestrictions) ?? "") + (getFileUploaderMsg(Roles.Logo, roleRestrictions) ?? "Please ensure your file meets the upload requirements.") }web/src/pages/SubmitList/ListParameters/Policy.tsx (2)
13-14: Consider standardizing import paths.For consistency, consider using absolute imports (starting with
src/) for both imports:-import { getFileUploaderMsg } from "src/utils"; -import useIsDesktop from "hooks/useIsDesktop"; +import { getFileUploaderMsg } from "src/utils"; +import { useIsDesktop } from "src/hooks/useIsDesktop";
84-88: Consider improving message composition.While the current implementation works, the message concatenation could be more elegant:
- msg={"You can add the List Policy file." + (getFileUploaderMsg(Roles.Policy, roleRestrictions) ?? "")} + msg={[ + "You can add the List Policy file.", + getFileUploaderMsg(Roles.Policy, roleRestrictions) + ].filter(Boolean).join(" ")}This approach:
- Improves readability
- Handles null/undefined values more elegantly
- Makes it easier to add more message parts in the future
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
web/src/components/ActionButton/Modal/EvidenceUpload.tsx(4 hunks)web/src/pages/SubmitList/ListParameters/LogoUpload.tsx(3 hunks)web/src/pages/SubmitList/ListParameters/Policy.tsx(3 hunks)web/src/utils/index.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- web/src/utils/index.ts
- web/src/components/ActionButton/Modal/EvidenceUpload.tsx
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Redirect rules - curate-v2
- GitHub Check: Header rules - curate-v2
- GitHub Check: Pages changed - curate-v2
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (5)
web/src/pages/SubmitList/ListParameters/LogoUpload.tsx (3)
12-13: LGTM! New imports align with the PR objectives.The added imports for file upload messaging and responsive design support the enhanced functionality described in the PR objectives.
31-31: Verify the significant margin increase.The margin-bottom has been increased substantially from the previous values. While this might improve spacing, please verify that this doesn't create excessive vertical gaps in the UI, especially on smaller screens.
The new text styling for the upload message looks good and will improve readability.
Also applies to: 35-38
44-45: LGTM! Clean implementation of role-based restrictions and responsive design.The addition of roleRestrictions and isDesktop hooks follows React best practices and supports the enhanced file upload messaging feature.
web/src/pages/SubmitList/ListParameters/Policy.tsx (2)
36-36: Verify the significant margin increase.The margin-bottom has been increased substantially from the previous values. Please confirm if this large spacing is intentional and necessary for the design.
The new styling for the
smallelement improves text formatting and alignment.Also applies to: 40-43
48-50: LGTM! Good enhancement of component logic.The addition of
roleRestrictionsandisDesktopimproves the component's functionality by:
- Supporting role-based file restrictions
- Enabling responsive behavior based on device type
PR-Codex overview
This PR primarily updates the version of the
@kleros/kleros-apppackage and enhances file upload functionality across multiple components by integrating role-based restrictions and adjusting UI elements for better user experience.Detailed summary
@kleros/kleros-appfrom^2.0.1to^2.0.2inpackage.jsonandyarn.lock.roleRestrictionsandisDesktophooks in various components.getFileUploaderMsgto display file upload restrictions.margin-bottomforStyledFileUploader.ImageInput,FileInput,Policy,EvidenceUpload, andLogoUploadcomponents.Summary by CodeRabbit
Release Notes
New Features
Improvements
Dependency Updates
@kleros/kleros-apppackage to version 2.0.2.