fix: Revert "feat: Overflow tabs list view" #34613
Conversation
This reverts commit 519b53e.
WalkthroughRecent updates introduce structural improvements and UI modifications in the IDE's tab management and editor pane components. Notably, changes in tab identifiers and removal of unnecessary sanitization enhance code cleanliness and performance. Key updates include refined imports, layout adjustments, and the incorporation of a new Changes
Poem
Tip Early access features: enabledWe are currently testing the following features in early access:
Note:
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 (
|
…o revert-34150-overflow-tabs-list-view
![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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (20)
- app/client/cypress/e2e/Regression/ClientSide/IDE/IDE_Add_Pane_Interactions_spec.ts (1 hunks)
- app/client/cypress/support/Pages/IDE/FileTabs.ts (1 hunks)
- app/client/src/ce/pages/Editor/IDE/EditorPane/JS/hooks.ts (2 hunks)
- app/client/src/ce/pages/Editor/IDE/EditorPane/Query/hooks.tsx (2 hunks)
- app/client/src/ce/selectors/appIDESelectors.ts (2 hunks)
- app/client/src/pages/Editor/IDE/EditorPane/EditorPaneSegments.tsx (2 hunks)
- app/client/src/pages/Editor/IDE/EditorPane/JS/JSRender.test.tsx (4 hunks)
- app/client/src/pages/Editor/IDE/EditorPane/JS/List.tsx (2 hunks)
- app/client/src/pages/Editor/IDE/EditorPane/Query/QueryRender.test.tsx (17 hunks)
- app/client/src/pages/Editor/IDE/EditorTabs/Container.tsx (1 hunks)
- app/client/src/pages/Editor/IDE/EditorTabs/FileTabs.test.tsx (4 hunks)
- app/client/src/pages/Editor/IDE/EditorTabs/FileTabs.tsx (1 hunks)
- app/client/src/pages/Editor/IDE/EditorTabs/FullScreenTabs.tsx (1 hunks)
- app/client/src/pages/Editor/IDE/EditorTabs/ListButton.tsx (2 hunks)
- app/client/src/pages/Editor/IDE/EditorTabs/SearchableFilesList.tsx (2 hunks)
- app/client/src/pages/Editor/IDE/EditorTabs/SplitScreenTabs.tsx (1 hunks)
- app/client/src/pages/Editor/IDE/EditorTabs/StyledComponents.tsx (2 hunks)
- app/client/src/pages/Editor/IDE/MainPane/index.tsx (2 hunks)
- app/client/src/pages/Editor/IDE/hooks.ts (1 hunks)
- app/client/src/utils/URLUtils.ts (1 hunks)
Files not reviewed due to errors (2)
- app/client/cypress/e2e/Regression/ClientSide/IDE/IDE_Add_Pane_Interactions_spec.ts (no review received)
- app/client/src/pages/Editor/IDE/EditorTabs/FileTabs.tsx (no review received)
Files skipped from review due to trivial changes (3)
- app/client/cypress/support/Pages/IDE/FileTabs.ts
- app/client/src/pages/Editor/IDE/EditorPane/JS/JSRender.test.tsx
- app/client/src/pages/Editor/IDE/EditorTabs/Container.tsx
Additional context used
Biome
app/client/src/utils/URLUtils.ts
[error] 21-23: This else clause can be omitted because previous branches break early.
Unsafe fix: Omit the else clause.
(lint/style/noUselessElse)
app/client/src/ce/pages/Editor/IDE/EditorPane/Query/hooks.tsx
[error] 214-214: Avoid redundant double-negation.
It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation(lint/complexity/noExtraBooleanCast)
Additional comments not posted (61)
app/client/src/pages/Editor/IDE/MainPane/index.tsx (2)
6-6: Update the import path forEditorTabs.The import path for
EditorTabshas been updated to useFullScreenTabs.
21-21: Verify the correct usage of the newEditorTabsimport.Ensure that the
FullScreenTabscomponent is correctly implemented and used here.Verification successful
The
FullScreenTabscomponent is correctly implemented and used.
FullScreenTabsis imported and defined correctly.- The component is exported properly.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of `FullScreenTabs`. # Test: Search for the `FullScreenTabs` component. Expect: Correct implementation. rg --type tsx 'FullScreenTabs'Length of output: 58
Script:
#!/bin/bash # Description: Verify the implementation of `FullScreenTabs`. # Find all .tsx files and search for the FullScreenTabs component within those files fd -e tsx -x rg 'FullScreenTabs' {}Length of output: 168
app/client/src/pages/Editor/IDE/EditorTabs/ListButton.tsx (2)
12-12: Update the import path forTabTextContainer.The import path for
TabTextContainerhas been updated to useStyledComponents.
53-53: Verify the correct usage of the newTabTextContainerimport.Ensure that the
TabTextContainercomponent is correctly implemented and used here.Verification successful
The
TabTextContainercomponent is correctly implemented and used.The
TabTextContaineris defined inStyledComponents.tsxand properly imported and used inListButton.tsx.
StyledComponents.tsx:export const TabTextContainer = styled.span` width: 100%; text-overflow: ellipsis; white-space: nowrap; overflow: hidden; `;
ListButton.tsx:import { ListIconContainer, TabTextContainer } from "./StyledComponents"; ... <TabTextContainer>{item.title}</TabTextContainer>Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of `TabTextContainer`. # Test: Search for the `TabTextContainer` component. Expect: Correct implementation. rg --type tsx 'TabTextContainer'Length of output: 60
Script:
#!/bin/bash # Description: Verify the implementation of `TabTextContainer`. # Find all TypeScript and TSX files and search for `TabTextContainer` component. fd -e ts -e tsx -x grep -Hn 'TabTextContainer' {}Length of output: 2092
Script:
#!/bin/bash # Description: Verify the implementation of TabTextContainer in StyledComponents.tsx. # Extract the implementation of TabTextContainer from StyledComponents.tsx. ast-grep --lang tsx --pattern 'export const TabTextContainer = $_' --jsonLength of output: 682
app/client/src/pages/Editor/IDE/EditorPane/EditorPaneSegments.tsx (3)
9-9: Update the import path forEditorTabs.The import path for
EditorTabshas been updated to useSplitScreenTabs.
27-27: Verify the correct usage of thegapprop inFlexcomponent.Ensure that the
gapprop is correctly implemented and used here.Verification successful
The
gapprop is correctly implemented and used in theFlexcomponent.
app/client/src/widgets/wds/WDSStatsWidget/component/index.tsxapp/client/src/pages/Editor/IDE/EditorPane/index.tsxapp/client/src/pages/Editor/IDE/EditorPane/EditorPaneSegments.tsxapp/client/packages/design-system/widgets/src/testing/ComplexForm.tsxapp/client/packages/design-system/widgets/src/components/Icon/stories/Icon.stories.tsxapp/client/packages/design-system/widgets/src/components/TextInput/stories/TextInput.stories.tsxapp/client/packages/design-system/widgets/src/components/Select/stories/Select.stories.tsxapp/client/packages/design-system/widgets/src/components/IconButton/stories/IconButton.stories.tsxapp/client/packages/design-system/widgets/src/components/Flex/stories/Flex.stories.tsxScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of the `gap` prop in `Flex` component. # Test: Search for the `gap` prop usage in `Flex` component. Expect: Correct implementation. rg --type tsx 'gap="spacing-2"'Length of output: 61
Script:
#!/bin/bash # Description: Verify the implementation of the `gap` prop in `Flex` component. # Test: Search for the `gap` prop usage in `Flex` component. Expect: Correct implementation. rg --glob '*.tsx' 'gap="spacing-2"'Length of output: 1756
32-32: Verify the correct usage of the newEditorTabsimport.Ensure that the
SplitScreenTabscomponent is correctly implemented and used here.Verification successful
The
SplitScreenTabscomponent is correctly implemented and imported asEditorTabsinEditorPaneSegments.tsx.
app/client/src/pages/Editor/IDE/EditorTabs/SplitScreenTabs.tsx: Defines and exportsSplitScreenTabs.app/client/src/pages/Editor/IDE/EditorPane/EditorPaneSegments.tsx: ImportsSplitScreenTabsasEditorTabs.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of `SplitScreenTabs`. # Test: Search for the `SplitScreenTabs` component. Expect: Correct implementation. rg --type tsx 'SplitScreenTabs'Length of output: 59
Script:
#!/bin/bash # Description: Verify the implementation of `SplitScreenTabs`. # Test: Search for the `SplitScreenTabs` component in all TypeScript files. rg 'SplitScreenTabs' --type-add 'typescript:*.{ts,tsx}' --type typescriptLength of output: 385
app/client/src/ce/selectors/appIDESelectors.ts (2)
48-59: Ensure correct keying and filtering inselectJSSegmentEditorTabs.The selector
selectJSSegmentEditorTabscorrectly useskeyByandfilter(Boolean)to ensure that only valid items are returned. This looks good, but please verify that allitemshave uniquekeyproperties to avoid any potential conflicts.
61-72: Ensure correct keying and filtering inselectQuerySegmentEditorTabs.Similar to the selector
selectJSSegmentEditorTabs,selectQuerySegmentEditorTabsuseskeyByandfilter(Boolean). Ensure that allitemshave uniquekeyproperties to avoid any potential conflicts.app/client/src/pages/Editor/IDE/EditorTabs/FileTabs.test.tsx (5)
4-4: Verify the necessity ofEntityItemimport.The import of
EntityItemfrom@appsmith/entities/IDE/constantsseems unused. If it is indeed unnecessary, consider removing it.
27-29: Good implementation of the tab rendering test.The test correctly verifies that the
editorTabsContaineris rendered and that each tab is rendered with the correct content.
32-34: Ensure test IDs are unique and descriptive.The test IDs used for tabs should be unique and descriptive to avoid any potential conflicts and to make the tests more readable.
48-50: Good implementation of the tab click test.The test correctly verifies that clicking a tab calls the
mockNavigateToTabfunction with the correct tab.
Line range hint
62-67: Good implementation of the close click test.The test correctly verifies that clicking the close button calls the
mockOnClosefunction with the correct tab key.app/client/src/pages/Editor/IDE/EditorTabs/StyledComponents.tsx (5)
2-2: Verify the necessity ofFleximport.The import of
Flexfromdesign-systemseems necessary for the styled components. Ensure it is used correctly.
4-17: Good documentation for the tab styling logic.The documentation provides a clear explanation of the logic behind the tab styling. This is helpful for future maintenance.
18-55: Good implementation ofStyledTabcomponent.The
StyledTabcomponent is well-defined with appropriate styles for different states (default, active, hover). Ensure that the CSS variables used are defined in the design system.
57-62: Good implementation ofTabTextContainercomponent.The
TabTextContainercomponent correctly handles text overflow and ensures that the text is displayed appropriately within the tab.
64-74: Good implementation ofTabIconContainercomponent.The
TabIconContainercomponent is well-defined with appropriate styles for the icon within the tab.app/client/src/pages/Editor/IDE/EditorTabs/FullScreenTabs.tsx (5)
1-1: Good use ofuseCallbackforsetSplitScreenMode.The
useCallbackhook is correctly used to memoize thesetSplitScreenModefunction, which helps in optimizing performance.
28-33: Ensure analytics event is logged correctly.The analytics event
EDITOR_MODE_CHANGEis logged when the editor mode is changed to split screen. Verify that this event is logged correctly and that the event name is consistent with other event names in the project.
36-36: Verify the selectortabsConfig.tabsSelector.Ensure that the selector
tabsConfig.tabsSelectorcorrectly retrieves the tabs for the current segment.
38-40: Ensure correct conditional rendering logic.The component correctly renders
nullbased on the conditions forisSideBySideEnabled,ideViewMode, andsegment. Verify that these conditions are correct and consistent with the application's requirements.
42-64: Good implementation of theFullScreenTabscomponent.The
FullScreenTabscomponent is well-defined with appropriate rendering logic for the tabs and the minimize button. Ensure that the CSS classes and data-testid attributes used are consistent with the project's conventions.app/client/src/pages/Editor/IDE/EditorTabs/SearchableFilesList.tsx (1)
16-16: LGTM! The import is appropriate.The import of
TabTextContainerfromStyledComponentsaligns with its usage in the file.app/client/src/pages/Editor/IDE/EditorTabs/SplitScreenTabs.tsx (26)
1-1: LGTM! The import is appropriate.The import of
useCallbackfrom "react" aligns with its usage in the file.
2-2: LGTM! The blank line improves readability.The blank line enhances the readability and separation of import statements.
3-3: LGTM! The import is appropriate.The import of
FileTabsfrom "./FileTabs" aligns with its usage in the file.
4-4: LGTM! The import is appropriate.The import of
useDispatchanduseSelectorfrom "react-redux" aligns with their usage in the file.
5-5: LGTM! The import is appropriate.The import of
getIDEViewModeandgetIsSideBySideEnabledfrom "selectors/ideSelectors" aligns with their usage in the file.
6-6: LGTM! The import is appropriate.The import of
Containerfrom "./Container" aligns with its usage in the file.
7-7: LGTM! The import is appropriate.The import of
useCurrentEditorStateanduseIDETabClickHandlersfrom "../hooks" aligns with their usage in the file.
8-11: LGTM! The import is appropriate.The import of
EditorEntityTab,EditorViewModefrom "@appsmith/entities/IDE/constants" aligns with their usage in the file.
12-12: LGTM! The import is appropriate.The import of
TabSelectorsfrom "./constants" aligns with its usage in the file.
13-13: LGTM! The import is appropriate.The import of
Announcementfrom "../EditorPane/components/Announcement" aligns with its usage in the file.
14-14: LGTM! The import is appropriate.The import of
SearchableFilesListfrom "./SearchableFilesList" aligns with its usage in the file.
15-15: LGTM! The import is appropriate.The import of
AddButtonfrom "./AddButton" aligns with its usage in the file.
16-16: LGTM! The import is appropriate.The import of
ButtonandTooltipfrom "design-system" aligns with their usage in the file.
17-20: LGTM! The import is appropriate.The import of
MAXIMIZE_BUTTON_TOOLTIPandcreateMessagefrom "@appsmith/constants/messages" aligns with their usage in the file.
21-21: LGTM! The import is appropriate.The import of
AnalyticsUtilfrom "@appsmith/utils/AnalyticsUtil" aligns with its usage in the file.
22-22: LGTM! The import is appropriate.The import of
setIdeEditorViewModefrom "actions/ideActions" aligns with its usage in the file.
23-23: LGTM! The blank line improves readability.The blank line enhances the readability and separation of import statements.
24-24: LGTM! The component declaration is appropriate.The declaration of the
SplitScreenTabsfunctional component aligns with its usage in the file.
25-25: LGTM! The dispatch declaration is appropriate.The declaration of the
dispatchconstant usinguseDispatchaligns with its usage in the file.
26-26: LGTM! The selector declaration is appropriate.The declaration of the
isSideBySideEnabledconstant usinguseSelectoraligns with its usage in the file.
27-27: LGTM! The selector declaration is appropriate.The declaration of the
ideViewModeconstant usinguseSelectoraligns with its usage in the file.
28-28: LGTM! The state declaration is appropriate.The declaration of the
segmentconstant usinguseCurrentEditorStatealigns with its usage in the file.
29-29: LGTM! The handler declarations are appropriate.The declarations of
closeClickHandlerandtabClickHandlerconstants usinguseIDETabClickHandlersalign with their usage in the file.
30-30: LGTM! The blank line improves readability.The blank line enhances the readability and separation of code blocks.
31-31: LGTM! The configuration declaration is appropriate.The declaration of the
tabsConfigconstant usingTabSelectorsaligns with its usage in the file.
32-32: LGTM! The blank line improves readability.The blank line enhances the readability and separation of code blocks.
app/client/src/ce/pages/Editor/IDE/EditorPane/JS/hooks.ts (1)
98-98: Ensure the correct component is used.The
componentfor the route with the keyJSEmptyhas been changed toListJS. Ensure that this is the intended component and that it doesn't introduce any regression.app/client/src/pages/Editor/IDE/EditorPane/JS/List.tsx (1)
80-80: Ensure consistent styling.The
flexattribute has been added to theFlexcomponent. Ensure that the styling is consistent with the rest of the application.app/client/src/ce/pages/Editor/IDE/EditorPane/Query/hooks.tsx (1)
164-164: Ensure the correct component is used.The
componentfor the route with the keyQueryEmptyhas been changed toListQuery. Ensure that this is the intended component and that it doesn't introduce any regression.app/client/src/pages/Editor/IDE/hooks.ts (1)
217-217: Ensure the correct parameter type is used.The
closeClickHandlerfunction now takes anactionIdparameter of typestring | undefined. Ensure that this change is consistent with the rest of the codebase and doesn't introduce any type-related issues.app/client/src/pages/Editor/IDE/EditorPane/Query/QueryRender.test.tsx (6)
91-91: LGTM!The test correctly checks for the presence of the new tab and the close button in the blank state.
124-124: LGTM!The test correctly checks for the presence of the new tab and the close button in the split screen blank state.
164-164: LGTM!The test correctly checks for the presence of various elements, including the active state of the tabs and the form in full screen mode.
208-208: LGTM!The test correctly checks for the presence of various elements, including the active state of the tabs and the form in split screen mode.
247-247: LGTM!The test correctly checks for the presence of various elements, including the new tab and the close button in full screen mode.
281-281: LGTM!The test correctly checks for the presence of various elements, including the new tab and the close button in split screen mode.
albinAppsmith
changed the title
## Description This reverts commit 519b53e. Fixes #`Issue Number` _or_ Fixes `Issue URL` > [!WARNING] > _If no issue exists, please create an issue first, and check with the maintainers if the issue is valid._ ## Automation /ok-to-test tags="@tag.All" ### 🔍 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/9755650204> > Commit: 3e273b6 > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=9755650204&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.All` <!-- 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 - **Refactor** - Improved tab closing interaction in the IDE pane. - Updated components for better consistency and performance in the Editor. - Renamed and reorganized components for clarity. - **New Features** - Added `FullScreenTabs` component for a more immersive tab management experience. - **Style** - Enhanced styling for tabs and related UI elements to improve user experience. - **Tests** - Refined test cases for better accuracy and reliability. - **Performance** - Enhanced selector functions for better memoization and performance in the IDE. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Description
This reverts commit 519b53e.
Fixes #
Issue Numberor
Fixes
Issue URLWarning
If no issue exists, please create an issue first, and check with the maintainers if the issue is valid.
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/9755650204
Commit: 3e273b6
Cypress dashboard.
Tags:
@tag.AllCommunication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
Refactor
New Features
FullScreenTabscomponent for a more immersive tab management experience.Style
Tests
Performance