fix/button-with-menu-screen-freeze #517
Conversation
WalkthroughThe pull request introduces a modification to the Changes
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 (
|
![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
🔭 Outside diff range comments (1)
packages/react-filerobot-image-editor/src/components/common/ButtonWithMenu/index.jsx (1)
Line range hint
124-130: Security: Avoid using dangerouslySetInnerHTML for iconsUsing
dangerouslySetInnerHTMLwith user-provided content can expose the application to XSS attacks. Consider using a dedicated icon component or SVG renderer instead.Here's a safer approach:
- <span dangerouslySetInnerHTML={{ __html: item.icon }} /> + <IconComponent name={item.icon} />
🧹 Nitpick comments (1)
packages/react-filerobot-image-editor/src/components/common/ButtonWithMenu/index.jsx (1)
Line range hint
41-47: Consider memoizing filtered menu itemsThe
filteredMenuItemsarray is recreated on every render. Consider memoizing it withuseMemoto optimize performance, especially if the menu items array is large.- const filteredMenuItems = menuItems.filter(Boolean); + const filteredMenuItems = useMemo(() => menuItems.filter(Boolean), [menuItems]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/react-filerobot-image-editor/src/components/common/ButtonWithMenu/index.jsx(1 hunks)
🔇 Additional comments (1)
packages/react-filerobot-image-editor/src/components/common/ButtonWithMenu/index.jsx (1)
105-105: LGTM! This change fixes the screen freeze issue.
The modification to use Boolean(anchorEl) for the menu's open state ensures proper cleanup and prevents unnecessary renders when the anchor element is not present.
Let's verify there are no other instances of menus that might need similar fixes:
✅ Verification successful
The fix is correctly implemented and no other instances need similar changes
The change to use Boolean(anchorEl) for the menu's open state in ButtonWithMenu component is the right approach. After analyzing the codebase:
- The ButtonWithMenu component is the only place using the StyledMenu with an open prop controlled by anchorEl
- Other menu implementations (like in App/index.jsx) use different patterns and don't suffer from the same issue
- The CropPresetsOption component uses StyledMenu but doesn't have an open prop, as it's used differently
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other menu components that might need similar fixes
ast-grep --pattern 'StyledMenu open={$_}'
Length of output: 43
Script:
#!/bin/bash
# Let's try a broader search for menu components and their open prop patterns
rg -l "Menu.*open.*=" --type jsx --type tsx
# Also search for specific StyledMenu usage
rg "StyledMenu" --type jsx --type tsx
Length of output: 139
Script:
#!/bin/bash
# Search for Menu components and their open prop patterns without file type restrictions
rg -l "Menu.*open.*="
# Search for StyledMenu usage
rg "StyledMenu" -A 2 -B 2
Length of output: 18646
Script:
#!/bin/bash
# Let's check the Menu open prop usage in App/index.jsx since it was found in the first search
rg "Menu.*open.*=" -A 2 -B 2 packages/react-filerobot-image-editor/src/components/App/index.jsx
# Also check for any other Menu components that might need similar fixes
rg "Menu.*\{.*\}" -A 2 -B 2 packages/react-filerobot-image-editor/src/components/common/ButtonWithMenu/index.jsx
Length of output: 734
On adding more save options or opening the Watermark tool, the screen freezes completely. This happens in the ButtonWithMenu component.
Summary by CodeRabbit
ButtonWithMenucomponent by allowing the menu to open conditionally based on the presence of an anchor element.