fix: prevent menu content to be rendered when there are no options #38329
Conversation
WalkthroughThe pull request addresses a UI rendering issue in the Changes
Assessment against linked issues
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
Documentation and Community
|
github-actions
bot
added
Bug
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/12468477995. |
![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 (1)
app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionRun.tsx (1)
75-79: Consider using a unique identifier instead of 'label' in the 'key' prop.
If labels can be duplicated or changed, it might lead to React rendering conflicts. Prefer using something guaranteed unique (like an ID) in the key.-<JSFunctionItem - key={option.label} - onSelect={onFunctionSelect} - option={option} /> +<JSFunctionItem + key={option.value} // or option.id if available + onSelect={onFunctionSelect} + option={option} />
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionRun.tsx(1 hunks)
🔇 Additional comments (1)
app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionRun.tsx (1)
72-73: The conditional rendering approach looks good.
This short-circuit ensures the menu is displayed only if there are options, preventing an empty or unusable menu.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionRun.tsx (1)
72-82: Use a More Descriptive Condition Check.The double negation pattern (e.g., !!props.options.length) can be replaced by a more explicit comparison (e.g., props.options.length > 0) to improve clarity.
- {!!props.options.length && ( + {props.options.length > 0 && (
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionRun.tsx(1 hunks)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionRun.tsx (1)
72-73: Consider a clearer conditional expression.
Using the double negation (!!) makes sense functionally. However, a more explicit conditional like "props.options.length > 0" could be more readable.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionRun.tsx(1 hunks)
🔇 Additional comments (1)
app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionRun.tsx (1)
74-80: Ensure the key prop is truly unique.
Using option.label as the key works if its value is guaranteed unique. Otherwise, consider including a unique ID to avoid potential rendering issues.
|
Deploy-Preview-URL: https://ce-38329.dp.appsmith.com |
…38329) ## Description Fixes an issue where JS run menu is being shown with no items in it. Fixes #38151 ## Automation /ok-to-test tags="@tag.IDE" ### 🔍 Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/12468478974> > Commit: f032137 > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=12468478974&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.IDE` > Spec: > <hr>Mon, 23 Dec 2024 15:12:31 UTC <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Improved dropdown menu rendering logic to prevent displaying an empty menu when no options are available. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Description
Fixes an issue where JS run menu is being shown with no items in it.
Fixes #38151
Automation
/ok-to-test tags="@tag.IDE"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/12468478974
Commit: f032137
Cypress dashboard.
Tags:
@tag.IDESpec:
Mon, 23 Dec 2024 15:12:31 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit