-
Notifications
You must be signed in to change notification settings - Fork 387
Custom crop buttons functionality added. #533
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
base: master
Are you sure you want to change the base?
Custom crop buttons functionality added. #533
Conversation
WalkthroughThe changes introduce dynamic crop presets to the image editor's crop tool. A new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant ToolsBar
participant CustomCropButtons
participant Store
User->>App: Open "ADJUST" tab
App->>ToolsBar: Render with dynamicButtons/upperToolbar if Crop.dynamicButtons enabled
ToolsBar->>CustomCropButtons: Render crop preset buttons
User->>CustomCropButtons: Click crop preset button
CustomCropButtons->>Store: Dispatch selectTool (crop) and setCropRatio
CustomCropButtons->>Store: Optionally dispatch autoResize and zoomCanvas
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🧹 Nitpick comments (2)
packages/react-filerobot-image-editor/src/components/ToolsBar/index.jsx (1)
63-76: Incomplete dependency list foritemsmemo
itemsdepends onupperToolbar,allPresets, and the translation functiont, yet none of them are in the dependency list.
React will therefore re‑render with stale values if any of these change.- }, [tabTools, currentToolId, dynamicCropToolId]); + }, [ + tabTools, + currentToolId, + dynamicCropToolId, + upperToolbar, + allPresets, + t, + ]);packages/react-filerobot-image-editor/src/components/tools/Crop/CustomCropButtons.jsx (1)
89-93: Selection condition is fragile and hard‑to‑read
- Comparing
currentRatioto the literal string"Crop"seems incorrect (the ratio is numeric /'original'/'custom').- Re‑computing ratio via
toPrecisedFloat(width/height)on every render is unnecessary.Consider simplifying:
- isSelected || - (currentRatio === 'Crop' && item.ratio === 'custom') || - currentRatio === - (item.ratio ?? toPrecisedFloat(item.width / item.height)) + isSelected || + currentRatio === (item.ratio ?? toPrecisedFloat(item.width / item.height))This keeps the semantics and removes the confusing
'Crop'comparison.
Optionally memoise the derived ratio for performance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
packages/react-filerobot-image-editor/src/actions/selectTool.js(1 hunks)packages/react-filerobot-image-editor/src/components/App/index.jsx(4 hunks)packages/react-filerobot-image-editor/src/components/ToolsBar/index.jsx(3 hunks)packages/react-filerobot-image-editor/src/components/tools/Crop/CustomCropButtons.jsx(1 hunks)packages/react-filerobot-image-editor/src/context/defaultTranslations.js(1 hunks)packages/react-filerobot-image-editor/src/index.d.ts(3 hunks)public/demo-config.js(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
packages/react-filerobot-image-editor/src/actions/selectTool.js (2)
packages/react-filerobot-image-editor/src/components/ToolsBar/index.jsx (1)
selectTool(41-50)packages/react-filerobot-image-editor/src/actions/updateState.js (1)
payload(7-10)
packages/react-filerobot-image-editor/src/index.d.ts (1)
packages/react-filerobot-image-editor/src/components/tools/Watermark/Watermark.jsx (1)
loadAndSetWatermarkImg(138-159)
🪛 Biome (1.9.4)
packages/react-filerobot-image-editor/src/index.d.ts
[error] 276-276: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
🔇 Additional comments (9)
packages/react-filerobot-image-editor/src/context/defaultTranslations.js (1)
116-120: Added translations for custom crop presetsThese new translation keys provide text labels for the custom crop aspect ratio options, which is a nice touch for internationalization support. The naming is clear and follows the existing pattern in the file.
public/demo-config.js (1)
176-177: Placeholder configuration for new dynamic buttons featureThese commented out options show how to enable the new dynamic crop buttons feature. The configuration is well-placed within the existing crop tool settings and provides a good reference for future users.
packages/react-filerobot-image-editor/src/actions/selectTool.js (1)
3-19: Improved state handling for dynamic crop buttonsThe refactored
selectToolfunction now properly handles dynamic crop button selection, even when the same tool is selected multiple times. This is necessary to support changing crop ratios without changing the currently selected tool.A few observations:
- The code uses optional chaining (
payload?.dynamicButtons) which is good for safety- The function now consistently returns a new state object which helps with reactivity
- The spread operator is used correctly to only add properties when needed
packages/react-filerobot-image-editor/src/components/App/index.jsx (5)
6-6: Import TABS_IDS for conditional toolbar renderingAdded import for TABS_IDS which will be used for conditional rendering of the dynamic crop toolbar.
53-54: Added state and config access for dynamic toolbarExtracting
tabIdfrom store andCropfrom config to enable context-aware dynamic crop buttons. This provides the necessary information to conditionally render the toolbar.Also applies to: 71-72
340-348: Added conditional dynamic crop toolbarThis is the core UI implementation of the custom crop buttons feature. The conditional rendering ensures the toolbar only appears when:
- Crop's dynamicButtons is enabled
- We're not in canvas-only mode
- The current tab is the ADJUST tab
The additional toolbar is positioned above the canvas, providing a good UX for crop operations.
350-355: Added dynamic buttons support to main toolbarUpdated the main toolbar to also receive the dynamic buttons flag, ensuring consistency between both toolbars. This follows the pattern of passing the same prop to both toolbar instances.
340-355:❓ Verification inconclusive
Verify mobile responsiveness of dual toolbars
With the addition of a second toolbar in some cases, it's important to ensure the UI remains responsive on mobile devices.
🏁 Script executed:
#!/bin/bash # Check if there are any specific mobile or responsive styles for the toolbars # Look for responsive styles related to toolbars echo "Checking for responsive styles for toolbars..." rg -A 5 "isPhoneScreen|@media|responsive|mobile" --type jsx --type css packages/react-filerobot-image-editor/src/components/ToolsBar/Length of output: 266
🏁 Script executed:
#!/bin/bash # List all JSX and CSS files in the ToolsBar component directory echo "Files under ToolsBar/:" find packages/react-filerobot-image-editor/src/components/ToolsBar -type f \( -iname "*.jsx" -o -iname "*.css" \) -print # Search for responsive or mobile-specific styling in JS/JSX echo -e "\nSearching JS/JSX for isPhoneScreen, mobile, tablet, or responsive flags..." rg -n "isPhoneScreen|mobile|tablet|responsive" packages/react-filerobot-image-editor/src/components/ToolsBar # Search CSS for media queries echo -e "\nSearching CSS for @media queries..." rg -n "@media" packages/react-filerobot-image-editor/src/components/ToolsBarLength of output: 3838
Verify mobile rendering of dual toolbars
We inspected
packages/react-filerobot-image-editor/src/components/ToolsBar/*and confirmed that responsiveness is handled via theisPhoneScreenprop and Styled Components rather than CSS media queries:
ToolsBar/index.jsxaccepts and passes downisPhoneScreen.ToolsBar.styled.jsapplies conditional styles based onisPhoneScreen.ToolsBarItemButton.jsxusesusePhoneScreen(320)to adjust icon sizes and spacing.No CSS
@mediarules were found, as all responsive behavior is JS‑driven. Please manually test the dual‑toolbar layout on various mobile screen sizes to ensure both upper and lower toolbars render correctly and remain usable.packages/react-filerobot-image-editor/src/index.d.ts (1)
291-303: Type declaration looks goodThe addition of the
dynamicButtons?: boolean;field and the reformattedlockCropAreaAtunion accurately reflect the new feature and improve readability. No issues spotted here.
Description:
Summary by CodeRabbit
New Features
Improvements