feat: Trigger autocomplete even outside bindings #39446
Conversation
WalkthroughThe changes update several Cypress test cases and code editor utilities. In the tests, the serialized JSON and array values passed to methods now have an appended space character. In the code editor components, type safety improvements are applied and a new utility function ( Changes
Sequence Diagram(s)sequenceDiagram
participant Editor as Code Editor
participant Hint as Hint Helper
participant Util as Autocomplete Utils
participant Tern as Tern Service
Editor->>Hint: User types in editor
Hint->>Util: Check if autocomplete should be triggered
Util-->>Hint: Return decision (show/hide)
Hint->>Tern: Request code completions based on context
Tern-->>Editor: Return formatted completion suggestions
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (3)
⏰ Context from checks skipped due to timeout of 90000ms (8)
🔇 Additional comments (6)
🪧 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
Enhancement
|
/build-deploy-preview |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/13540563905. |
|
Deploy-Preview-URL: https://ce-39446.dp.appsmith.com |
|
/build-deploy-preview |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/13651914111. |
|
/build-deploy-preview |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/13652662670. |
|
Deploy-Preview-URL: https://ce-39446.dp.appsmith.com |
|
/build-deploy-preview |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/13670017466. |
|
Deploy-Preview-URL: https://ce-39446.dp.appsmith.com |
![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)
app/client/cypress/e2e/Regression/ClientSide/Widgets/JSONForm/JSONForm_CurrencyField_spec.ts (1)
51-51: 🛠️ Refactor suggestionRemove Sleep usage.
Using
agHelper.Sleep(500)violates the coding guidelines. Replace with proper Cypress waiting strategies.- agHelper.Sleep(500); + agHelper.WaitUntil(() => agHelper.IsElementVisible(locators._jsonFormInputField("employee_id")), + "Wait for input field to be ready");
🧹 Nitpick comments (1)
app/client/src/utils/autocomplete/CodemirrorTernService.ts (1)
1378-1389: Fixed mode detection condition for JavaScript.Changed the condition from checking if mode name is not JavaScript to checking if it is JavaScript. This ensures expected type is only set when appropriate.
Consider adding a comment explaining why the condition was changed to make the intent clearer.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/client/cypress/e2e/Regression/ClientSide/Debugger/Widget_property_navigation_spec.ts(1 hunks)app/client/cypress/e2e/Regression/ClientSide/Widgets/JSONForm/JSONForm_CurrencyField_spec.ts(1 hunks)app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/AddNewRow1_spec.js(1 hunks)app/client/src/components/editorComponents/CodeEditor/codeEditorUtils.ts(2 hunks)app/client/src/components/editorComponents/CodeEditor/hintHelpers.ts(2 hunks)app/client/src/utils/autocomplete/CodemirrorTernService.ts(7 hunks)
✅ Files skipped from review due to trivial changes (1)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/AddNewRow1_spec.js
🧰 Additional context used
📓 Path-based instructions (1)
`app/client/cypress/**/**.*`: Review the following e2e test ...
app/client/cypress/**/**.*: Review the following e2e test code written using the Cypress test library. Ensure that:
- Follow best practices for Cypress code and e2e automation.
- Avoid using cy.wait in code.
- Avoid using cy.pause in code.
- Avoid using agHelper.sleep().
- Use locator variables for locators and do not use plain strings.
- Use data-* attributes for selectors.
- Avoid Xpaths, Attributes and CSS path.
- Avoid selectors like .btn.submit or button[type=submit].
- Perform logins via API with LoginFromAPI.
- Perform logout via API with LogOutviaAPI.
- Perform signup via API with SignupFromAPI.
- Avoid using it.only.
- Avoid using after and aftereach in test cases.
- Use multiple assertions for expect statements.
- Avoid using strings for assertions.
- Do not use duplicate filenames even with different paths.
- Avoid using agHelper.Sleep, this.Sleep in any file in code.
app/client/cypress/e2e/Regression/ClientSide/Debugger/Widget_property_navigation_spec.tsapp/client/cypress/e2e/Regression/ClientSide/Widgets/JSONForm/JSONForm_CurrencyField_spec.ts
🧠 Learnings (1)
app/client/src/utils/autocomplete/CodemirrorTernService.ts (1)
Learnt from: ashit-rath
PR: appsmithorg/appsmith#33726
File: app/client/src/utils/autocomplete/CodemirrorTernService.ts:199-199
Timestamp: 2024-11-12T08:11:36.416Z
Learning: The `entityDef` property in `CodeMirrorTernService` class within `CodemirrorTernService.ts` is intentionally initialized as an empty object.
🪛 Biome (1.9.4)
app/client/src/components/editorComponents/CodeEditor/codeEditorUtils.ts
[error] 10-10: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: perform-test / rts-build / build
- GitHub Check: perform-test / client-build / client-build
- GitHub Check: perform-test / server-build / server-unit-tests
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-lint / client-lint
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-prettier / prettier-check
- GitHub Check: client-build / client-build
🔇 Additional comments (13)
app/client/cypress/e2e/Regression/ClientSide/Widgets/JSONForm/JSONForm_CurrencyField_spec.ts (1)
26-26: Be consistent with JSON serialization.The addition of an empty space after JSON.stringify() appears to be a deliberate change, likely to trigger proper parsing behavior in the JSONForm component.
app/client/cypress/e2e/Regression/ClientSide/Debugger/Widget_property_navigation_spec.ts (1)
86-86: Be consistent with JSON serialization.The addition of an empty space after JSON.stringify() appears to be a deliberate change, likely to trigger proper parsing behavior in the JSONForm component, consistent with changes in other test files.
app/client/src/components/editorComponents/CodeEditor/hintHelpers.ts (3)
10-10: New import for autocomplete with binding brackets.This import adds the utility function needed to determine if autocomplete should be shown outside binding brackets.
49-49: Changed default behavior to show autocomplete.Initial value of
shouldShowis nowtruerather thanfalse, which aligns with the PR objective to make autocomplete more available.
55-57: Added support for showing autocomplete outside binding brackets.This condition checks if autocomplete should be shown with binding brackets even when the cursor is not inside a binding, enhancing the autocomplete experience. The new functionality correctly preserves the existing behavior for other scenarios.
app/client/src/components/editorComponents/CodeEditor/codeEditorUtils.ts (3)
9-9: Improved type safety for removeNewLineChars.Type parameter changed from
anytostringfor better type safety.
13-13: Enhanced type handling in getInputValue.Improves type safety by:
- Changing parameter type from
anytounknown- Explicitly ensuring the return type is always a string
This change avoids potential type-related issues and makes the function more robust.
Also applies to: 20-20
170-191: New function to determine if autocomplete should be shown outside binding brackets.This function implements the core logic for the new feature, enabling autocomplete suggestions for single-word inputs outside binding brackets, while maintaining existing behavior for SQL mode and special inputs.
The logic correctly excludes:
- SQL mode (where different autocomplete rules apply)
- Empty values or slash commands (which have different handling)
- Multi-word inputs (where autocomplete would be less relevant)
app/client/src/utils/autocomplete/CodemirrorTernService.ts (5)
24-24: Added import for cursor binding detection.Imports the utility function needed to check if cursor is inside binding brackets.
520-521: Added detection for JavaScript actions.New variable to determine if the current field is a JavaScript action, which influences autocomplete presentation.
556-556: Added cursor position binding detection.Determines if the cursor is inside binding brackets to control the autocomplete behavior appropriately.
596-599: Enhanced autocomplete presentation for non-binding contexts.When cursor is outside binding brackets and not in a JS action field, this wraps completions with
{{ }}to maintain the expected binding format while providing autocomplete anywhere in the editor.This is the key implementation for the PR's objective of triggering autocomplete outside binding brackets.
756-773: Improved cursor positioning after completion selection.Added logic to detect if completion ends with binding brackets and position the cursor appropriately:
- For functions: Places cursor between parentheses
- For completions with binding brackets: Places cursor before the closing brackets
This enhances user experience by placing the cursor in the most useful position after selecting an autocompletion.
…appsmith into feat/autocomplete-triggers
Description
Shows autocomplete with results in binding brackets
{{ <result> }}when the user is typing something outside the bindingFixes #39112
Automation
/ok-to-test tags="@tag.All"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/13699728311
Commit: cd3816b
Cypress dashboard.
Tags:
@tag.AllSpec:
Thu, 06 Mar 2025 14:46:52 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit