feat: generate documentation for selected columns and doc editor updates #1525
Conversation
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
webview_panels/src/modules/documentationEditor/DocumentationEditor.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-react". (The package "eslint-plugin-react" was not found when loaded as a Node module from the directory "/webview_panels".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-react" was referenced from the config file in "webview_panels/.eslintrc.cjs". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. WalkthroughThis pull request introduces significant changes to the webview panels, focusing on enhancing the documentation editor's functionality. The modifications include restructuring state management by removing page selection logic, introducing a search query feature, and refactoring various components to improve user interaction. Key changes involve updating the Changes
Possibly related PRs
Suggested reviewers
🪧 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
CodeRabbit Configuration File (
|
saravmajestic
changed the title
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
❌ Changes requested. Reviewed everything up to cd7526c in 1 minute and 51 seconds
More details
- Looked at
1624lines of code in24files - Skipped
0files when reviewing. - Skipped posting
12drafted comments based on config settings.
1. src/webview_provider/docsEditPanel.ts:955
- Draft comment:
Ensure thatthis.documentationis correctly updated before this line to avoid sending stale data to the webview. This change adds the documentation to the response, which is necessary for keeping the webview in sync. - Reason this comment was not posted:
Confidence changes required:20%
The code indocsEditPanel.tsis responsible for handling the documentation editing panel in a VSCode extension. The change in line 955 addsdocumentation: this.documentationto the message body when saving documentation. This ensures that the updated documentation is sent back to the webview after saving. This change seems correct and aligns with the intent to update the webview with the latest documentation state.
2. webview_panels/.storybook/preview-head.html:12
- Draft comment:
Ensure that the CSS variables for--vscode-panel-backgroundare correctly defined in the rest of the application to maintain consistent theming. - Reason this comment was not posted:
Confidence changes required:10%
The CSS changes inpreview-head.htmlare related to setting the background color for different themes. This is a common practice to ensure the UI matches the VSCode theme. The changes are straightforward and do not introduce any issues.
3. webview_panels/src/modules/commonActionButtons/CommonActionButtons.tsx:62
- Draft comment:
Consider using a switch statement foractionto improve readability if more actions are added in the future. - Reason this comment was not posted:
Confidence changes required:10%
InCommonActionButtons.tsx, the use ofPopoverWithButtonto group action buttons is a good approach to declutter the UI. The logic for setting actions is clear and concise.
4. webview_panels/src/modules/commonActionButtons/FeedbackButton.tsx:29
- Draft comment:
Ensure thatbuttonPropsdoes not override critical props likeonClickunintentionally. Consider explicitly spreadingbuttonPropsbefore definingonClick. - Reason this comment was not posted:
Confidence changes required:20%
TheFeedbackButtoncomponent usessendTelemetryEventandvscode.postMessageto handle feedback actions. The use ofbuttonPropsallows for flexible button styling, which is a good practice.
5. webview_panels/src/modules/commonActionButtons/HelpButton.tsx:33
- Draft comment:
Consider adding a dependency array to theuseEffecthook to ensure it only runs on mount and unmount. - Reason this comment was not posted:
Confidence changes required:10%
TheHelpButtoncomponent uses aDrawerto display help content. The use ofuseEffectto open the drawer on mount is appropriate for this use case.
6. webview_panels/src/modules/documentationEditor/DocumentationEditor.tsx:116
- Draft comment:
Ensure that theSearchColumnsInputcomponent correctly updates the state to filter columns as expected. - Reason this comment was not posted:
Confidence changes required:10%
InDocumentationEditor.tsx, the removal of theselectedPageslogic simplifies the component. The addition ofSearchColumnsInputandBulkGenerateButtonaligns with the PR's goal to improve usability.
7. webview_panels/src/modules/documentationEditor/components/conversation/ShowConversationsButton.tsx:13
- Draft comment:
Ensure thatonCloseis always provided to avoid potential runtime errors. - Reason this comment was not posted:
Confidence changes required:10%
TheShowConversationsButtoncomponent dispatches an action to update the state. TheonClosecallback is used to close the popover, which is a good practice for managing UI state.
8. webview_panels/src/modules/documentationEditor/components/docGenerator/BulkGenerateButton.tsx:162
- Draft comment:
Consider adding error handling for thebulkGenerateDocsfunction to provide user feedback in case of failures. - Reason this comment was not posted:
Confidence changes required:20%
TheBulkGenerateButtoncomponent uses aPopoverWithButtonto display options for bulk generation. The logic for handling different options is clear and well-structured.
9. src/webview_provider/docsEditPanel.ts:952
- Draft comment:
Use semantic versioning (semver) for version comparisons instead of direct array index comparisons. ReplacedbtVersion.join(".")with a semver comparison. (Also applicable in other parts of the code where version comparison is done.) - Reason this comment was not posted:
Comment was not on a valid diff hunk.
10. webview_panels/src/modules/documentationEditor/components/docGenerator/BulkGenerateButton.tsx:120
- Draft comment:
Please provide a return type for theonOptionSelectfunction to ensure easier refactoring and better readability. - Reason this comment was not posted:
Comment was on unchanged code.
11. webview_panels/src/modules/documentationEditor/components/docGenerator/DocGenSelectedColumns.tsx:55
- Draft comment:
Please provide a return type for thehandleGeneratefunction to ensure easier refactoring and better readability. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
While the rule about return types is in the additional rules, it specifically mentions "utility functions". This handleGenerate is an event handler within a React component, not a utility function. Event handlers commonly don't have explicit return types. The TypeScript compiler can easily infer the Promise return type here.
The rule about return types could be interpreted more broadly to include all functions. Also, being explicit about return types generally improves code clarity.
The rule specifically calls out utility functions, and this is clearly an event handler. Adding return types to every event handler would add unnecessary verbosity without significant benefit.
The comment should be deleted as it applies a rule meant for utility functions to an event handler where the return type is obvious and not critical for refactoring.
12. webview_panels/src/modules/documentationEditor/components/docGenerator/DocGenSelectedColumns.tsx:72
- Draft comment:
Please provide a return type for thehandleFilterChangefunction to ensure easier refactoring and better readability. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
While the rule mentions return types for utility functions, this is actually an event handler, not a utility function. Event handlers typically don't return values and their void return type is usually implicit in React. Adding a return type here wouldn't provide meaningful value for refactoring since it's a simple event handler that just calls setState.
The rule about return types could be interpreted to apply to all functions, not just utility functions. Also, being explicit about return types can help with documentation.
The rule specifically mentions utility functions, and this is clearly an event handler. Adding void return types to every React event handler would add noise without providing value.
The comment should be deleted as it applies a rule meant for utility functions to a React event handler where the return type is implicitly void.
Workflow ID: wflow_mmPsjZXV0KO7N3oq
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
| const options = [ | ||
| { label: "Generate all", value: "all" }, | ||
| { label: "Generate only missing columns", value: "missing" }, | ||
| ]; | ||
|
|
||
| const bulkGenerateDocs = async ( |
There was a problem hiding this comment.
Please provide a return type for the bulkGenerateDocs function to ensure easier refactoring and better readability.
![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
🧹 Nitpick comments (18)
webview_panels/src/modules/documentationEditor/components/docGenerator/DocGeneratorColumn.tsx (1)
105-106: Citations and CoachAiIfModified usage
Displaying<Citations>and<CoachAiIfModified>alongside the doc generator provides extra context and AI suggestions. Ensure the underlying logic gracefully handles empty or null citation data.webview_panels/src/modules/documentationEditor/components/docGenerator/DocGeneratorColumnsList.tsx (2)
35-41: Filtering logic is well-structured but consider trimming whitespace
The memoized filter function is concise. You might want to trim user input (searchQuery.trim().toLowerCase()) to handle extra spaces before or after the keyword.- column.name.toLowerCase().includes(searchQuery.toLowerCase()), + const query = searchQuery.trim().toLowerCase(); + column.name.toLowerCase().includes(query),
52-55: User caution alert is beneficial
Showing a warning is a good way of preventing accidental overwrites. If you find users need more contextual guidance, consider linking to relevant docs or a one-click rollback button to revert undesired changes.webview_panels/src/modules/documentationEditor/components/docGenerator/BulkGenerateButton.tsx (1)
124-147: Option selection logic is robust.
The switch statement is easy to follow, with good error logging in the catch block. Consider adding user-facing feedback if an error occurs.webview_panels/src/modules/documentationEditor/components/search/SearchColumnsInput.tsx (1)
1-6: New search component is coherent and user-friendly.
Consider implementing a small debounce inhandleChangeto prevent excessive dispatch calls on fast typing.Also applies to: 7-9, 10-12, 14-26
webview_panels/src/uiCore/components/button/Button.tsx (2)
7-7: Add documentation for new prop.
TheshowTextAlwaysprop is self-explanatory, but adding a concise comment or docstring would help other developers understand its significance and usage.
12-12: Consider default behavior forshowTextAlways.
WhenshowTextAlwaysis set totrue, you override the hover logic. Ensure that this behavior is clearly documented, or consider applying a more descriptive name (e.g.,alwaysShowLabel) for added clarity.Also applies to: 35-35
webview_panels/src/modules/commonActionButtons/HelpButton.tsx (1)
8-11: Use descriptive enum labels if possible.
WhileDOCUMENTATIONandTESTSare sufficiently descriptive, some teams prefer longer labels or doc comments for clarity. Consider adding brief doc comments for each field in case the enum grows in the future.webview_panels/src/modules/commonActionButtons/CommonActionButtons.tsx (1)
62-63: Side-by-side rendering of settings and help.
Conditionally renderingDocGeneratorSettingsandHelpButtonmaintains a clean UI. Ensure they don’t overlap or create unexpected layout changes when toggled rapidly.webview_panels/src/modules/documentationEditor/components/settings/DocGeneratorSettings.tsx (1)
23-25: Automatic drawer opening.
CallingdrawerRef.current?.open()insideuseEffectensures the drawer is immediately shown.Consider adding a conditional check or a component-level prop to allow toggling whether the drawer should always auto-open.
webview_panels/src/modules/documentationEditor/components/saveDocumentation/SaveDocumentation.tsx (2)
17-17: Remove or replace 'noop' if not intentional.
noopmight be a placeholder for an onClick, but consider removing or replacing it with a semantic function if you plan to handle a real action or event in the future.
49-49: Document the shape of 'documentation' if needed.
Includingdocumentation: DBTDocumentationin the result is helpful. Consider adding or verifying a JSDoc or type doc reference to clarify the possible fields withinDBTDocumentation.webview_panels/src/modules/documentationEditor/DocumentationEditor.tsx (1)
116-116: SearchColumnsInput positioning.
Placing the search bar to the left is logical. Ensure the input width is suitable for user queries, especially for longer column names.webview_panels/src/modules/documentationEditor/components/docGenerator/DocGenSelectedColumns.tsx (1)
110-172: Drawer-based UI for bulk generation.
The layout is readable, with minimal nesting. Consider adding a dynamic title or subheader inside the drawer if users need additional instructions.webview_panels/src/uiCore/components/dropdownButton/styles.module.scss (1)
2-2: Avoid relying on!importantif possible.Using
!importantmight introduce specificity conflicts later on. Limit its usage if you can handle the styling specificity in another way (e.g., by restructuring your class hierarchy or utilizing more specific selectors).webview_panels/src/modules/documentationEditor/styles.module.scss (3)
Line range hint
258-301: Avoid using!importantin CSS rules.The use of
!importanton line 260 should be avoided as it makes styles harder to maintain and override when needed. Consider these alternatives:
- Increase specificity using parent selectors
- Modify the base styles in Bootstrap's popover configuration
.bulk_gen_popover { :global .popover-body{ - padding: 0 1rem !important; + padding: 0 1rem; } // If specificity is needed, use: // &:global(.bulk_gen_popover) .popover-body { // padding: 0 1rem; // } }
303-321: Use CSS variables for colors instead of hardcoded values.For better theme support and consistency, replace hardcoded colors with CSS variables.
.docsIcon { path { - fill: #198754; + fill: var(--success-color); // or appropriate semantic color variable } } .docsIconInBtn { path { - fill: #fff; + fill: var(--text-color--inverse); // or appropriate semantic color variable } }
336-355: Remove duplicate background property.The
backgroundproperty is defined twice for the file path input.:global #file_path { border-radius: 2px; border: 1px solid var(--stroke--disable, #424750); background: var(--background--02); margin-bottom: 0; flex: 1; font-size: 1rem; padding: 8px 12px; - background: var(--background--02); color: var(--text-color--title); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
src/webview_provider/docsEditPanel.ts(1 hunks)webview_panels/.storybook/preview-head.html(1 hunks)webview_panels/src/modules/commonActionButtons/CommonActionButtons.tsx(1 hunks)webview_panels/src/modules/commonActionButtons/FeedbackButton.tsx(1 hunks)webview_panels/src/modules/commonActionButtons/HelpButton.tsx(2 hunks)webview_panels/src/modules/documentationEditor/DocumentationEditor.tsx(2 hunks)webview_panels/src/modules/documentationEditor/components/conversation/ShowConversationsButton.tsx(1 hunks)webview_panels/src/modules/documentationEditor/components/docGenerator/BulkGenerateButton.tsx(2 hunks)webview_panels/src/modules/documentationEditor/components/docGenerator/DocGenSelectedColumns.tsx(1 hunks)webview_panels/src/modules/documentationEditor/components/docGenerator/DocGeneratorColumn.tsx(2 hunks)webview_panels/src/modules/documentationEditor/components/docGenerator/DocGeneratorColumnsList.tsx(3 hunks)webview_panels/src/modules/documentationEditor/components/saveDocumentation/SaveDocumentation.tsx(3 hunks)webview_panels/src/modules/documentationEditor/components/search/SearchColumnsInput.tsx(1 hunks)webview_panels/src/modules/documentationEditor/components/settings/DocGeneratorSettings.tsx(3 hunks)webview_panels/src/modules/documentationEditor/components/tests/EntityWithTests.tsx(1 hunks)webview_panels/src/modules/documentationEditor/state/documentationSlice.ts(2 hunks)webview_panels/src/modules/documentationEditor/state/types.ts(1 hunks)webview_panels/src/modules/documentationEditor/styles.module.scss(2 hunks)webview_panels/src/testUtils/documentation/index.ts(1 hunks)webview_panels/src/uiCore/components/button/Button.tsx(2 hunks)webview_panels/src/uiCore/components/dropdownButton/DropdownButton.tsx(1 hunks)webview_panels/src/uiCore/components/dropdownButton/styles.module.scss(1 hunks)webview_panels/src/uiCore/components/popoverWithButton/PopoverWithButton.tsx(2 hunks)webview_panels/src/uiCore/index.ts(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
webview_panels/src/modules/documentationEditor/components/docGenerator/BulkGenerateButton.tsx
[error] 177-192: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
🔇 Additional comments (70)
webview_panels/src/modules/documentationEditor/components/docGenerator/DocGeneratorColumn.tsx (3)
29-30: Streamlined destructuring without selectedPages logic
Removing selectedPages and focusing on { currentDocsData, project } aligns with the updated approach, simplifying state management. This change looks good.
32-32: Minor code style change
An empty line has been added for readability. No action needed.
92-98: Unconditional rendering of DocGeneratorInput
Always displaying <DocGeneratorInput> helps maintain a consistent user experience. Consider verifying if conditional rendering is needed for specialized scenarios. Otherwise, it’s good to keep it unconditional if it’s always relevant to the flow.
webview_panels/src/modules/documentationEditor/components/docGenerator/DocGeneratorColumnsList.tsx (3)
7-7: Importing DBTModelTest Looks Good
This explicit type import indicates that you're following best practices by using the right interface/type for column tests.
63-65: Improved key for DocGeneratorColumn
Appending the column.type in the component key helps avoid potential collisions with identically named columns. If columns have unique IDs, you might also consider those for even more consistency.
11-11: State slice refactor aligns with the new search approach
Replacing selectedPages with searchQuery indicates a pivot toward a single UI state for column filtering. This is a coherent approach that simplifies logic. Ensure you’ve updated any references to the old property in other files.
✅ Verification successful
The selectedPages to searchQuery refactor is safe and isolated
The code inspection shows that the selectedPages references found in the codebase (HelpButton.tsx) are unrelated to this component's state management. They refer to a different feature for help page navigation. The DocGeneratorColumnsList component correctly implements column filtering using the new searchQuery state, and there are no lingering references to the old property that need updating.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify references to 'selectedPages' if any still exist in the codebase
rg -A 3 "selectedPages"
Length of output: 23
Script:
#!/bin/bash
# Let's try a case-insensitive search and also look for kebab-case variant
rg -i "selected.?pages?"
# Let's also check the specific file and its imports for context
cat "webview_panels/src/modules/documentationEditor/components/docGenerator/DocGeneratorColumnsList.tsx"
Length of output: 3266
webview_panels/src/modules/documentationEditor/components/docGenerator/BulkGenerateButton.tsx (9)
10-10: No issues with the updated import.
The new usage of PopoverWithButton is aligned with the enhanced UI flow.
14-14: Import statement looks good.
DocGenSelectedColumns is correctly imported for side-panel usage.
15-15: Ensure noop usage is intentional.
Currently, noop does nothing. Confirm it’s used only as a placeholder.
17-19: Enum introduction is straightforward.
Defining SidePanelState improves type safety and clarity.
22-24: State initialization is well-structured.
Using a union type of SidePanelState | undefined is clear for panel control.
27-32: Destructuring from context is appropriate.
Makes the code more readable and ensures easy access to relevant state values.
36-38: Reset function looks good.
Setting sidePanelState to undefined is sufficient for closing the side panel.
40-46: Creating options for documentation generation is clear.
The currently commented-out "Tests" section leaves room for future expansion.
151-155: Conditional color assignment is intuitive.
Switching between "secondary" and "primary" based on generation status is user-friendly.
webview_panels/src/modules/documentationEditor/components/conversation/ShowConversationsButton.tsx (1)
6-10: Prop usage and button logic are consistent.
Calling onClose before changing panel state is logical if you must close popovers beforehand.
Also applies to: 13-13, 17-18
webview_panels/src/uiCore/components/dropdownButton/DropdownButton.tsx (1)
16-19: Flexible color prop for IconButton.
Providing a default color ("primary") if none is specified makes the component more reusable.
webview_panels/src/modules/commonActionButtons/FeedbackButton.tsx (1)
7-15: Optional onClose and buttonProps are well-integrated.
The code effectively uses them for extended customization and consistent closure logic.
Also applies to: 17-17, 25-30
webview_panels/src/uiCore/index.ts (1)
47-47: Exporting types from re-exporting modules.
Exporting ButtonProps improves type visibility for downstream consumers. Verify that the rest of the library correctly references ButtonProps through this re-export to avoid confusion.
webview_panels/src/modules/commonActionButtons/HelpButton.tsx (3)
4-4: Ensure consistent import usage.
All core UI components are imported from @uicore within other files, which is consistent. This line looks good and follows the established pattern.
15-15: Review default drawer behavior.
drawerRef.current?.open() will open the drawer on mount. Ensure that automatically showing help content is desired for all user workflows. If not, consider a conditional or user-initiated approach.
Also applies to: 17-19
33-33: Seamless user experience with the Drawer.
Rendering the Drawer with ref allows external triggers, ensuring a smoother user experience. Good implementation!
webview_panels/src/modules/commonActionButtons/CommonActionButtons.tsx (3)
2-2: Imports are well-organized and minimal.
Importing only the necessary icons and hooks helps keep the bundle small. This is a good practice.
Also applies to: 6-7
9-12: Enum usage for better readability.
Defining SelectedAction as an enum helps clarify button behaviors. The code remains maintainable if new actions are added later.
14-14: Confirm the popover width and usage.
The PopoverWithButton approach is clean and modular. Just ensure that the width="auto" setting accommodates all button texts and doesn’t lead to overflow in smaller viewports.
Also applies to: 18-60
webview_panels/src/modules/documentationEditor/components/settings/DocGeneratorSettings.tsx (4)
1-1: Good introduction of DrawerRef.
Importing DrawerRef lays the groundwork for controlling the Drawer component imperatively.
14-14: Efficient usage of React’s hooks.
Introducing useEffect and useRef is a clean approach to manage the drawer’s open state on mount.
21-21: Ref for Drawer is well-structured.
Storing the drawer reference here simplifies interaction with imperative methods like open and close.
53-53: Prop adjustments for Drawer component.
Ref forwarding simplifies invocation of open/close methods; removing the button props declutters the interface.
webview_panels/src/modules/documentationEditor/components/tests/EntityWithTests.tsx (1)
3-3: Neat import usage.
Importing the refined DBTModelTest type directly avoids extraneous context references.
webview_panels/src/modules/documentationEditor/state/types.ts (1)
84-84: Introduction of the searchQuery property.
Replacing page-based selection with a searchQuery aligns with the new search-driven approach.
webview_panels/src/testUtils/documentation/index.ts (2)
35-35: Expanded column list for tests.
Increasing columns from 10 to 20 can yield broader coverage in test scenarios.
39-39: Dynamically assigning patchPath.
Using a random file path varies test data, improving realism and coverage.
webview_panels/src/modules/documentationEditor/components/saveDocumentation/SaveDocumentation.tsx (9)
3-9: Consolidate UI imports into a single place.
These new imports from @uicore are fine, but consider grouping them with the same import style for consistency and readability, especially if there are more related components coming in the future.
14-14: Confirm that updateCurrentDocsData is being used in all relevant places.
This action is crucial to persist new or updated doc states. Ensure no call sites or edge cases were missed, especially where columns or doc tests might also need updating.
20-27: Clarify usage in doc comment.
This multiline comment is clear about different use cases. Make sure these cases stay updated if requirements change, so the comment doesn’t become outdated.
30-30: Ref usage looks good.
Storing the popover ref is an effective approach to control this UI element. No issues here.
Line range hint 32-34: Optional alignment check.
The function signature suggests generateForColumns might also potentially need this logic, but that’s a separate context. Just ensure all logic around popover states is consistent.
41-43: Practical approach to optional dialog types.
Passing dialogType as an optional argument is flexible and straightforward. Good usage of TypeScript union for clarity.
53-55: Timely state update.
Dispatching updateCurrentDocsData immediately after a successful save ensures the UI stays in sync. No issues here.
59-65: Excellent event handling pattern.
Preventing propagation and then conditionally opening the popover streamlines user flow. The logic is clear.
82-132: Clean popover integration with conditional rendering.
The structure nicely toggles between existing file saving and new file creation. The layout is user-friendly. Just ensure consistent styling across different popover interactions.
webview_panels/src/modules/documentationEditor/DocumentationEditor.tsx (6)
7-7: UI component usage from "@uicore".
Using Stack to lay out components is a straightforward approach. No concerns here, but keep an eye on spacing in narrower viewports.
14-14: Redux action usage.
Double-check that updateCurrentDocsData is invoked correctly in all relevant code paths when new doc content or columns are generated.
24-25: Modular approach with new imports.
The newly introduced SearchColumnsInput and BulkGenerateButton are well-placed in separate files. Good practice for maintainability.
29-29: Context usage.
Multiple property destructuring is fine. Just confirm you aren’t missing any state fields that might be necessary for the added search or bulk-generate features.
118-119: SaveDocumentation and BulkGenerateButton order.
Repositioning these actions for better visibility is consistent with the PR objectives. Approved.
128-134: Model doc input refactor.
DocGeneratorInput calling onModelDocSubmit is straightforward. Ensure that the new search logic or UI modifications don’t interfere with generating the model doc.
webview_panels/src/modules/documentationEditor/components/docGenerator/DocGenSelectedColumns.tsx (13)
1-2: Icon imports look consistent.
Importing from @assets/icons along with your usual approach is good. No concerns here.
3-3: Reuse existing doc context properly.
Excellent that you’re leveraging useDocumentationContext() for centralized state. Make sure you handle potential null states or empty data gracefully.
4-14: Clean UI imports.
All references from @uicore are easy to follow. The approach for a custom drawer plus a list is fine for the user selection UI.
19-25: Clear interface definition.
Specifying your props is helpful and ensures type safety. Keep these in sync with changes in bulk generation logic as the code evolves.
31-34: New component introduction.
This is a well-scoped component. The doc comment at lines 27-30 helps future maintainers.
36-38: State variables initialization.
Managing generation and search in local state is straightforward. No immediate issues found.
39-41: currentDocsData potential null check.
You properly destructure currentDocsData from context. Consider verifying that currentDocsData?.columns is not null before usage to prevent runtime errors.
43-45: Automated drawer opening.
Automatically opening the drawer on mount is user-friendly. Approved.
47-53: Filtering columns by search query.
Using useMemo for performance is a good approach. Just ensure it recalculates if columns change externally, which it should since you’re referencing currentDocsData?.columns.
55-70: Robust error handling.
The try/catch around generateForColumns is good. Logging the error with panelLogger helps with debugging.
72-74: Search event handling.
Straightforward approach. No issues seen.
76-98: Convenient selection logic.
Grouping selection into “all”, “documented”, or “missing” is a great user experience improvement. Just ensure you handle columns with partial data if that case can arise.
100-107: Column toggling logic.
Toggling inclusion in selectedColumns is standard. This approach should scale well.
webview_panels/src/modules/documentationEditor/state/documentationSlice.ts (3)
32-32: Introducing 'searchQuery' in the initial state.
This property is consistent with the newly added search logic. Ensure it’s reset or cleared appropriately when switching entities or contexts if needed.
39-44: setSearchQuery reducer definition.
Straightforward and essential for updating the UI based on the user’s search input.
288-288: Exporting setSearchQuery.
Exporting the new action ensures availability wherever search is needed. No issues here.
src/webview_provider/docsEditPanel.ts (1)
955-955: Include additional context in the webview response.
It's helpful that you're returning the up-to-date documentation object after saving. This ensures the UI can remain in sync with the most recent changes, preventing stale data from being displayed.
webview_panels/src/uiCore/components/dropdownButton/styles.module.scss (1)
11-14: Ensure consistent styling for secondary buttons.
You’ve introduced a specific style rule for .btn-secondary:last-child. Verify that in all contexts (e.g. theming, hover states) it behaves consistently with the rest of your button styling and doesn’t override global or parent styles unintentionally.
webview_panels/.storybook/preview-head.html (1)
7-12: Confirm proper integration with light/dark themes.
New CSS variables (--vscode-panel-background) for light and dark themes will help with consistency. Validate that these styles integrate correctly, especially if there are existing theme variables or any global styles that might conflict.
webview_panels/src/uiCore/components/popoverWithButton/PopoverWithButton.tsx (2)
32-33: Provide a default for popoverProps.
Having popoverProps = {} prevents errors when no props are passed, improving reliability. This is a good best practice that helps maintain robust code.
72-73: Use consistent class merging strategy.
Merging className with a local style class is a reasonable approach. However, ensure that additional classes or style overrides from popoverProps won’t be lost.
webview_panels/src/modules/documentationEditor/styles.module.scss (1)
323-334: LGTM! Well-structured search input styles.
The implementation provides a clean and accessible search interface with appropriate positioning and dimensions.
| <> | ||
| <div ref={ref}> | ||
| <PopoverWithButton | ||
| width="auto" | ||
| button={ | ||
| <DropdownButton onToggleClick={noop} onClick={noop} color={color}> | ||
| <ShinesIcon /> Bulk generate | ||
| </DropdownButton> | ||
| } | ||
| popoverProps={{ | ||
| placement: "bottom", | ||
| hideArrow: true, | ||
| className: classes.bulkGenPopover, | ||
| }} | ||
| > | ||
| {({ styles, close }) => ( | ||
| <div className={classes.popover}> | ||
| <div className={styles.popoverActions}> | ||
| <List> | ||
| {Object.entries(options).map(([_key, actions]) => ( | ||
| <> | ||
| {/* <li className={classes.sectionTitle}>{key}</li> */} | ||
| {actions.map((option) => ( | ||
| <li key={option.label}> | ||
| <Button | ||
| color="secondary" | ||
| onClick={() => { | ||
| close(); | ||
| void onOptionSelect(option.value); | ||
| }} | ||
| > | ||
| {option.label} | ||
| </Button> | ||
| </li> | ||
| ))} | ||
| </> | ||
| ))} | ||
| </List> | ||
| </div> | ||
| </div> | ||
| )} | ||
| </PopoverWithButton> | ||
| </div> | ||
| {sidePanelState === SidePanelState.DOCUMENTATION_SELECTED ? ( | ||
| <DocGenSelectedColumns | ||
| onClose={resetSidepanelState} | ||
| generateForColumns={bulkGenerateDocs} | ||
| /> | ||
| ) : null} | ||
| </> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add unique key to fragment in the mapped elements.
A missing key triggers a React warning and may cause rendering issues.
Below is a suggested diff to fix the missing key property:
- {Object.entries(options).map(([_key, actions]) => (
- <>
- {actions.map((option) => (
- <li key={option.label}>
- ...
- </li>
- ))}
- </>
- ))}
+ {Object.entries(options).map(([optionGroup, actions]) => (
+ <React.Fragment key={optionGroup}>
+ {actions.map((option) => (
+ <li key={option.value}>
+ ...
+ </li>
+ ))}
+ </React.Fragment>
+ ))}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <> | |
| <div ref={ref}> | |
| <PopoverWithButton | |
| width="auto" | |
| button={ | |
| <DropdownButton onToggleClick={noop} onClick={noop} color={color}> | |
| <ShinesIcon /> Bulk generate | |
| </DropdownButton> | |
| } | |
| popoverProps={{ | |
| placement: "bottom", | |
| hideArrow: true, | |
| className: classes.bulkGenPopover, | |
| }} | |
| > | |
| {({ styles, close }) => ( | |
| <div className={classes.popover}> | |
| <div className={styles.popoverActions}> | |
| <List> | |
| {Object.entries(options).map(([_key, actions]) => ( | |
| <> | |
| {/* <li className={classes.sectionTitle}>{key}</li> */} | |
| {actions.map((option) => ( | |
| <li key={option.label}> | |
| <Button | |
| color="secondary" | |
| onClick={() => { | |
| close(); | |
| void onOptionSelect(option.value); | |
| }} | |
| > | |
| {option.label} | |
| </Button> | |
| </li> | |
| ))} | |
| </> | |
| ))} | |
| </List> | |
| </div> | |
| </div> | |
| )} | |
| </PopoverWithButton> | |
| </div> | |
| {sidePanelState === SidePanelState.DOCUMENTATION_SELECTED ? ( | |
| <DocGenSelectedColumns | |
| onClose={resetSidepanelState} | |
| generateForColumns={bulkGenerateDocs} | |
| /> | |
| ) : null} | |
| </> | |
| <> | |
| <div ref={ref}> | |
| <PopoverWithButton | |
| width="auto" | |
| button={ | |
| <DropdownButton onToggleClick={noop} onClick={noop} color={color}> | |
| <ShinesIcon /> Bulk generate | |
| </DropdownButton> | |
| } | |
| popoverProps={{ | |
| placement: "bottom", | |
| hideArrow: true, | |
| className: classes.bulkGenPopover, | |
| }} | |
| > | |
| {({ styles, close }) => ( | |
| <div className={classes.popover}> | |
| <div className={styles.popoverActions}> | |
| <List> | |
| {Object.entries(options).map(([optionGroup, actions]) => ( | |
| <React.Fragment key={optionGroup}> | |
| {/* <li className={classes.sectionTitle}>{key}</li> */} | |
| {actions.map((option) => ( | |
| <li key={option.value}> | |
| <Button | |
| color="secondary" | |
| onClick={() => { | |
| close(); | |
| void onOptionSelect(option.value); | |
| }} | |
| > | |
| {option.label} | |
| </Button> | |
| </li> | |
| ))} | |
| </React.Fragment> | |
| ))} | |
| </List> | |
| </div> | |
| </div> | |
| )} | |
| </PopoverWithButton> | |
| </div> | |
| {sidePanelState === SidePanelState.DOCUMENTATION_SELECTED ? ( | |
| <DocGenSelectedColumns | |
| onClose={resetSidepanelState} | |
| generateForColumns={bulkGenerateDocs} | |
| /> | |
| ) : null} | |
| </> |
🧰 Tools
🪛 Biome (1.9.4)
[error] 177-192: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
|
If we move the bulk generate up, we also need to generate model description if missing. |
@sakibuiux : this looks disabled. can we change this style?
|
@sakibuiux : The position of the column search can be put on the right side of the columns header as it only applies to columns. |
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 3df20ec in 33 seconds
More details
- Looked at
115lines of code in4files - Skipped
0files when reviewing. - Skipped posting
5drafted comments based on config settings.
1. webview_panels/src/uiCore/components/dropdownButton/DropdownButton.tsx:12
- Draft comment:
Ensure consistent styling by using the same color logic for both Button and IconButton components. Consider setting a default color at the top level or ensuring both components use the same default. - Reason this comment was not posted:
Confidence changes required:50%
The DropdownButton component is using the color prop for both the Button and IconButton components. This could lead to inconsistent styling if the color prop is not provided, as the default color for Button is set to 'primary', but IconButton uses props.color with a fallback to 'primary'.
2. webview_panels/src/modules/documentationEditor/components/docGenerator/BulkGenerateButton.tsx:161
- Draft comment:
Consider handling theonToggleClickandonClickevents properly or remove them if they are not needed, instead of usingnoop. - Reason this comment was not posted:
Confidence changes required:50%
The use ofnoopfor bothonToggleClickandonClickin the DropdownButton component within BulkGenerateButton.tsx is unnecessary. It would be better to handle these events properly or remove them if not needed.
3. webview_panels/src/modules/documentationEditor/components/docGenerator/BulkGenerateButton.tsx:148
- Draft comment:
Consider providing user feedback or implementing a retry mechanism for error handling ingenerateDocsForMissingColumnsandgenerateForAllfunctions. - Reason this comment was not posted:
Confidence changes required:50%
ThegenerateDocsForMissingColumnsandgenerateForAllfunctions in BulkGenerateButton.tsx do not handle errors beyond logging them. It would be beneficial to provide user feedback or retry mechanisms.
4. webview_panels/src/modules/documentationEditor/components/docGenerator/DocGeneratorColumnsList.tsx:49
- Draft comment:
Ensurecolumn.nameis not null or undefined before callingtoLowerCase()to avoid potential runtime errors. - Reason this comment was not posted:
Confidence changes required:50%
ThefilteredColumnsfunction in DocGeneratorColumnsList.tsx usestoLowerCase()for case-insensitive search, which is fine, but it should handle potential null or undefined values forcolumn.namegracefully.
5. webview_panels/src/modules/documentationEditor/components/docGenerator/BulkGenerateButton.tsx:42
- Draft comment:
Ensure all labels use full words for clarity. The label 'Generate only missing columns' is clear, but ensure consistency across all labels. - Reason this comment was not posted:
Confidence changes required:50%
The label 'Generate all' was changed to 'Generate for all columns' inBulkGenerateButton.tsx, which is a good change for clarity. However, similar changes should be applied to other labels for consistency.
Workflow ID: wflow_WqaPeC8Mfytj83w4
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 7dbe00b in 48 seconds
More details
- Looked at
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
2drafted comments based on config settings.
1. webview_panels/src/modules/documentationEditor/components/docGenerator/BulkGenerateButton.tsx:42
- Draft comment:
Ensure that the label change from 'Generate for all columns' to 'Generate all columns' is consistently applied across the application to avoid confusion. - Reason this comment was not posted:
Confidence changes required:20%
The label change from 'Generate for all columns' to 'Generate all columns' is a minor text change and does not affect functionality. However, it is important to ensure consistency across the application.
2. webview_panels/src/modules/documentationEditor/components/docGenerator/BulkGenerateButton.tsx:42
- Draft comment:
Do not abbreviate terms in labels. Use 'Generate for all columns' instead of 'Generate all columns'. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
While we do have a rule about not abbreviating terms, this isn't actually an abbreviation - it's just more concise phrasing. "Generate all columns" is grammatically correct and clear. The meaning is identical. This seems like an unnecessary nitpick that doesn't improve code quality or clarity.
Maybe there's a consistency argument - are other similar labels in the codebase using "Generate for" instead of "Generate"? The rule about abbreviations might extend to preferring more verbose phrasing in general.
The rule about abbreviations is specifically about not shortening words (like "env" vs "environment"), not about grammatical variations that maintain the same meaning. The shorter version is actually better UI copy.
This comment should be deleted. It's not addressing an actual abbreviation, and the changed version is equally clear while being more concise.
Workflow ID: wflow_VlwMZ6Sgis62GQWf
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
@mdesmet in bulk generate, we specify "Generate only missing columns". In that case, do we need to generate for model as well? |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
webview_panels/src/modules/documentationEditor/DocumentationEditor.tsx (2)
127-133: Add key prop to DocGeneratorInput for React reconciliationWhile the DocGeneratorInput props are well structured, consider adding a key prop to help React efficiently handle component updates when the model changes.
<DocGeneratorInput + key={currentDocsData.name} entity={currentDocsData} type={EntityType.MODEL} onSubmit={onModelDocSubmit} placeholder="Describe your model" title={`Model: ${currentDocsData.name}`} />
Line range hint
89-93: Enhance error message for better user feedbackConsider providing more specific error messages to help users understand and resolve issues when doc generation fails.
} catch (error) { panelLogger.error("error while generating doc for model", error); postMessageToDataPilot({ id, - response: (error as Error).message, + response: `Failed to generate documentation: ${(error as Error).message}. Please try again or contact support if the issue persists.`, state: RequestState.ERROR, }); }webview_panels/src/modules/documentationEditor/components/docGenerator/BulkGenerateButton.tsx (1)
146-148: Enhance error handling with specific error messages.The current error logging is generic. Consider adding more context about the specific operation that failed.
- panelLogger.error("error generating for columns", value, err); + panelLogger.error( + `Failed to generate documentation for ${value} columns:`, + err instanceof Error ? err.message : err + );webview_panels/src/modules/documentationEditor/components/docGenerator/DocGeneratorColumnsList.tsx (2)
Line range hint
15-35: Consider memoizing testsPerColumns with a more efficient data structureWhile the current implementation works, consider using Map or Set for better performance with large datasets. The current implementation creates a new object spread on each iteration.
const testsPerColumns = useMemo(() => { - return ( - currentDocsTests?.reduce( - (acc: Record<string, DBTModelTest[]>, columnTest) => { - if (!columnTest.column_name) { - return acc; - } - acc[columnTest.column_name] = acc[columnTest.column_name] ?? []; - return { - ...acc, - [columnTest.column_name]: [ - ...acc[columnTest.column_name], - columnTest, - ], - }; - }, - {}, - ) ?? {} - ); + const testMap = new Map<string, DBTModelTest[]>(); + currentDocsTests?.forEach((columnTest) => { + if (!columnTest.column_name) return; + const tests = testMap.get(columnTest.column_name) ?? []; + tests.push(columnTest); + testMap.set(columnTest.column_name, tests); + }); + return testMap; }, [currentDocsTests]);
36-42: Enhance search functionality with more robust filteringThe current search implementation could be improved to:
- Trim whitespace from search input
- Support multiple search terms
- Include column type in search
const filteredColumns = useMemo(() => { if (!currentDocsData?.columns) return []; - if (!searchQuery) return currentDocsData.columns; - return currentDocsData.columns.filter((column) => - column.name.toLowerCase().includes(searchQuery.toLowerCase()), - ); + const query = searchQuery?.trim().toLowerCase() ?? ''; + if (!query) return currentDocsData.columns; + const terms = query.split(/\s+/); + return currentDocsData.columns.filter((column) => + terms.every(term => + column.name.toLowerCase().includes(term) || + column.type.toLowerCase().includes(term) + ) + ); }, [currentDocsData?.columns, searchQuery]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
webview_panels/src/modules/documentationEditor/DocumentationEditor.tsx(2 hunks)webview_panels/src/modules/documentationEditor/components/docGenerator/BulkGenerateButton.tsx(2 hunks)webview_panels/src/modules/documentationEditor/components/docGenerator/DocGeneratorColumnsList.tsx(3 hunks)webview_panels/src/uiCore/components/dropdownButton/DropdownButton.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- webview_panels/src/uiCore/components/dropdownButton/DropdownButton.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
webview_panels/src/modules/documentationEditor/components/docGenerator/BulkGenerateButton.tsx
[error] 180-195: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
🔇 Additional comments (11)
webview_panels/src/modules/documentationEditor/DocumentationEditor.tsx (4)
7-7: LGTM: Import changes align with UI simplification goals
The import changes reflect the removal of unused components and addition of new UI components that support the PR's objectives of improving editor usability.
Also applies to: 14-14, 24-24
28-31: LGTM: Simplified state management
The state management has been streamlined to focus on essential data (currentDocsData and currentDocsTests), removing unnecessary page selection complexity.
115-119: LGTM: Header reorganization improves accessibility
The new header layout successfully implements the PR objectives by:
- Moving save and bulk generate options to the top
- Consolidating action buttons for better organization
Line range hint 114-143: Verify the UI changes match the demo video
The implementation successfully addresses all PR objectives. However, please verify that the final UI layout matches the demonstration video provided in the PR, particularly:
- The positioning of the bulk generate and save buttons
- The consolidated "more options" menu
- The overall header organization
webview_panels/src/modules/documentationEditor/components/docGenerator/BulkGenerateButton.tsx (4)
22-38: LGTM! Clean state management implementation.
The transition from boolean state to an enum-based approach improves type safety and extensibility. The reset function is well-defined and follows the single responsibility principle.
Line range hint 49-65: Add return type to bulkGenerateDocs function.
For better type safety and documentation, specify the return type.
-const bulkGenerateDocs = async (
+const bulkGenerateDocs = async (
columns: DBTDocumentationColumn[],
isAll: boolean,
-) => {
+): Promise<DBTDocumentationColumn[]> => {151-152: LGTM! Clean button state management.
The button color logic effectively reflects the component's state through the UI, improving user feedback.
Also applies to: 160-165
179-196:
Add key prop to mapped elements.
React requires a unique key prop for elements in an array to maintain component identity across re-renders.
-{Object.entries(options).map(([_key, actions]) => (
- <>
+{Object.entries(options).map(([key, actions]) => (
+ <React.Fragment key={key}>
{actions.map((option) => (
<li key={option.label}>
<Button
color="secondary"
onClick={() => {
close();
void onOptionSelect(option.value);
}}
>
{option.label}
</Button>
</li>
))}
- </>
+ </React.Fragment>
))}Likely invalid or redundant comment.
🧰 Tools
🪛 Biome (1.9.4)
[error] 180-195: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
webview_panels/src/modules/documentationEditor/components/docGenerator/DocGeneratorColumnsList.tsx (3)
7-8: LGTM: Import changes align with new search functionality
The new imports support the column search feature mentioned in the PR objectives.
12-12: LGTM: State management simplified
Removal of selectedPages in favor of searchQuery aligns with the PR objective of removing page selection logic and enhancing search functionality.
65-70: Verify column type availability for key prop
The key now includes both name and type. Let's verify that column type is always available to prevent potential key issues.
✅ Verification successful
Column type is always available in the key prop
Based on the code analysis, the column type is guaranteed to be available for the key prop because:
- The
DBTDocumentationColumninterface extendsMetadataColumnwhich definestypeas an optional property - The actual data comes from
currentDocsData.columnswhich is typed asDBTDocumentationColumn[] - Even if the type is undefined, it will be stringified as "undefined" in the key, still maintaining uniqueness with the column name
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if there are any columns without type in the codebase
ast-grep --pattern 'columns: [
{
name: $_,
type: undefined,
$$$
}
]'
Length of output: 86
Script:
#!/bin/bash
# Let's check the column type definition and usage in the codebase
rg "type.*Column" -A 5 -B 5
# Also check for column interface or type definitions
ast-grep --pattern 'interface $_ {
name: $_,
type: $_,
$$$
}'
# And check actual column assignments
ast-grep --pattern 'column: {
name: $_,
type: $_,
$$$
}'
Length of output: 66991
Script:
#!/bin/bash
# Let's check the column type usage in the specific file and its imports
rg "type.*column" webview_panels/src/modules/documentationEditor/components/docGenerator/DocGeneratorColumnsList.tsx -A 5 -B 5
# Also check the column type definition in the component's directory
rg "type.*Column" webview_panels/src/modules/documentationEditor/components/docGenerator/ -A 5 -B 5
# And check for any column interfaces in the component's directory
ast-grep --pattern 'interface $_ {
name: string,
$$$
}' --file webview_panels/src/modules/documentationEditor/components/docGenerator/DocGeneratorColumnsList.tsx
Length of output: 578
Script:
#!/bin/bash
# Let's check the column type definition in the component's directory
rg "type.*Column|interface.*Column" webview_panels/src/modules/documentationEditor/components/docGenerator/ -A 5 -B 5
# Also check any imports that might contain column type definitions
rg "import.*Column.*from" webview_panels/src/modules/documentationEditor/components/docGenerator/ -A 2 -B 2
# And check the actual usage of column prop in the component
rg "column[^a-zA-Z]" webview_panels/src/modules/documentationEditor/components/docGenerator/DocGeneratorColumnsList.tsx -A 2 -B 2
Length of output: 6148
Script:
#!/bin/bash
# Let's check the DBTDocumentationColumn type definition
rg "type.*DBTDocumentationColumn|interface.*DBTDocumentationColumn" webview_panels/src/modules/documentationEditor/state/types.ts -A 5 -B 5
# Also check the currentDocsData type to see how columns are defined
rg "currentDocsData" webview_panels/src/modules/documentationEditor/state/ -A 5 -B 5
Length of output: 14441
| import { useRef, useState } from "react"; | ||
| import classes from "../../styles.module.scss"; | ||
| import { mergeCurrentAndIncomingDocumentationColumns } from "@modules/documentationEditor/utils"; | ||
| import DocGenSelectedColumns from "./DocGenSelectedColumns"; | ||
| import { noop } from "antd/es/_util/warning"; |
There was a problem hiding this comment.
Avoid importing from internal antd utilities.
The noop function is imported from antd's internal utilities (antd/es/_util/warning). Internal modules are not part of the public API and may change without notice.
Replace with a local implementation:
-import { noop } from "antd/es/_util/warning";
+const noop = () => {};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { noop } from "antd/es/_util/warning"; | |
| const noop = () => {}; |
| <SearchColumnsInput /> | ||
| </Stack> | ||
| {isDocumentationPageSelected ? ( | ||
| <Alert color="warning"> | ||
| Note: If you don’t want to override existing documentation, please | ||
| (re)generate documentation at the individual column level below | ||
| </Alert> | ||
| ) : null} | ||
| <Alert color="warning"> | ||
| Note: If you don’t want to override existing documentation, please | ||
| (re)generate documentation at the individual column level below | ||
| </Alert> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve alert message clarity and SearchColumnsInput placement
The alert message could be more actionable, and based on the PR comments from sakibuiux, the SearchColumnsInput placement should be reviewed.
- Make the alert message more actionable:
- Note: If you don't want to override existing documentation, please
- (re)generate documentation at the individual column level below
+ Warning: Bulk generation will override existing documentation.
+ Use individual column controls below to selectively generate documentation.- Consider moving SearchColumnsInput as suggested in PR comments:
<Stack className={classes.columnHeader}>
- <Stack>
- <h3>Columns</h3>
- <SyncWithDatabase />
- </Stack>
- <SearchColumnsInput />
+ <Stack style={{ flex: 1 }}>
+ <h3>Columns</h3>
+ <SyncWithDatabase />
+ </Stack>
+ <Stack style={{ alignItems: 'flex-end' }}>
+ <SearchColumnsInput />
+ </Stack>
</Stack>Committable suggestion skipped: line range outside the PR's diff.
Probably yes, but we should rename it to "Generate only missing" |
Overview
Problem
Improve doc editor usage
Solution
Screenshot/Demo
https://www.loom.com/share/460ccd4aa2da4c34902ef7121e26c81f
How to test
Checklist
README.mdupdated and added information about my changeImportant
Enhance documentation editor with column search, bulk generate options, and improved UI organization.
SearchColumnsInputfor column name search inDocumentationEditor.tsx.CommonActionButtons.tsx.PopoverWithButtonfor button interactions inPopoverWithButton.tsx.BulkGenerateButton.tsxfor all, missing, or selected columns.DocGenSelectedColumns.tsxfor selecting specific columns for documentation generation.setSearchQueryaction indocumentationSlice.tsto manage search state.SaveDocumentation.tsx.styles.module.scssfor better UI consistency.FeedbackButton.tsxandHelpButton.tsxto include telemetry events.This description was created by
for 7dbe00b. It will automatically update as commits are pushed.
Summary by CodeRabbit
Release Notes
New Features
SearchColumnsInputcomponent for improved search functionality.DocGenSelectedColumnscomponent for managing database column selections during documentation generation.CommonActionButtonswith a popover interface for easier access to settings and help options.FeedbackButtonwithin the action buttons for user feedback.HelpButtoncomponent with improved state management for help documentation.Improvements
SaveDocumentationcomponent for a more streamlined save process.DocumentationEditorcomponent for better user experience.EntityWithTestscomponent by removing unnecessary dependencies.Bug Fixes
Style