chore: Updating the function icon on JS editor #37340
Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (2)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughThe pull request introduces a new JavaScript function icon, Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
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
IDE Pod
|
/build-deploy-preview skip-tests=true |
github-actions
bot
added
the
skip-changelog
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/11797429337. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
app/client/src/components/editorComponents/ActionCreator/helpers.tsx (1)
Line range hint
282-284: Consider addressing the eslint TODOsMultiple TODOs indicate the need to fix explicit
anytypes. Addressing these would improve type safety.Would you like me to create a GitHub issue to track the TypeScript improvements needed in this file?
Also applies to: 449-451, 566-568, 633-635
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (1)
app/client/packages/design-system/ads/src/__assets__/icons/menu/js-function-v2.svgis excluded by!**/*.svg
📒 Files selected for processing (3)
app/client/packages/design-system/ads/src/Icon/Icon.provider.tsx(2 hunks)app/client/src/components/editorComponents/ActionCreator/helpers.tsx(2 hunks)app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionRun.tsx(1 hunks)
🔇 Additional comments (5)
app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionRun.tsx (2)
Line range hint 31-45: LGTM! Clean implementation of feature flag toggle
The component properly handles the transition between old and new versions using the feature flag.
Line range hint 63-70: LGTM! Functionality preserved while updating UI
The icon update maintains all existing button functionality, accessibility features, and test hooks.
app/client/src/components/editorComponents/ActionCreator/helpers.tsx (1)
611-611: LGTM: Icon update is consistent across function types
The icon update from "js-function" to "js-function-v2" is applied consistently for both regular JS functions and module instance functions.
Let's verify no legacy icon references remain:
Also applies to: 661-661
✅ Verification successful
LGTM: Icon update is consistent and no legacy references found
All references to name="js-function" have been successfully updated to name="js-function-v2" across the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining references to the old icon name
# Expect: No occurrences of the old icon name
rg "name=\"js-function\"" --type ts --type tsx
Length of output: 74
Script:
#!/bin/bash
# Step 1: List available file types in rg
rg --type-list
# Step 2: Search for any remaining references to the old icon name in TypeScript and TSX files using glob patterns
rg 'name="js-function"' --glob '*.ts' --glob '*.tsx'
Length of output: 4876
app/client/packages/design-system/ads/src/Icon/Icon.provider.tsx (2)
966-968: LGTM: Icon import follows established patterns.
The import statement is well-structured and follows the existing conventions for icon imports.
1291-1291: LGTM: Icon lookup entry is properly defined.
The icon mapping follows the established naming convention and is correctly integrated into the lookup object.
app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionRun.tsx
Outdated
Show resolved
Hide resolved
|
Deploy-Preview-URL: https://ce-37340.dp.appsmith.com |
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (1)
app/client/src/assets/icons/menu/js-function-v2.svgis excluded by!**/*.svg
📒 Files selected for processing (1)
app/client/src/icons/MenuIcons.tsx(1 hunks)
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/11811788030. |
|
Deploy-Preview-URL: https://ce-37340.dp.appsmith.com |
Description
Updating the function icon on JS editor to use the below icon:
https://www.figma.com/design/8L9BXMzNTKboGWlHpdXyYP/Appsmith-IDE?node-id=3320-86616&t=m7Tv72GIRzKZCugh-4
Fixes #37331
Automation
/ok-to-test tags="@tag.JS"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/11811778322
Commit: 1e8555b
Cypress dashboard.
Tags:
@tag.JSSpec:
Wed, 13 Nov 2024 07:03:04 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
js-function-v2) for enhanced visual representation in the application.DataSourcesIconV2).Bug Fixes
Documentation