feat: Adds a status indicator setting in List Item and Tab#39938
feat: Adds a status indicator setting in List Item and Tab#39938
Conversation
WalkthroughThis pull request introduces a mechanism to indicate unsaved changes and add dynamic line highlighting in several components. It adds a new property Changes
Sequence Diagram(s)sequenceDiagram
participant UIComponent as UI Component
participant Selector as Dirty State Selector
participant Badge as Badge Component
participant Wrapper as UnsavedChangesWrapper
UIComponent->>Selector: Retrieve dirty state
Selector-->>UIComponent: Return (true/false)
UIComponent->>Wrapper: Pass showUnsavedChanges prop
Wrapper->>Badge: Conditionally render Badge if true
sequenceDiagram
participant CodeEditor as CodeEditor
participant Util as getHighlightedLines Utility
participant LazyEditor as LazyCodeEditor
CodeEditor->>Util: getHighlightedLines(collection)
Util-->>CodeEditor: Return highlighted lines (array)
CodeEditor->>LazyEditor: Update with highlightedLines prop
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code Definitions (1)app/client/packages/design-system/ads/src/Badge/Badge.styles.tsx (1)
⏰ Context from checks skipped due to timeout of 90000ms (8)
🔇 Additional comments (5)
🪧 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
|
![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 (4)
app/client/src/components/editorComponents/CodeEditor/index.tsx (1)
1650-1662: Update line highlighting implementation could be optimized.While the implementation works correctly, it might be inefficient for files with many lines since it clears and re-applies all highlights on every update.
Consider optimizing this implementation to only update lines that have changed:
private updateLineHighlighting = (lines: number[]) => { - // Clear existing highlights - for (let i = 0; i < this.editor.lineCount(); i++) { - this.editor.removeLineClass(i, "background", "highlighted-line"); - } - - // Add new highlights - lines.forEach((lineNumber) => { - if (lineNumber >= 0 && lineNumber < this.editor.lineCount()) { - this.editor.addLineClass(lineNumber, "background", "highlighted-line"); - } - }); + const lineCount = this.editor.lineCount(); + const lineSet = new Set(lines); + + // Update highlighting efficiently + for (let i = 0; i < lineCount; i++) { + if (lineSet.has(i) && i < lineCount) { + this.editor.addLineClass(i, "background", "highlighted-line"); + } else { + this.editor.removeLineClass(i, "background", "highlighted-line"); + } + } };Also, ensure there's a corresponding CSS rule for the
highlighted-lineclass in your stylesheet.app/client/src/ce/selectors/entitiesSelector.ts (3)
1762-1775: Improve handling of undefined isDirtyMap.The selector works well for determining if an action's schema generation state is dirty, but it could be more explicit in handling the undefined case.
export const getActionSchemaDirtyState = createSelector( getAction, (state: AppState) => getPluginByPackageName(state, PluginPackageName.APPSMITH_AI), (action, agentPlugin) => { if (!action) return false; if (agentPlugin?.id === action.pluginId) { return false; } - return action.isDirtyMap?.SCHEMA_GENERATION; + return action.isDirtyMap ? !!action.isDirtyMap.SCHEMA_GENERATION : false; }, );
1777-1787: Improve handling of undefined isDirtyMap in collection selector.Similar to the previous selector, this could more explicitly handle undefined cases.
export const getJSCollectionSchemaDirtyState = createSelector( (state: AppState, collectionId: string) => getJSCollection(state, collectionId), (jsCollection) => { if (!jsCollection) return false; return jsCollection.actions.some( - (action) => action.isDirtyMap?.SCHEMA_GENERATION, + (action) => action.isDirtyMap && !!action.isDirtyMap.SCHEMA_GENERATION, ); }, );
1789-1797: Improve handling of undefined isDirtyMap in action selector.Make the handling of undefined isDirtyMap more explicit in this selector too.
export const getJSCollectionActionSchemaDirtyState = createSelector( (state: AppState, collectionId: string, actionId: string) => getJSCollectionAction(state, collectionId, actionId), (action) => { if (!action) return false; - return action.isDirtyMap?.SCHEMA_GENERATION; + return action.isDirtyMap ? !!action.isDirtyMap.SCHEMA_GENERATION : false; }, );
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
app/client/packages/design-system/ads/src/List/List.tsx(3 hunks)app/client/packages/design-system/ads/src/List/List.types.tsx(1 hunks)app/client/packages/design-system/ads/src/Templates/EditableDismissibleTab/EditableDismissibleTab.tsx(3 hunks)app/client/packages/design-system/ads/src/Templates/EditableDismissibleTab/EditableDismissibleTab.types.ts(1 hunks)app/client/packages/design-system/ads/src/Templates/EditableEntityName/EditableEntityName.tsx(0 hunks)app/client/packages/design-system/ads/src/Templates/EntityExplorer/EntityItem/EntityItem.types.ts(0 hunks)app/client/src/ce/pages/Editor/JSEditor/utils/getHighlightedLines.ts(1 hunks)app/client/src/ce/selectors/entitiesSelector.ts(2 hunks)app/client/src/components/editorComponents/CodeEditor/index.tsx(5 hunks)app/client/src/components/editorComponents/CodeEditor/styledComponents.ts(1 hunks)app/client/src/ee/pages/Editor/JSEditor/utils/getHighlightedLines.ts(1 hunks)app/client/src/pages/AppIDE/components/JSEntityItem/JSEntityItem.tsx(3 hunks)app/client/src/pages/AppIDE/components/QueryEntityItem/QueryEntityItem.tsx(3 hunks)app/client/src/pages/AppIDE/layouts/components/EditorTabs/EditableTab.tsx(3 hunks)app/client/src/pages/Editor/JSEditor/Form.tsx(3 hunks)app/client/src/pages/Editor/JSEditor/JSEditorForm/JSEditorForm.tsx(2 hunks)app/client/src/pages/Editor/JSEditor/index.tsx(0 hunks)
💤 Files with no reviewable changes (3)
- app/client/packages/design-system/ads/src/Templates/EntityExplorer/EntityItem/EntityItem.types.ts
- app/client/packages/design-system/ads/src/Templates/EditableEntityName/EditableEntityName.tsx
- app/client/src/pages/Editor/JSEditor/index.tsx
🧰 Additional context used
🧬 Code Definitions (3)
app/client/src/pages/AppIDE/components/JSEntityItem/JSEntityItem.tsx (1)
app/client/src/ce/selectors/entitiesSelector.ts (1)
getJSCollectionSchemaDirtyState(1777-1787)
app/client/src/pages/AppIDE/components/QueryEntityItem/QueryEntityItem.tsx (1)
app/client/src/ce/selectors/entitiesSelector.ts (1)
getActionSchemaDirtyState(1762-1775)
app/client/src/pages/AppIDE/layouts/components/EditorTabs/EditableTab.tsx (1)
app/client/src/ce/selectors/entitiesSelector.ts (2)
getJSCollectionSchemaDirtyState(1777-1787)getActionSchemaDirtyState(1762-1775)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- 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-build / client-build
- GitHub Check: client-prettier / prettier-check
- GitHub Check: chromatic-deployment
- GitHub Check: storybook-tests
- GitHub Check: chromatic-deployment
🔇 Additional comments (31)
app/client/src/ee/pages/Editor/JSEditor/utils/getHighlightedLines.ts (1)
1-2: LGTM - Clean export implementationThis correctly re-exports the Community Edition implementation of
getHighlightedLines, following the project's standard pattern for EE/CE code organization.app/client/src/components/editorComponents/CodeEditor/styledComponents.ts (1)
260-262: Good styling for highlighted linesThe implementation uses an appropriate semi-transparent yellow background that should provide good visibility without being too distracting.
app/client/src/ce/pages/Editor/JSEditor/utils/getHighlightedLines.ts (1)
1-7: Placeholder implementation looks goodThe CE implementation correctly returns an empty array to indicate no highlighted lines by default. The ESLint disable comment is appropriate since the parameter is intentionally unused in this implementation.
app/client/packages/design-system/ads/src/Templates/EditableDismissibleTab/EditableDismissibleTab.types.ts (1)
31-32: Well-documented property additionThe new
showUnsavedChangesproperty is properly typed as an optional boolean with clear JSDoc documentation. This maintains backward compatibility with existing implementations.app/client/packages/design-system/ads/src/Templates/EditableDismissibleTab/EditableDismissibleTab.tsx (3)
10-10: Import added for new status badge component.The Badge import is correctly added to support the new status indicator feature.
26-26: New prop added for indicating unsaved changes.The
showUnsavedChangesprop is properly destructured from props to support the status indicator feature.
65-65: Conditional rendering of warning badge implemented.Clean implementation using a ternary operator to conditionally render the warning badge only when there are unsaved changes.
app/client/src/pages/Editor/JSEditor/JSEditorForm/JSEditorForm.tsx (2)
19-19: Added optional property for highlighted lines.The
highlightedLinesproperty is correctly defined as an optional array of numbers with the proper TypeScript syntax.
48-48: Passing highlightedLines prop to LazyCodeEditor.The prop is correctly passed to the LazyCodeEditor component to support visual highlighting of specific code lines.
app/client/packages/design-system/ads/src/List/List.tsx (3)
29-29: Import added for Badge component.The Badge import is correctly added to support the status indicator feature in list items.
90-90: New showUnsavedChanges prop added to ListItem.This prop is properly added to the component properties to support the status indicator feature.
164-164: Conditional rendering of warning badge in ListItem.The implementation correctly renders the warning badge only when showUnsavedChanges is true, placed appropriately within the TopContentWrapper.
app/client/src/pages/AppIDE/components/JSEntityItem/JSEntityItem.tsx (3)
4-7: Updated imports to include state selector.The import statement is properly restructured to include the new selector for checking JS collection schema dirty state.
111-113: Added selector to detect unsaved changes in JS actions.Good implementation using Redux's useSelector to determine if the JS collection schema has unsaved changes.
128-128: Passing dirty state to EntityItem component.The component correctly passes the schema dirty state to the EntityItem via the showUnsavedChanges prop, enabling visual indication of unsaved changes.
app/client/packages/design-system/ads/src/List/List.types.tsx (1)
41-42: LGTM! Well-documented property addition.The new
showUnsavedChangesproperty is properly documented and follows the interface's existing conventions. The optional nature ensures backward compatibility.app/client/src/pages/AppIDE/components/QueryEntityItem/QueryEntityItem.tsx (3)
5-9: Import statement looks good.The
getActionSchemaDirtyStateselector is properly imported and grouped with related selectors.
121-123: Good implementation of the selector for tracking schema changes.The selector is properly used with
useSelectorto retrieve the dirty state of the action schema.
136-136: Good use of the new prop to display unsaved changes.The
showUnsavedChangesprop is correctly passed to theEntityItemcomponent to indicate when there are unsaved changes.app/client/src/pages/Editor/JSEditor/Form.tsx (3)
60-60: Good import for line highlighting functionality.The
getHighlightedLinesfunction is properly imported to enable line highlighting in the editor.
291-291: Good implementation of line highlighting.The
getHighlightedLinesfunction is used withcurrentJSCollectionto determine which lines should be highlighted in the editor.
379-379: Proper prop passing for highlighted lines.The
highlightedLinesvalue is correctly passed to theEditorFormcomponent to enable visual line highlighting.app/client/src/pages/AppIDE/layouts/components/EditorTabs/EditableTab.tsx (5)
9-13: Good organization of imports.The imports have been properly grouped and organized to include the necessary selectors for checking schema dirty states.
68-70: Good implementation of JS schema dirty state tracking.The selector is properly used with
useSelectorto retrieve the dirty state of JavaScript collection schemas.
72-74: Good implementation of action schema dirty state tracking.The selector is properly used with
useSelectorto retrieve the dirty state of action schemas.
76-76: Effective combination of dirty states.The logical OR operator is appropriately used to determine if either schema has unsaved changes.
92-92: Good use of the new prop to display unsaved changes in tabs.The
showUnsavedChangesprop is correctly passed to theEditableDismissibleTabcomponent to indicate when there are unsaved changes.app/client/src/components/editorComponents/CodeEditor/index.tsx (3)
263-263: LGTM, well-typed property for line highlighting.The new
highlightedLinesoptional property is properly documented and typed as an array of numbers, making the intention clear.
504-508: Good handling of initial highlighted lines.This code properly checks for the existence of
highlightedLineswith optional chaining before applying the highlighting on initial render.
622-625: Clean implementation of highlighted lines update.The code correctly:
- Uses deep equality check to prevent unnecessary updates
- Handles the undefined case with empty array fallback
app/client/src/ce/selectors/entitiesSelector.ts (1)
457-461: Good use of flatMap for collecting actions.This selector effectively flattens all JS collection actions into a single array, which is cleaner than nesting loops.
…rg#39938) ## Description Adds ability across UI elements to show warning indicators ## Automation /ok-to-test tags="@tag.Sanity" ### 🔍 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/14083540998> > Commit: 022e693 > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=14083540998&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.Sanity` > Spec: > <hr>Wed, 26 Mar 2025 13:24:06 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 - **New Features** - Added visual indicators for unsaved changes across list and tab components—badges now appear to alert users of pending modifications. - Enhanced the code editor by introducing dynamic line highlighting, allowing users to easily identify and navigate to specific code sections. - Introduced new selectors to improve management of JavaScript collection actions and their schema states. - **Style** - Upgraded badge styling with a new "info" variant and adjustable size options, resulting in a more refined and consistent user experience. - Added a new styled component for unsaved changes, enhancing visual feedback for users. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
2 participants
Description
Adds ability across UI elements to show warning indicators
Automation
/ok-to-test tags="@tag.Sanity"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/14083540998
Commit: 022e693
Cypress dashboard.
Tags:
@tag.SanitySpec:
Wed, 26 Mar 2025 13:24:06 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Style