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/commonActionButtons/CommonActionButtons.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 save documentation process, adding new components like Changes
Sequence DiagramsequenceDiagram
participant User
participant DocumentationEditor
participant SearchColumnsInput
participant DocGeneratorColumnsList
participant SaveDocumentation
User->>DocumentationEditor: Open documentation editor
User->>SearchColumnsInput: Enter search query
SearchColumnsInput->>DocumentationEditor: Dispatch search query
DocumentationEditor->>DocGeneratorColumnsList: Filter columns
User->>SaveDocumentation: Save documentation
SaveDocumentation->>DocumentationEditor: Save and return updated documentation
Possibly related PRs
Suggested reviewers
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 🪧 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)
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.
SearchColumnsInputcomponent for searching columns by name inDocumentationEditor.tsx.CommonActionButtons.tsx.PopoverWithButtonto handle button interactions inPopoverWithButton.tsx.BulkGenerateButton.tsxwith options for 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 cd7526c. 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.Improvements
SaveDocumentationcomponent for a more streamlined save process.Bug Fixes
Style