feat: ES-162 add welcome screen to select scan root #97
Conversation
WalkthroughThe changes add functionality for managing recent scan roots across the system. A new method is introduced in the backend ( Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant WS as WelcomeScreen
participant CS as ConfigStore
participant AP as App (Go)
participant CFG as Config
U->>WS: Click "Select Recent Folder"
WS->>CS: Request recent scan roots
CS->>AP: Call GetRecentScanRoots()
AP->>CFG: Retrieve recent scan roots
CFG-->>AP: Return list of scan roots
AP-->>CS: Return scan roots list
CS-->>WS: Update UI with scan roots
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
frontend/src/hooks/useResults.tsx (1)
29-30: Improved API clarityGood change to make the hook's interface more explicit by returning specific properties instead of spreading all queryResult properties. This improves API stability and prevents unintentional exposure of internal details.
Consider adding JSDoc comments to document the return value of this hook, especially since you've made the interface more explicit:
/** * Hook to fetch and manage scan results * @returns {Object} Object containing results data, loading state, and reset function * @returns {ResultsData} data - The fetched results data * @returns {boolean} isLoading - Whether the results are currently loading * @returns {() => void} reset - Function to reset the query and refetch results */frontend/src/style.css (1)
97-104: Good addition of global base styles.The new
@layer baseblock properly establishes consistent styling for the entire application with border, outline, background, and text color defaults. These styles will help maintain visual consistency for the new WelcomeScreen component.Consider consolidating the body styles from lines 5-12 with the new body styles in this layer for better maintainability in the future.
internal/config/config.go (1)
189-194: Use the slices.Index function for more concise code.The Go slices package provides a cleaner way to find an element's index.
- for i, p := range c.recentScanRoots { - if p == path { - c.recentScanRoots = slices.Delete(c.recentScanRoots, i, i+1) - break - } - } + if idx := slices.Index(c.recentScanRoots, path); idx != -1 { + c.recentScanRoots = slices.Delete(c.recentScanRoots, idx, idx+1) + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
app.go(1 hunks)frontend/src/components/WelcomeScreen.tsx(1 hunks)frontend/src/components/ui/card.tsx(1 hunks)frontend/src/hooks/useResults.tsx(1 hunks)frontend/src/routes/root.tsx(2 hunks)frontend/src/stores/useConfigStore.ts(4 hunks)frontend/src/style.css(1 hunks)frontend/wailsjs/go/main/App.d.ts(1 hunks)frontend/wailsjs/go/main/App.js(1 hunks)internal/config/config.go(8 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (16)
frontend/wailsjs/go/main/App.js (1)
9-11: Addition looks good!Properly implemented function to call the Go backend's
GetRecentScanRootsmethod to retrieve recent scan root directories.frontend/wailsjs/go/main/App.d.ts (1)
8-9: Type declaration matches implementationThe TypeScript type declaration correctly specifies that the function returns a Promise resolving to an array of strings, matching its implementation in App.js.
app.go (1)
197-199: Implementation follows existing patternsThis method follows the same pattern as other getter methods in the file, properly delegating to the configuration object and handling error returns consistently.
frontend/src/stores/useConfigStore.ts (4)
27-35: The import looks good.The addition of
GetRecentScanRootsimport is correctly included with the other App imports.
37-42: Good addition of recentScanRoots property.The
recentScanRootsproperty is correctly added to the ResultsState interface as a string array.
58-60: Proper initialization of the new state property.The
recentScanRootsproperty is correctly initialized as an empty array, consistent with the rest of the state properties.
74-80: Properly updated config retrieval.The
getInitialConfigmethod is correctly updated to fetch and set the recent scan roots along with the other configuration properties.frontend/src/routes/root.tsx (3)
24-34: Imports are properly updated.The imports have been correctly updated to include the new WelcomeScreen component and useConfigStore, while removing the unused useEffect import.
39-40: Good extraction of scanRoot from store.The scanRoot is correctly extracted from the config store using the selector pattern.
57-59: Good conditional rendering of WelcomeScreen.The conditional rendering is correctly implemented to show the WelcomeScreen when scanRoot is '/'.
frontend/src/components/ui/card.tsx (3)
5-19: Well-implemented Card component.The Card component follows best practices:
- Uses forwardRef for proper ref forwarding
- Applies the cn utility for class name merging
- Sets a displayName for better debugging
- Properly typed with TypeScript
20-75: Card subcomponents follow consistent patterns.All card subcomponents (CardHeader, CardTitle, CardDescription, CardContent, CardFooter) follow the same well-structured pattern with proper typing, className handling, and displayName settings. This ensures consistency and maintainability.
76-76: Exports are properly defined.All card components are correctly exported as named exports.
frontend/src/components/WelcomeScreen.tsx (1)
38-132: Component structure looks good!The WelcomeScreen component is well-structured with clear separation of concerns, proper error handling, and a clean UI layout using Card components. The implementation provides a good user experience by offering both new scan options and access to recent folders.
internal/config/config.go (2)
239-241: Good error handling on AddRecentScanRoot.The code properly logs errors when adding a recent scan root fails, which is important for debugging. This ensures that even if the configuration update fails, it won't block the main workflow.
35-36: Import addition is appropriate.The addition of the slices package is correct for the new functionality that manages the recent scan roots array.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/src/lib/utils.ts (1)
52-61: Well-implemented platform-specific path detection!The implementation of
isDefaultPathis clear and handles the default paths correctly for different platforms. It's a good approach to use a switch statement for platform-specific checks.For Windows, consider if there are other drive letters you might want to check for (D:, E:, etc.) or if you need to handle UNC paths in the future, but this implementation is sufficient for the current use case of detecting default paths.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
frontend/src/components/WelcomeScreen.tsx(1 hunks)frontend/src/lib/utils.ts(1 hunks)frontend/src/routes/root.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/components/WelcomeScreen.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (3)
frontend/src/routes/root.tsx (3)
53-65: Excellent fix for event handlers with proper cleanup!The event handlers are now correctly implemented with cleanup functions, which addresses the issue mentioned in the previous review. This prevents memory leaks and ensures event handlers won't be duplicated when the component re-renders.
67-69: Good implementation of the welcome screen conditional renderingThis conditional rendering logic implements the PR objective of showing a welcome screen when the scan root is set to a default path. The integration with the
isDefaultPathutility function and environment platform detection provides a clean, cross-platform solution.
41-42: Appropriate use of hooks for environment and configurationThese state hooks provide the necessary data for the welcome screen feature implementation, keeping the component clean and following React best practices.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/test.yml (1)
93-104: Establish and Verify Python Virtual Environment for ScanossThe new step creates and activates a Python virtual environment, upgrades pip, installs the
scanosspackage, and verifies its installation through multiple checks. This adds reliability to the integration test environment.
- Consider adding error handling (for example, using
set -eat the start of the script) to ensure that any command failure aborts the step immediately.- Additionally, wrapping
$VIRTUAL_ENVin quotes (e.g.,"$VIRTUAL_ENV/bin") when appending toGITHUB_PATHcan safeguard against potential issues with spaces in directory paths.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/test.yml(2 hunks)
🔇 Additional comments (3)
.github/workflows/test.yml (3)
19-19: Update Build Job EnvironmentThe update from
ubuntu-20.04toubuntu-22.04is a beneficial modernization that takes advantage of newer OS packages and security improvements. Please verify that all dependencies and build steps remain compatible with this newer image.
91-91: Specify Python Version for Integration TestsChanging the Python version from a floating
"3.x"to a fixed"3.8"ensures consistency across environments. Confirm that all tests and scripts are compatible with Python 3.8.
109-111: Activate Virtual Environment in Integration Test RunThe integration tests now correctly activate the Python virtual environment before executing
go testwith the integration tag. Ensure that this activation is necessary for your tests and that all required environment variables are available during the test execution.
matiasdaloia
force-pushed
the
feature/mdaloia/ES-162-SCANOSS-Code-Compare-Add-UI-for-selecting-the-scan-root-if-the-path-is-
branch
from
28a85ca to
7b14836
Compare
matiasdaloia
deleted the
feature/mdaloia/ES-162-SCANOSS-Code-Compare-Add-UI-for-selecting-the-scan-root-if-the-path-is-
branch
What
Summary by CodeRabbit
New Features
Style
Bug Fixes
Chores