-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
feat: IDE tab list search view #34759
Conversation
…o ide-tab-list-search-view
…o ide-tab-list-search-view
…o ide-tab-list-search-view
WalkthroughThe changes introduce a search bar and add button to the list views in the editor pane of a client application. The update includes state management for search terms, fuzzy search functionality, and relevant tests. The modifications streamline the process of searching and adding items within the lists, enhancing user experience. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI
participant SearchBar
participant Backend
User->>SearchBar: Enter search term
SearchBar->>Backend: Fetch filtered data
Backend-->>SearchBar: Return filtered data
SearchBar-->>UI: Display search results
User->>UI: Click "Add" button
UI->>Backend: Add new item
Backend-->>UI: Confirm addition
UI-->>User: Display updated list
Assessment against linked issues
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 as PR comments)
Additionally, you can add CodeRabbit Configration 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: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
app/client/yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
Files selected for processing (7)
- app/client/package.json (1 hunks)
- app/client/src/ce/constants/messages.ts (1 hunks)
- app/client/src/pages/Editor/IDE/EditorPane/JS/List.tsx (6 hunks)
- app/client/src/pages/Editor/IDE/EditorPane/Query/List.tsx (5 hunks)
- app/client/src/pages/Editor/IDE/EditorPane/components/AddAndSearchbar.tsx (1 hunks)
- app/client/src/pages/Editor/IDE/EditorPane/fuzzySearchInFiles.test.ts (1 hunks)
- app/client/src/pages/Editor/IDE/EditorPane/utils.ts (1 hunks)
Files skipped from review due to trivial changes (2)
- app/client/package.json
- app/client/src/ce/constants/messages.ts
Additional comments not posted (12)
app/client/src/pages/Editor/IDE/EditorPane/components/AddAndSearchbar.tsx (2)
10-25
: LGTM!The component logic is correct and adheres to best practices.
1-8
: Ensure all imported components are used.The
Button
andSearchInput
components from the design system are used, but theFlex
component should be checked for usage.Verification successful
All imported components are used.
The
Flex
component is utilized in the file, so there are no issues with unused imports.
Flex
component usage:
<Flex alignItems="center" flexDirection="row" gap="spaces-3" px="spaces-3">
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify usage of Flex component in AddAndSearchbar.tsx # Test: Ensure that the Flex component is actually used in the file. rg --type tsx 'Flex' app/client/src/pages/Editor/IDE/EditorPane/components/AddAndSearchbar.tsxLength of output: 122
Script:
#!/bin/bash # Description: Verify usage of Flex component in AddAndSearchbar.tsx # Test: Ensure that the Flex component is actually used in the file. rg 'Flex' app/client/src/pages/Editor/IDE/EditorPane/components/AddAndSearchbar.tsxLength of output: 234
app/client/src/pages/Editor/IDE/EditorPane/utils.ts (2)
1-12
: Ensure all imported types are used.The
EditorSegmentList
type from selectors is used. The FUSE_OPTIONS are correctly defined for fuzzy search.
14-30
: LGTM!The function logic is correct and adheres to best practices.
app/client/src/pages/Editor/IDE/EditorPane/fuzzySearchInFiles.test.ts (2)
1-20
: Ensure all imported constants are used.The
PluginType
constant from entities is used. The sample data is correctly defined for testing.
22-47
: LGTM!The test cases provide sufficient coverage and are correctly defined.
app/client/src/pages/Editor/IDE/EditorPane/Query/List.tsx (2)
Line range hint
1-22
:
Ensure all imported hooks and selectors are used.The imported hooks and selectors are used correctly. The component defines local state and uses selectors to fetch data.
Line range hint
26-110
:
LGTM!The search functionality is correctly implemented and the rendering logic is updated accordingly.
app/client/src/pages/Editor/IDE/EditorPane/JS/List.tsx (4)
37-39
: State and search functionality look good.The addition of
searchTerm
state andonSearch
function is appropriate for the search functionality. The use offuzzySearchInFiles
to filter files based on the search term is correct.Also applies to: 47-48, 56-58
69-75
: Rendering search bar looks good.The use of
AddAndSearchbar
component to render the search bar is appropriate.
Line range hint
89-126
:
Rendering search results looks good.The use of
localFiles
array to render search results and the message for no search results are appropriate.
1-3
: Imports look good, but verify the removal ofButton
.The new imports for
useState
,AddAndSearchbar
, andfuzzySearchInFiles
are necessary for the new search functionality. Ensure that theButton
import removal is consistent with the summary.Also applies to: 21-23
Verification successful
Verification successful:
Button
import removed.The
Button
import has been successfully removed fromapp/client/src/pages/Editor/IDE/EditorPane/JS/List.tsx
, as verified by the absence of this import in the search results.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the removal of `Button` import. # Test: Search for `Button` import. Expect: No occurrences of `Button` import. rg --type-add 'tsx' --type 'tsx' 'import { Button } from'Length of output: 169
Script:
#!/bin/bash # Description: Verify the removal of `Button` import. # Test: Search for `Button` import. Expect: No occurrences of `Button` import. rg --type-add 'tsx' --type 'tsx' 'import { Button } from'Length of output: 169
Script:
#!/bin/bash # Description: Verify the removal of `Button` import. # Test: Search for `Button` import. Expect: No occurrences of `Button` import. rg --type-add 'tsx' --type 'tsx' 'import { Button } from'Length of output: 169
Script:
#!/bin/bash # Description: Verify the removal of `Button` import. # Test: Search for `Button` import. Expect: No occurrences of `Button` import. grep -r 'import { Button } from' .Length of output: 7385
…o ide-tab-list-search-view
<SearchInput onChange={onSearch} size="sm" /> | ||
{hasAddPermission ? ( | ||
<Button | ||
className="t--add-item !min-w-[24px]" |
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.
Use data-testid
for t--add-item
instead of className
.
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.
Will take this up in a separate PR since this change is not introduced by this PR rather this is a change in location. Also, this update will cause updates in some cypress files as well.
…o ide-tab-list-search-view
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: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- app/client/cypress/support/Pages/JSEditor.ts (1 hunks)
Additional context used
Path-based instructions (1)
app/client/cypress/support/Pages/JSEditor.ts (1)
Pattern
app/client/cypress/**/**.*
: Follow best practices for Cypress code and e2e automation.
Avoid using cy.wait in code.
Avoid using cy.pause in code.
Use variables for locators, not strings.
Usedata-*
attributes for selectors; avoid Xpaths and CSS attributes.
Avoid selectors like.btn.submit
orbutton[type=submit]
.
Perform logins via API withLoginFromAPI
.
Only interact with controlled sites/servers.
Ensure tests can run withit.only
and are independent.
Usebefore
,beforeEach
,after
,afterEach
correctly; clean state before tests.
Check new specs for flakiness by running them 10 times on CI.
Use multiple assertions; don't treat Cypress as unit tests.
Use constants for strings.
Include datasource operations inbefore
hooks.
Additional comments not posted (2)
app/client/cypress/support/Pages/JSEditor.ts (2)
108-108
: LGTM! The method callPageLeftPane.switchToAddNew()
is correctly integrated.This change replaces the old method of clicking directly on the new JS object element and improves the clarity and maintainability of the code.
109-109
: LGTM! The old code is appropriately commented out.Commenting out the old direct click method helps maintain the context and provides a fallback if needed.
…o ide-tab-list-search-view
…o ide-tab-list-search-view
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: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- app/client/src/pages/Editor/IDE/EditorPane/JS/List.tsx (6 hunks)
- app/client/src/pages/Editor/IDE/EditorPane/Query/List.tsx (4 hunks)
Files skipped from review as they are similar to previous changes (2)
- app/client/src/pages/Editor/IDE/EditorPane/JS/List.tsx
- app/client/src/pages/Editor/IDE/EditorPane/Query/List.tsx
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: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- app/client/cypress/support/Pages/JSEditor.ts (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- app/client/cypress/support/Pages/JSEditor.ts
…o ide-tab-list-search-view
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: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- app/client/src/pages/Editor/IDE/EditorPane/JS/BlankState.tsx (1 hunks)
- app/client/src/pages/Editor/IDE/EditorPane/Query/BlankState.tsx (1 hunks)
- app/client/src/pages/Editor/IDE/EditorPane/Query/QueryRender.test.tsx (3 hunks)
- app/client/src/pages/Editor/IDE/EditorPane/components/AddAndSearchbar.tsx (1 hunks)
- app/client/src/pages/Editor/IDE/EditorPane/components/EmptyState.tsx (2 hunks)
Files skipped from review due to trivial changes (3)
- app/client/src/pages/Editor/IDE/EditorPane/JS/BlankState.tsx
- app/client/src/pages/Editor/IDE/EditorPane/Query/BlankState.tsx
- app/client/src/pages/Editor/IDE/EditorPane/Query/QueryRender.test.tsx
Files skipped from review as they are similar to previous changes (1)
- app/client/src/pages/Editor/IDE/EditorPane/components/AddAndSearchbar.tsx
Additional comments not posted (2)
app/client/src/pages/Editor/IDE/EditorPane/components/EmptyState.tsx (2)
10-10
: LGTM!The addition of the
buttonTestId
prop to theEmptyStateProps
interface is consistent and straightforward.
15-15
: LGTM!The usage of the
buttonTestId
prop as thedata-testid
attribute for theButton
component enhances testability. This is a good practice for identifying elements in tests.Also applies to: 51-51
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: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- app/client/src/pages/Editor/IDE/EditorPane/JS/JSRender.test.tsx (1 hunks)
Files skipped from review due to trivial changes (1)
- app/client/src/pages/Editor/IDE/EditorPane/JS/JSRender.test.tsx
Description
This PR adds search functionality to the overflow list of tabs.
Fixes #34293
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/9873481673
Commit: c09ae9f
Cypress dashboard.
Tags:
@tag.All
Spec:
Wed, 10 Jul 2024 12:35:26 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Improvements
"t--add-item"
) for improved testability.Bug Fixes
Tests
Chores
design-system
dependency to version2.1.43
.