Add folder filters and update UI filter actions#146
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdds folder-scoped filters and priority-based backend matching; replaces dropdown actions with a menubar and modal-driven Include/Dismiss/Replace/Skip flows; introduces PathBreadcrumb, ComponentSelector, ReplaceComponentSection, tabs/menubar UI primitives, editor-focus-aware shortcuts, and tests for folder/priority behavior. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Frontend as FilterActionModal
participant Store as ComponentFilterStore
participant Backend as FilterComponents API
User->>Frontend: Open modal & select folder via PathBreadcrumb
User->>Frontend: Confirm action (Include/Dismiss/Replace/Skip)
Frontend->>Frontend: Build payload (action, filterBy="by_folder", folderPath, purl?, comment, license, replaceWith)
Frontend->>Store: onFilterComponent(payload)
Store->>Store: Detect filterBy == "by_folder" and aggregate purls
Store->>Backend: FilterComponents(list of per-purl DTOs)
Backend->>Backend: Match using MatchesPath/MatchesAnyPurl & select best by Priority/Compare
Backend-->>Store: Success
Store->>Store: Advance to next result / update undo state
Store-->>Frontend: Acknowledge
Frontend-->>User: Close modal / show toast
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔍 SCANOSS Code Similarity Detected📄 1 snippet matches found 🔗 View detailed findings on commit 3927a66 Files with similarities:
💡 Click the commit link above to see detailed annotations for each match. |
SCANOSS SCAN Completed 🚀
View more details on SCANOSS Action Summary |
isasmendiagus
force-pushed
the
feat/SP-2883/add-folder-Actions
branch
from
3927a66 to
189c9cf
Compare
🔍 SCANOSS Code Similarity Detected📄 1 snippet matches found 🔗 View detailed findings on commit 189c9cf Files with similarities:
💡 Click the commit link above to see detailed annotations for each match. |
SCANOSS SCAN Completed 🚀
View more details on SCANOSS Action Summary |
![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
🤖 Fix all issues with AI agents
In `@frontend/src/components/FilterComponentActions.tsx`:
- Around line 55-87: The shortcut lookups use camelCase object properties (e.g.,
KEYBOARD_SHORTCUTS.includeFileWithComments) but KEYBOARD_SHORTCUTS is keyed by
entities.Action enum values; update all shortcut props passed to
FilterActionButton to index KEYBOARD_SHORTCUTS with the appropriate
entities.Action enum keys (for Include: entities.Action.IncludeFileWithComments,
entities.Action.IncludeFileWithoutComments,
entities.Action.IncludeComponentWithComments,
entities.Action.IncludeComponentWithoutComments; for Remove/Dismiss: the
corresponding Dismiss* enum keys; for Replace: the corresponding Replace* enum
keys) so the .keys values come from the enum-keyed entries rather than undefined
camelCase properties.
In `@frontend/src/components/ReplaceComponentSection.tsx`:
- Around line 63-67: The current useEffect replaces the entire
declaredComponents state whenever componentsData changes, which discards
locally-added items; change the effect in ReplaceComponentSection so that when
componentsData is present it merges incoming components with existing
declaredComponents (using unique ids or keys to avoid duplicates) instead of
calling setDeclaredComponents(componentsData) outright, or alternatively update
the add flow in handleComponentSelected to use react-query’s
queryClient.setQueryData to optimistically insert the new component into the
componentsData cache so refetches won’t wipe locally-added items.
🧹 Nitpick comments (5)
backend/entities/result.go (1)
45-48: Documentation note acknowledged.The TODO highlights a valid concern about the pointer-to-slice pattern adding complexity. Converting
*[]stringto[]stringwithomitemptywould simplify nil checks while preserving JSON behavior.Would you like me to open an issue to track this refactor, or generate a diff for the change?
frontend/src/components/ComponentSelector.tsx (1)
3-3: Minor: Copyright year inconsistency.This file uses
2024while other new files in this PR (e.g.,PathBreadcrumb.tsx,ReplaceComponentSection.tsx) use2026.frontend/src/components/ReplaceComponentSection.tsx (1)
79-96: Consider adding error feedback for license fetch failures.The catch block silently clears
matchedLicenses. While the fallback is reasonable, users might benefit from knowing why no licenses appear for a component lookup failure.💡 Optional: Add user feedback
} catch { setMatchedLicenses([]); + toast({ + title: 'Notice', + description: 'Could not fetch licenses for this component', + variant: 'default', + }); }backend/entities/scanoss_settings_test.go (1)
319-392: Consider adding a test case for path-only filter without trailing slash.The
GetResultFilterTypetests don't cover the case where a filter has only aPathwithout a trailing slash and noPurl. This maps toByPurlin the current implementation, which may be intentional but should be explicitly tested to document the expected behavior.{ name: "path only without trailing slash (no purl)", filter: ComponentFilter{Path: "src/file.js"}, expected: ByPurl, // or ByFile if the implementation is updated },frontend/src/components/FilterActionButton.tsx (1)
61-69:handleOpenModalshould be memoized withuseCallback.
handleOpenModalis included in theuseMemodependency array foreventHandlerMap, but it's recreated on every render. This causeseventHandlerMapto be recreated unnecessarily, potentially triggering re-registrations inuseMenuEvents.Proposed fix
+import { useCallback, useMemo, useState } from 'react'; -import { useMemo, useState } from 'react'; ... - const handleOpenModal = () => { - if (!isCompletedResult) { - setModalOpen(true); - } - }; + const handleOpenModal = useCallback(() => { + if (!isCompletedResult) { + setModalOpen(true); + } + }, [isCompletedResult]); - const handleConfirm = async (args: OnFilterComponentArgs) => { - await onAdd(args); - }; + const handleConfirm = useCallback(async (args: OnFilterComponentArgs) => { + await onAdd(args); + }, [onAdd]);
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (16)
CHANGELOG.mdbackend/entities/component.gobackend/entities/result.gobackend/entities/scanoss_settings.gobackend/entities/scanoss_settings_test.gofrontend/package.jsonfrontend/src/components/ComponentSelector.tsxfrontend/src/components/FilterActionButton.tsxfrontend/src/components/FilterActionModal.tsxfrontend/src/components/FilterComponentActions.tsxfrontend/src/components/PathBreadcrumb.tsxfrontend/src/components/ReplaceComponentDialog.tsxfrontend/src/components/ReplaceComponentSection.tsxfrontend/src/components/ui/tabs.tsxfrontend/src/lib/utils.tsfrontend/src/modules/components/stores/useComponentFilterStore.ts
💤 Files with no reviewable changes (1)
- frontend/src/components/ReplaceComponentDialog.tsx
🚧 Files skipped from review as they are similar to previous changes (5)
- CHANGELOG.md
- frontend/src/lib/utils.ts
- frontend/package.json
- frontend/src/components/FilterActionModal.tsx
- backend/entities/component.go
🧰 Additional context used
🧬 Code graph analysis (7)
frontend/src/components/FilterComponentActions.tsx (3)
frontend/src/components/FilterActionButton.tsx (1)
FilterActionButton(47-136)backend/entities/component.go (2)
FilterAction(86-86)Replace(91-91)frontend/src/lib/shortcuts.ts (1)
KEYBOARD_SHORTCUTS(32-164)
frontend/src/components/ComponentSelector.tsx (8)
frontend/src/hooks/useEnvironment.tsx (1)
useEnvironment(37-66)frontend/src/hooks/useKeyboardShortcut.tsx (1)
useKeyboardShortcut(31-52)frontend/src/lib/shortcuts.ts (2)
KEYBOARD_SHORTCUTS(32-164)getShortcutDisplay(166-180)frontend/src/components/ui/popover.tsx (3)
Popover(31-31)PopoverTrigger(31-31)PopoverContent(31-31)frontend/src/components/ui/button.tsx (1)
Button(57-57)frontend/src/lib/utils.ts (1)
cn(27-29)frontend/src/components/ShortcutBadge.tsx (1)
ShortcutBadge(1-3)frontend/src/components/ui/scroll-area.tsx (1)
ScrollArea(46-46)
frontend/src/components/ReplaceComponentSection.tsx (8)
frontend/src/components/ui/use-toast.ts (2)
useToast(192-192)toast(192-192)frontend/wailsjs/go/service/ComponentServiceImpl.js (1)
GetDeclaredComponents(25-27)frontend/wailsjs/go/service/LicenseServiceImpl.js (1)
GetLicensesByPurl(9-11)frontend/src/components/ui/label.tsx (1)
Label(24-24)frontend/src/components/ComponentSelector.tsx (1)
ComponentSelector(47-132)frontend/src/components/SelectLicenseList.tsx (1)
SelectLicenseList(53-113)frontend/src/components/NewComponentDialog.tsx (1)
NewComponentDialog(46-107)frontend/src/components/OnlineComponentSearchDialog.tsx (1)
OnlineComponentSearchDialog(91-234)
frontend/src/components/PathBreadcrumb.tsx (1)
frontend/src/lib/utils.ts (2)
getPathSegments(31-33)cn(27-29)
frontend/src/components/ui/tabs.tsx (1)
frontend/src/lib/utils.ts (1)
cn(27-29)
backend/entities/scanoss_settings_test.go (4)
backend/entities/scanoss_settings.go (3)
ComponentFilter(79-86)SettingsFile(43-46)Bom(73-77)backend/entities/result.go (5)
Result(42-51)FilterType(127-127)ByFolder(132-132)ByFile(130-130)ByPurl(131-131)backend/entities/component.go (3)
Include(89-89)Remove(90-90)Replace(91-91)backend/entities/keyboard.go (1)
Action(31-31)
backend/entities/scanoss_settings.go (2)
frontend/wailsjs/go/models.ts (1)
ComponentFilter(29-50)backend/entities/result.go (3)
Result(42-51)ByFolder(132-132)ByFile(130-130)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (17)
backend/entities/result.go (1)
130-133: LGTM!The new
ByFolderfilter type follows the established pattern and integrates cleanly with the existingByFileandByPurlconstants to support folder-scoped filtering.frontend/src/components/PathBreadcrumb.tsx (1)
36-57: LGTM!Clean, focused component with appropriate memoization. The use of
indexas key is acceptable here since segments are derived deterministically fromfilePathand won't reorder independently.frontend/src/components/ui/tabs.tsx (1)
1-53: LGTM!Standard shadcn/ui Tabs implementation following Radix UI best practices. The
forwardRefpattern,displayNameassignments, andcnutility usage are all consistent with the existing UI component patterns.frontend/src/components/ComponentSelector.tsx (1)
47-131: LGTM!Well-structured component with good UX touches: keyboard shortcut integration with visual badge, conditional online search option, and clean separation between local components and actions.
frontend/src/components/ReplaceComponentSection.tsx (1)
98-109: LGTM!The component selection flow is well-implemented: properly handles new components, resets license state, and uses a composite key pattern to force
SelectLicenseListre-renders. The dialog coordination is clean.Also applies to: 117-154
backend/entities/scanoss_settings.go (4)
94-113: LGTM! Priority scoring logic is clear and well-documented.The priority hierarchy (path+purl=4 > purl=2 > path=1 > empty=0) correctly prioritizes more specific rules. The implementation aligns with the documented behavior.
115-143: LGTM! Comparison logic correctly implements priority-based sorting.The two-tier comparison (priority score first, then path length) provides sensible tie-breaking that naturally favors more specific paths.
145-178: LGTM! Path and purl matching logic handles constraints correctly.The folder detection via trailing slash and the wildcard behavior for empty constraints are implemented correctly. The nil-safe handling in
AppliesToproperly converts nil Purl to an empty slice.
220-236: LGTM! Priority-based selection correctly finds the best matching filter.The loop iterates all candidates and uses
Compareto select the highest-priority match rather than returning the first match. This ensures folder rules don't override more specific file+purl rules.backend/entities/scanoss_settings_test.go (4)
34-73: LGTM! Priority tests cover all score combinations.The table-driven tests verify all priority tiers including the distinction between folder paths and file paths at the same score level.
75-140: LGTM! AppliesTo tests comprehensively cover constraint combinations.Good coverage of edge cases including nil Purl handling and the requirement that both path and purl must match when both are specified.
142-186: LGTM! Compare tests validate the priority ordering.Tests correctly verify that higher-priority filters sort before lower-priority ones, and that longer paths win at the same score.
188-317: LGTM! IsResultInList tests verify folder matching and priority selection.Good coverage of folder prefix matching, nested paths, non-matching paths, and the priority system ensuring the highest-priority match is selected regardless of list order.
frontend/src/modules/components/stores/useComponentFilterStore.ts (2)
38-47: LGTM! Extended interface supports folder-level filtering.The new
folderPathandpurlfields along with the'by_folder'filter type provide the necessary structure for folder-based filter operations.
72-87: EnsurefolderPathincludes trailing slash for folder rule detection.The backend's
getResultFilterTypeidentifies folder rules by checking ifPathends with/. If the modal/UI doesn't guarantee a trailing slash onfolderPath, folder filters may be misclassified as purl-only rules.Consider normalizing the path:
if (filterBy === 'by_folder' && folderPath) { + // Ensure folder path ends with / for backend folder rule detection + const normalizedPath = folderPath.endsWith('/') ? folderPath : `${folderPath}/`; const dto: entities.ComponentFilterDTO[] = [{ action, comment, license, purl: purl ?? '', - path: folderPath, + path: normalizedPath, ...(replaceWith && { replace_with: replaceWith }), }];Please verify how
folderPathis constructed in the UI to confirm whether normalization is needed.frontend/src/components/FilterActionButton.tsx (2)
71-83: Keyboard shortcuts consistently open the modal regardless of original intent.Previously, shortcuts like
shortcutKeysByFileWithoutCommentsvsshortcutKeysByFileWithCommentstriggered different behaviors. Now all four shortcuts open the same modal. This is intentional per the PR objectives (modal-driven workflow), but users relying on direct action shortcuts will experience a workflow change.
108-135: LGTM! Button and modal integration is clean.The conditional rendering of
FilterActionModalonly whenselectedResultexists prevents errors, and the modal receives all necessary props for the new workflow.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| func getResultFilterType(cf ComponentFilter) FilterType { | ||
| // Folder rule: path ends with / | ||
| if strings.HasSuffix(cf.Path, "/") { | ||
| return ByFolder | ||
| } | ||
| // File rule: has both path and purl | ||
| if cf.Path != "" && cf.Purl != "" { | ||
| return ByFile | ||
| } | ||
| // Component rule: purl only | ||
| return ByPurl | ||
| } |
There was a problem hiding this comment.
Path-only filter without trailing slash returns ByPurl unexpectedly.
When a filter has only Path set (no trailing slash, no Purl), this function returns ByPurl, which seems semantically incorrect. Consider whether this represents a valid use case or if it should return a different type.
For example, ComponentFilter{Path: "src/file.js"} would return ByPurl even though it has no purl constraint.
Potential fix if path-only without trailing slash should be treated as file rule
func getResultFilterType(cf ComponentFilter) FilterType {
// Folder rule: path ends with /
if strings.HasSuffix(cf.Path, "/") {
return ByFolder
}
// File rule: has both path and purl
if cf.Path != "" && cf.Purl != "" {
return ByFile
}
+ // File rule: has path only (no trailing slash)
+ if cf.Path != "" {
+ return ByFile
+ }
// Component rule: purl only
return ByPurl
}📝 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.
| func getResultFilterType(cf ComponentFilter) FilterType { | |
| // Folder rule: path ends with / | |
| if strings.HasSuffix(cf.Path, "/") { | |
| return ByFolder | |
| } | |
| // File rule: has both path and purl | |
| if cf.Path != "" && cf.Purl != "" { | |
| return ByFile | |
| } | |
| // Component rule: purl only | |
| return ByPurl | |
| } | |
| func getResultFilterType(cf ComponentFilter) FilterType { | |
| // Folder rule: path ends with / | |
| if strings.HasSuffix(cf.Path, "/") { | |
| return ByFolder | |
| } | |
| // File rule: has both path and purl | |
| if cf.Path != "" && cf.Purl != "" { | |
| return ByFile | |
| } | |
| // File rule: has path only (no trailing slash) | |
| if cf.Path != "" { | |
| return ByFile | |
| } | |
| // Component rule: purl only | |
| return ByPurl | |
| } |
| useEffect(() => { | ||
| if (componentsData) { | ||
| setDeclaredComponents(componentsData); | ||
| } | ||
| }, [componentsData]); |
There was a problem hiding this comment.
Potential data loss: Query refetch overwrites locally-added components.
When componentsData updates (e.g., on refetch, window focus), setDeclaredComponents(componentsData) replaces the entire list, losing any components added locally via handleComponentSelected that haven't been persisted.
Consider merging instead of replacing:
🔧 Proposed fix
useEffect(() => {
if (componentsData) {
- setDeclaredComponents(componentsData);
+ setDeclaredComponents((prev) => {
+ const existingPurls = new Set(componentsData.map((c) => c.purl));
+ const localOnly = prev.filter((c) => !existingPurls.has(c.purl));
+ return [...componentsData, ...localOnly];
+ });
}
}, [componentsData]);Alternatively, consider using react-query's queryClient.setQueryData to optimistically update the cache when adding new components, keeping the query as the single source of truth.
📝 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.
| useEffect(() => { | |
| if (componentsData) { | |
| setDeclaredComponents(componentsData); | |
| } | |
| }, [componentsData]); | |
| useEffect(() => { | |
| if (componentsData) { | |
| setDeclaredComponents((prev) => { | |
| const existingPurls = new Set(componentsData.map((c) => c.purl)); | |
| const localOnly = prev.filter((c) => !existingPurls.has(c.purl)); | |
| return [...componentsData, ...localOnly]; | |
| }); | |
| } | |
| }, [componentsData]); |
🤖 Prompt for AI Agents
In `@frontend/src/components/ReplaceComponentSection.tsx` around lines 63 - 67,
The current useEffect replaces the entire declaredComponents state whenever
componentsData changes, which discards locally-added items; change the effect in
ReplaceComponentSection so that when componentsData is present it merges
incoming components with existing declaredComponents (using unique ids or keys
to avoid duplicates) instead of calling setDeclaredComponents(componentsData)
outright, or alternatively update the add flow in handleComponentSelected to use
react-query’s queryClient.setQueryData to optimistically insert the new
component into the componentsData cache so refetches won’t wipe locally-added
items.
🔍 SCANOSS Code Similarity Detected📄 1 snippet matches found 🔗 View detailed findings on commit 7f9f3d0 Files with similarities:
💡 Click the commit link above to see detailed annotations for each match. |
SCANOSS SCAN Completed 🚀
View more details on SCANOSS Action Summary |
SCANOSS SCAN Completed 🚀
View more details on SCANOSS Action Summary |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@frontend/src/components/SkipActionButton.tsx`:
- Around line 46-47: handleOpenModal currently sets modalOpen(true)
unconditionally which allows menu events (from useMenuEvents) to open the modal
when no selectedResult exists; update handleOpenModal to first check
selectedResult and return early if falsy, and add a useEffect that watches
selectedResult and sets modalOpen(false) when selectedResult becomes
null/undefined to ensure stale modal state is cleared; reference the
handleOpenModal function, modalOpen state, selectedResult variable, and the
useMenuEvents integration to locate the changes.
In `@frontend/src/components/SkipActionModal.tsx`:
- Around line 44-54: getFileExtension currently uses filename.indexOf('.') which
returns the first dot and misparses names like "foo.bar.js"; change it to use
filename.lastIndexOf('.') and keep the existing guard for dot at position 0 and
-1 so it returns null for hidden files or no-extension names; update
getFileExtension to compute dotIndex = filename.lastIndexOf('.') and return
substring(dotIndex + 1). No changes required to buildPath other than referencing
it remains correct.
♻️ Duplicate comments (1)
frontend/src/components/FilterComponentActions.tsx (1)
60-82: VerifyKEYBOARD_SHORTCUTSkeys match the enum values.These lookups use dot properties (e.g.,
KEYBOARD_SHORTCUTS.include). Ifentities.Actionvalues are PascalCase (e.g.,Include), these will beundefinedand.keyswill throw at render time. Please confirm the enum values and, if needed, index via the enum keys instead (e.g.,KEYBOARD_SHORTCUTS[entities.Action.Include]).You can confirm quickly with:
#!/bin/bash # Locate the Action enum values (if defined in repo) and the shortcuts map. rg -n "enum Action" frontend/src -g '*.ts' -C2 rg -n "export const KEYBOARD_SHORTCUTS" frontend/src/lib/shortcuts.ts -n -A80
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
backend/entities/keyboard.gofrontend/package.json.md5frontend/src/components/FilterComponentActions.tsxfrontend/src/components/SkipActionButton.tsxfrontend/src/components/SkipActionModal.tsxfrontend/src/lib/shortcuts.tsfrontend/wailsjs/go/models.ts
✅ Files skipped from review due to trivial changes (1)
- frontend/package.json.md5
🧰 Additional context used
🧬 Code graph analysis (3)
frontend/src/components/SkipActionModal.tsx (7)
frontend/src/hooks/useEnvironment.tsx (1)
useEnvironment(37-66)frontend/src/lib/utils.ts (1)
getPathSegments(31-33)frontend/wailsjs/go/service/ScanossSettingsServiceImp.js (2)
AddStagedScanningSkipPattern(5-7)CommitStagedScanningSkipPatterns(9-11)frontend/src/hooks/useKeyboardShortcut.tsx (1)
useKeyboardShortcut(31-52)frontend/src/lib/shortcuts.ts (2)
KEYBOARD_SHORTCUTS(32-139)getShortcutDisplay(141-155)frontend/src/components/PathBreadcrumb.tsx (1)
PathBreadcrumb(36-58)frontend/src/components/ShortcutBadge.tsx (1)
ShortcutBadge(1-3)
frontend/src/components/FilterComponentActions.tsx (4)
frontend/src/components/FilterActionButton.tsx (1)
FilterActionButton(45-131)frontend/src/lib/shortcuts.ts (1)
KEYBOARD_SHORTCUTS(32-139)frontend/src/components/ui/separator.tsx (1)
Separator(29-29)frontend/src/components/SkipActionButton.tsx (1)
SkipActionButton(41-105)
backend/entities/keyboard.go (1)
frontend/wailsjs/go/models.ts (1)
Accelerator(830-843)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build for Linux (webkit40)
- GitHub Check: Build for Windows
- GitHub Check: Build for Linux (webkit41)
- GitHub Check: Build for MacOS
🔇 Additional comments (11)
frontend/src/components/FilterComponentActions.tsx (2)
32-35: Imports align with the new UI composition.Nice and clean additions for the skip action and separator.
84-88: Separation of BOM vs. skip actions is clear and readable.The vertical separator + dedicated skip button is a nice UX touch.
backend/entities/keyboard.go (2)
33-53: Action constants and binding list are consistent.The new generic/skip actions are declared and exposed via
AllShortcutActionscoherently.Also applies to: 95-101
179-242: Shortcut defaults are aligned with the new action set.Names, descriptions, and key bindings for include/dismiss/replace/skip look consistent.
frontend/src/components/SkipActionButton.tsx (2)
48-65: Direct skip flow and toast feedback look solid.
80-103: Modal rendering gated by selection is clean and safe.frontend/src/components/SkipActionModal.tsx (2)
116-137: Pattern staging + commit flow is clear and user-friendly.
145-193: Modal layout and warning UX look good.frontend/wailsjs/go/models.ts (2)
12-18: Action enum updates match the consolidated action set.
153-170: Verify consumers handle optionalComponentFilterDTO.purl.Line 155 makes
purloptional; ensure downstream usage doesn’t assume a value before using/serializing it.frontend/src/lib/shortcuts.ts (1)
78-114: Shortcut mapping reflects the new generic + skip actions.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| const handleOpenModal = useCallback(() => setModalOpen(true), []); | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n frontend/src/components/SkipActionButton.tsxRepository: scanoss/scanoss.cc
Length of output: 4451
Prevent stale modal-open state when no selection exists.
Line 46: handleOpenModal sets modalOpen unconditionally. The keyboard shortcut is guarded with enabled: !!selectedResult (line 72), but useMenuEvents at lines 75-78 has no such guard. If a menu event fires without a selection, modalOpen becomes true, but the modal won't render (line 96 checks selectedResult). When a result is later selected, the modal unexpectedly opens. Add a guard in handleOpenModal to return early if !selectedResult, and a useEffect to close the modal when selection clears.
🛠️ Proposed fix
-import { useCallback, useState } from 'react';
+import { useCallback, useEffect, useState } from 'react';
-const handleOpenModal = useCallback(() => setModalOpen(true), []);
+const handleOpenModal = useCallback(() => {
+ if (!selectedResult) return;
+ setModalOpen(true);
+}, [selectedResult]);
+
+useEffect(() => {
+ if (!selectedResult) setModalOpen(false);
+}, [selectedResult]);🤖 Prompt for AI Agents
In `@frontend/src/components/SkipActionButton.tsx` around lines 46 - 47,
handleOpenModal currently sets modalOpen(true) unconditionally which allows menu
events (from useMenuEvents) to open the modal when no selectedResult exists;
update handleOpenModal to first check selectedResult and return early if falsy,
and add a useEffect that watches selectedResult and sets modalOpen(false) when
selectedResult becomes null/undefined to ensure stale modal state is cleared;
reference the handleOpenModal function, modalOpen state, selectedResult
variable, and the useMenuEvents integration to locate the changes.
| function getFileExtension(filename: string): string | null { | ||
| const dotIndex = filename.indexOf('.'); | ||
| if (dotIndex === -1 || dotIndex === 0) return null; | ||
| return filename.substring(dotIndex + 1); | ||
| } | ||
|
|
||
| function buildPath(segments: string[], index: number): string { | ||
| const path = segments.slice(0, index + 1).join('/'); | ||
| const isFile = index === segments.length - 1; | ||
| return isFile ? path : path + '/'; | ||
| } |
There was a problem hiding this comment.
Use the last dot for extension parsing.
Line 45: indexOf('.') makes foo.bar.js produce bar.js. lastIndexOf('.') yields the expected js extension for skip patterns.
🛠️ Proposed fix
- const dotIndex = filename.indexOf('.');
+ const dotIndex = filename.lastIndexOf('.');📝 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.
| function getFileExtension(filename: string): string | null { | |
| const dotIndex = filename.indexOf('.'); | |
| if (dotIndex === -1 || dotIndex === 0) return null; | |
| return filename.substring(dotIndex + 1); | |
| } | |
| function buildPath(segments: string[], index: number): string { | |
| const path = segments.slice(0, index + 1).join('/'); | |
| const isFile = index === segments.length - 1; | |
| return isFile ? path : path + '/'; | |
| } | |
| function getFileExtension(filename: string): string | null { | |
| const dotIndex = filename.lastIndexOf('.'); | |
| if (dotIndex === -1 || dotIndex === 0) return null; | |
| return filename.substring(dotIndex + 1); | |
| } | |
| function buildPath(segments: string[], index: number): string { | |
| const path = segments.slice(0, index + 1).join('/'); | |
| const isFile = index === segments.length - 1; | |
| return isFile ? path : path + '/'; | |
| } |
🤖 Prompt for AI Agents
In `@frontend/src/components/SkipActionModal.tsx` around lines 44 - 54,
getFileExtension currently uses filename.indexOf('.') which returns the first
dot and misparses names like "foo.bar.js"; change it to use
filename.lastIndexOf('.') and keep the existing guard for dot at position 0 and
-1 so it returns null for hidden files or no-extension names; update
getFileExtension to compute dotIndex = filename.lastIndexOf('.') and return
substring(dotIndex + 1). No changes required to buildPath other than referencing
it remains correct.
🔍 SCANOSS Code Similarity Detected📋 8 full file matches found 🔗 View detailed findings on commit 43ba389 8 files contain code similarities 💡 Click the commit link above to see detailed annotations for each match. |
SCANOSS SCAN Completed 🚀
View more details on SCANOSS Action Summary |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/wailsjs/go/models.ts (1)
27-48: MakeComponentFilter.purloptional to matchComponentFilterDTO.purl.
ComponentFilter.purlat line 29 is declared as required (purl: string), while the equivalentComponentFilterDTO.purlat line 158 is optional (purl?: string). Since folder-level filter actions (IncludeFolder, DismissFolder, ReplaceFolder, SkipFolder) don't require a specific package URL and are actively used in the UI,ComponentFilter.purlshould also be marked optional to properly support these scenarios and maintain type consistency between the two classes.
🤖 Fix all issues with AI agents
In `@frontend/src/components/FilterActionModal.tsx`:
- Around line 82-83: The initial selectedPathIndex may become -1 when filePath
is empty; update the useState initializer for selectedPathIndex to use
Math.max(0, segments.length - 1) so it never goes negative (symbols: segments
from getPathSegments, selectedPathIndex, useState). Also update the related
useEffect assignments that compute folder/file initial indices (the assignments
around setSelectedPathIndex in the useEffect) to guard with Math.max(0, ...) the
same way (symbols: setSelectedPathIndex, useEffect) so both initial state and
subsequent effect updates handle empty filePath defensively.
| const segments = useMemo(() => getPathSegments(filePath), [filePath]); | ||
| const [selectedPathIndex, setSelectedPathIndex] = useState(segments.length - 1); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and examine FilterActionModal.tsx
fd -t f "FilterActionModal.tsx" --exec wc -l {}Repository: scanoss/scanoss.cc
Length of output: 113
🏁 Script executed:
# Get the full content of FilterActionModal.tsx to understand the context
fd -t f "FilterActionModal.tsx" --exec cat -n {}Repository: scanoss/scanoss.cc
Length of output: 10889
🏁 Script executed:
# Find getPathSegments implementation to understand its behavior with empty string
rg "getPathSegments" -A 5 -B 2Repository: scanoss/scanoss.cc
Length of output: 4514
🏁 Script executed:
# Check buildPath function to see how selectedPathIndex is used
rg "buildPath" -A 10 -B 2Repository: scanoss/scanoss.cc
Length of output: 4045
🏁 Script executed:
# Find where FilterActionModal is used/imported
rg "FilterActionModal" -B 2 -A 5Repository: scanoss/scanoss.cc
Length of output: 2959
🏁 Script executed:
# Check PathBreadcrumb to see how it handles empty segments
fd -t f "PathBreadcrumb.tsx" --exec cat -n {}Repository: scanoss/scanoss.cc
Length of output: 2665
🏁 Script executed:
# Search for where the modal is actually opened with props
rg "filePath" frontend/src/components/FilterActionModal.tsx -B 2 -A 2Repository: scanoss/scanoss.cc
Length of output: 768
Consider handling the empty filePath edge case for defensive consistency.
While the modal is guarded by selectedResult at the call site (FilterActionButton.tsx), the initialization at line 83 doesn't protect against segments.length - 1 becoming -1 if filePath is empty. This is inconsistent with line 113, which already uses Math.max(0, segments.length - 2) for the folder case. For defensive consistency, apply the same protection to the initial state and the file/component initializations:
Suggested fix
const segments = useMemo(() => getPathSegments(filePath), [filePath]);
- const [selectedPathIndex, setSelectedPathIndex] = useState(segments.length - 1);
+ const [selectedPathIndex, setSelectedPathIndex] = useState(Math.max(0, segments.length - 1));Also apply the same to lines 109 and 116 in the useEffect.
🤖 Prompt for AI Agents
In `@frontend/src/components/FilterActionModal.tsx` around lines 82 - 83, The
initial selectedPathIndex may become -1 when filePath is empty; update the
useState initializer for selectedPathIndex to use Math.max(0, segments.length -
1) so it never goes negative (symbols: segments from getPathSegments,
selectedPathIndex, useState). Also update the related useEffect assignments that
compute folder/file initial indices (the assignments around setSelectedPathIndex
in the useEffect) to guard with Math.max(0, ...) the same way (symbols:
setSelectedPathIndex, useEffect) so both initial state and subsequent effect
updates handle empty filePath defensively.
🔍 SCANOSS Code Similarity Detected📋 8 full file matches found 🔗 View detailed findings on commit e743adb 8 files contain code similarities 💡 Click the commit link above to see detailed annotations for each match. |
SCANOSS SCAN Completed 🚀
View more details on SCANOSS Action Summary |
🔍 SCANOSS Code Similarity Detected📋 9 full file matches found 🔗 View detailed findings on commit 9fbd0ed 9 files contain code similarities 💡 Click the commit link above to see detailed annotations for each match. |
SCANOSS SCAN Completed 🚀
View more details on SCANOSS Action Summary |
isasmendiagus
force-pushed
the
feat/SP-2883/add-folder-Actions
branch
from
9fbd0ed to
7f6dc17
Compare
🔍 SCANOSS Code Similarity Detected📋 1 full file matches found 🔗 View detailed findings on commit 7f6dc17 Files with similarities:
💡 Click the commit link above to see detailed annotations for each match. |
SCANOSS SCAN Completed 🚀
View more details on SCANOSS Action Summary |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/components/FilterActionButton.tsx (1)
79-169: Avoid direct “Replace file” action without replacement selection.For
FilterAction.Replace,handleDirectActionfires withfilterBy: 'by_file'but noreplaceWith, which likely fails or yields an incomplete filter. This diverges from the new modal flow (e.g., inFilterComponentActions). Make the “File” item open the modal for Replace.🐛 Suggested fix
+ const isReplace = action === FilterAction.Replace; + const handleFileSelection = isReplace ? handleOpenModalFile : handleDirectAction; @@ - <DropdownMenuItem onClick={handleDirectAction}> + <DropdownMenuItem onClick={handleFileSelection}> File <DropdownMenuShortcut>{shortcuts.file}</DropdownMenuShortcut> </DropdownMenuItem>
🤖 Fix all issues with AI agents
In `@app.go`:
- Around line 135-184: The Actions submenu items currently pass nil for
accelerators (e.g., IncludeMenu.AddText(..., nil, ...), ReplaceMenu.AddText(...,
nil, ...), SkipMenu.AddText(..., nil, ...)), so shortcut hints and OS-level
bindings are missing; obtain the accelerators from
keyboardService.GetGroupedShortcuts()[entities.GroupActions] (like File/View
menus do) and pass the corresponding map entry for each action key
(entities.ActionInclude, entities.ActionIncludeFolder,
entities.ActionReplaceComponent, entities.ActionSkipExtension, etc.) instead of
nil so items show shortcuts and work when the webview is unfocused.
In `@backend/entities/keyboard.go`:
- Around line 235-267: DefaultShortcuts is missing entries for the exported
actions ReplaceComponent and SkipExtension so they don't appear in the UI; add
two shortcut objects to DefaultShortcuts mirroring the pattern used for
Replace/Skip (include Name, Description, Accelerator, AlternativeAccelerator,
Keys, Group: GroupActions, and Action set to ReplaceComponent and SkipExtension
respectively) so the frontend shortcuts map and backend lists include those
actions.
In `@frontend/src/components/SkipActionModal.tsx`:
- Around line 88-128: When initializing/resetting in the useEffect that checks
initialSelection, guard the 'extension' branch so that if initialSelection ===
'extension' but extension is falsy or empty you instead setActiveTab('path') and
setSelectedPathIndex to the appropriate value (e.g., segments.length - 1 or
parent folder for 'folder') to avoid selecting a non-rendered tab; also update
getSkipPattern (and optionally getTargetType/getWarningMessage) to treat a
missing extension by returning the path-based pattern (via buildPath(segments,
selectedPathIndex)) so you never generate patterns like '*.null' and UI text
stays consistent.
In `@frontend/src/components/ui/menubar.tsx`:
- Around line 221-235: The component uses a misspelled property name
"displayname" so React DevTools won't show the correct name; update the
component's static property to MenubarShortcut.displayName = "MenubarShortcut"
(capital D) replacing the existing MenubarShortcut.displayname assignment to
ensure the component name appears properly in debugging tools.
♻️ Duplicate comments (1)
frontend/src/components/SkipActionModal.tsx (1)
44-47: Use the last dot for extension parsing.
indexOf('.')misparses names likefoo.bar.js→bar.js.lastIndexOf('.')yields the expectedjs.🛠️ Suggested fix
- const dotIndex = filename.indexOf('.'); + const dotIndex = filename.lastIndexOf('.');
🧹 Nitpick comments (2)
frontend/src/components/FilterComponentActions.tsx (2)
83-161: Consolidate modal-open logic to reduce duplication.The same
setFilterModalAction / setFilterModalInitialSelection / setFilterModalOpensequence is repeated across include/dismiss/replace handlers. A small helper keeps behavior consistent and easier to change.♻️ Suggested refactor
+ const openFilterModal = useCallback( + (action: FilterAction, initialSelection: FilterInitialSelection) => { + if (!isCompletedResult && selectedResult) { + setFilterModalAction(action); + setFilterModalInitialSelection(initialSelection); + setFilterModalOpen(true); + } + }, + [isCompletedResult, selectedResult] + ); + - const handleIncludeFolder = useCallback(() => { - if (!isCompletedResult && selectedResult) { - setFilterModalAction(FilterAction.Include); - setFilterModalInitialSelection('folder'); - setFilterModalOpen(true); - } - }, [isCompletedResult, selectedResult]); + const handleIncludeFolder = useCallback( + () => openFilterModal(FilterAction.Include, 'folder'), + [openFilterModal] + );
287-377: Keep Menubar shortcut labels in sync withKEYBOARD_SHORTCUTS.The displayed shortcuts are hardcoded (
I,Shift+I, etc.) and can drift from the real bindings (e.g., F1/F2 variants). Consider deriving labels fromKEYBOARD_SHORTCUTSso the menu stays accurate.💡 Example approach
-import { KEYBOARD_SHORTCUTS } from '@/lib/shortcuts'; +import { getShortcutDisplay, KEYBOARD_SHORTCUTS } from '@/lib/shortcuts'; +import useEnvironment from '@/hooks/useEnvironment'; @@ + const { modifierKey } = useEnvironment(); + const shortcutLabel = useCallback( + (keys: string) => getShortcutDisplay(keys, modifierKey.label)[0], + [modifierKey.label] + ); @@ - <MenubarItem onSelect={handleIncludeFile}> + <MenubarItem onSelect={handleIncludeFile}> File - <MenubarShortcut>I</MenubarShortcut> + <MenubarShortcut>{shortcutLabel(KEYBOARD_SHORTCUTS.include.keys)}</MenubarShortcut> </MenubarItem>
| // Actions menu with submenus | ||
| ActionsMenu := AppMenu.AddSubmenu("Actions") | ||
| for _, shortcut := range actionShortcuts { | ||
| sc := shortcut | ||
| ActionsMenu.AddText(sc.Name, sc.Accelerator, func(cd *menu.CallbackData) { | ||
| runtime.EventsEmit(a.ctx, string(sc.Action)) | ||
| }) | ||
| } | ||
|
|
||
| // Include submenu | ||
| IncludeMenu := ActionsMenu.AddSubmenu("Include") | ||
| IncludeMenu.AddText("Include file", nil, func(cd *menu.CallbackData) { | ||
| runtime.EventsEmit(a.ctx, string(entities.ActionInclude)) | ||
| }) | ||
| IncludeMenu.AddText("Include folder", nil, func(cd *menu.CallbackData) { | ||
| runtime.EventsEmit(a.ctx, string(entities.ActionIncludeFolder)) | ||
| }) | ||
| IncludeMenu.AddText("Include component", nil, func(cd *menu.CallbackData) { | ||
| runtime.EventsEmit(a.ctx, string(entities.ActionIncludeWithModal)) | ||
| }) | ||
|
|
||
| // Dismiss submenu | ||
| DismissMenu := ActionsMenu.AddSubmenu("Dismiss") | ||
| DismissMenu.AddText("Dismiss file", nil, func(cd *menu.CallbackData) { | ||
| runtime.EventsEmit(a.ctx, string(entities.ActionDismiss)) | ||
| }) | ||
| DismissMenu.AddText("Dismiss folder", nil, func(cd *menu.CallbackData) { | ||
| runtime.EventsEmit(a.ctx, string(entities.ActionDismissFolder)) | ||
| }) | ||
| DismissMenu.AddText("Dismiss component", nil, func(cd *menu.CallbackData) { | ||
| runtime.EventsEmit(a.ctx, string(entities.ActionDismissWithModal)) | ||
| }) | ||
|
|
||
| // Replace submenu | ||
| ReplaceMenu := ActionsMenu.AddSubmenu("Replace") | ||
| ReplaceMenu.AddText("Replace file", nil, func(cd *menu.CallbackData) { | ||
| runtime.EventsEmit(a.ctx, string(entities.ActionReplace)) | ||
| }) | ||
| ReplaceMenu.AddText("Replace folder", nil, func(cd *menu.CallbackData) { | ||
| runtime.EventsEmit(a.ctx, string(entities.ActionReplaceFolder)) | ||
| }) | ||
| ReplaceMenu.AddText("Replace component", nil, func(cd *menu.CallbackData) { | ||
| runtime.EventsEmit(a.ctx, string(entities.ActionReplaceComponent)) | ||
| }) | ||
|
|
||
| // Skip submenu | ||
| SkipMenu := ActionsMenu.AddSubmenu("Skip") | ||
| SkipMenu.AddText("Skip file", nil, func(cd *menu.CallbackData) { | ||
| runtime.EventsEmit(a.ctx, string(entities.ActionSkip)) | ||
| }) | ||
| SkipMenu.AddText("Skip folder", nil, func(cd *menu.CallbackData) { | ||
| runtime.EventsEmit(a.ctx, string(entities.ActionSkipFolder)) | ||
| }) | ||
| SkipMenu.AddText("Skip extension", nil, func(cd *menu.CallbackData) { | ||
| runtime.EventsEmit(a.ctx, string(entities.ActionSkipExtension)) | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, locate app.go and examine the lines in question
find . -name "app.go" -type f | head -5Repository: scanoss/scanoss.cc
Length of output: 180
🏁 Script executed:
#!/bin/bash
# Search for the file containing the menu setup code
rg -l "ActionsMenu.*AddSubmenu.*Actions" --type goRepository: scanoss/scanoss.cc
Length of output: 180
🏁 Script executed:
#!/bin/bash
# Look for keyboardService and GetGroupedShortcuts references
rg "GetGroupedShortcuts|keyboardService" --type go | head -20Repository: scanoss/scanoss.cc
Length of output: 2158
🏁 Script executed:
#!/bin/bash
# Search for GroupActions and Action definitions in entities
rg "GroupActions|type Action" --type go | head -30Repository: scanoss/scanoss.cc
Length of output: 819
🏁 Script executed:
#!/bin/bash
# Get the actual app.go file and examine lines around 135-184
wc -l app.goRepository: scanoss/scanoss.cc
Length of output: 72
🏁 Script executed:
#!/bin/bash
# Read the app.go file to see the BuildMenu function
cat -n app.go | sed -n '120,200p'Repository: scanoss/scanoss.cc
Length of output: 3579
🏁 Script executed:
#!/bin/bash
# Check the keyboard entities to understand the structure
cat -n backend/entities/keyboard.go | head -100Repository: scanoss/scanoss.cc
Length of output: 4335
🏁 Script executed:
#!/bin/bash
# Look for Shortcut and Accelerator definitions
rg "type Shortcut|type Accelerator|Action.*string" backend/entities/keyboard.go -A 2 | head -40Repository: scanoss/scanoss.cc
Length of output: 234
🏁 Script executed:
#!/bin/bash
# Check the BuildMenu function signature and beginning
cat -n app.go | sed -n '105,140p'Repository: scanoss/scanoss.cc
Length of output: 1338
🏁 Script executed:
#!/bin/bash
# Get more context - look for where groupedShortcuts is used or defined
rg "groupedShortcuts" app.go -A 2 -B 2Repository: scanoss/scanoss.cc
Length of output: 284
🏁 Script executed:
#!/bin/bash
# Check if there are any action accelerators being used elsewhere
rg "ActionInclude|ActionDismiss|ActionReplace|ActionSkip" backend/entities/keyboard.go -B 1 -A 1 | head -60Repository: scanoss/scanoss.cc
Length of output: 1851
🏁 Script executed:
#!/bin/bash
# Check the keyboard_service_in_memory_impl.go to see actual shortcuts defined
cat -n backend/service/keyboard_service_in_memory_impl.go | grep -A 5 "ActionInclude\|ActionDismiss\|ActionReplace\|ActionSkip" | head -80Repository: scanoss/scanoss.cc
Length of output: 44
🏁 Script executed:
#!/bin/bash
# Get the full keyboard service implementation to see what shortcuts are defined
wc -l backend/service/keyboard_service_in_memory_impl.goRepository: scanoss/scanoss.cc
Length of output: 115
🏁 Script executed:
#!/bin/bash
# Get the full keyboard_service_in_memory_impl.go file
cat -n backend/service/keyboard_service_in_memory_impl.goRepository: scanoss/scanoss.cc
Length of output: 2198
🏁 Script executed:
#!/bin/bash
# Find where DefaultShortcuts is defined
rg "DefaultShortcuts" --type go | head -10Repository: scanoss/scanoss.cc
Length of output: 315
🏁 Script executed:
#!/bin/bash
# Search for keyboard.go or shortcuts definitions
fd keyboard.go --type fRepository: scanoss/scanoss.cc
Length of output: 90
🏁 Script executed:
#!/bin/bash
# Get the full DefaultShortcuts definition
cat -n backend/entities/keyboard.go | sed -n '100,250p'Repository: scanoss/scanoss.cc
Length of output: 5857
🏁 Script executed:
#!/bin/bash
# Get more of the DefaultShortcuts to see all Action group shortcuts
cat -n backend/entities/keyboard.go | sed -n '250,300p'Repository: scanoss/scanoss.cc
Length of output: 1691
🏁 Script executed:
#!/bin/bash
# Check if ActionReplaceComponent and ActionSkipExtension have shortcuts defined
rg "ActionReplaceComponent|ActionSkipExtension" backend/entities/keyboard.go -B 2 -A 8Repository: scanoss/scanoss.cc
Length of output: 1138
Use keyboard shortcuts from the service for Actions menu items.
The Actions menu is hardcoded with nil accelerators, preventing shortcut hints from appearing in the menu and disabling OS-level menu bindings when the webview isn't focused. Extract accelerators from keyboardService.GetGroupedShortcuts()[entities.GroupActions] as done in the File and View menus. Actions without defined shortcuts (e.g., ActionReplaceComponent, ActionSkipExtension) will naturally receive nil accelerators via the map lookup.
💡 Suggested pattern
@@
- // Actions menu with submenus
- ActionsMenu := AppMenu.AddSubmenu("Actions")
+ // Actions menu with submenus
+ ActionsMenu := AppMenu.AddSubmenu("Actions")
+ actionAccelerators := map[entities.Action]*keys.Accelerator{}
+ for _, sc := range groupedShortcuts[entities.GroupActions] {
+ actionAccelerators[sc.Action] = sc.Accelerator
+ }
@@
- IncludeMenu.AddText("Include file", nil, func(cd *menu.CallbackData) {
+ IncludeMenu.AddText("Include file", actionAccelerators[entities.ActionInclude], func(cd *menu.CallbackData) {
runtime.EventsEmit(a.ctx, string(entities.ActionInclude))
})🤖 Prompt for AI Agents
In `@app.go` around lines 135 - 184, The Actions submenu items currently pass nil
for accelerators (e.g., IncludeMenu.AddText(..., nil, ...),
ReplaceMenu.AddText(..., nil, ...), SkipMenu.AddText(..., nil, ...)), so
shortcut hints and OS-level bindings are missing; obtain the accelerators from
keyboardService.GetGroupedShortcuts()[entities.GroupActions] (like File/View
menus do) and pass the corresponding map entry for each action key
(entities.ActionInclude, entities.ActionIncludeFolder,
entities.ActionReplaceComponent, entities.ActionSkipExtension, etc.) instead of
nil so items show shortcuts and work when the webview is unfocused.
| // Reset state when modal opens | ||
| useEffect(() => { | ||
| if (open) { | ||
| if (initialSelection === 'extension') { | ||
| setActiveTab('extension'); | ||
| setSelectedPathIndex(segments.length - 1); | ||
| } else { | ||
| setActiveTab('path'); | ||
| if (initialSelection === 'folder') { | ||
| // Select parent folder (one level up from file) | ||
| setSelectedPathIndex(Math.max(0, segments.length - 2)); | ||
| } else { | ||
| setSelectedPathIndex(segments.length - 1); | ||
| } | ||
| } | ||
| } | ||
| }, [open, segments.length, initialSelection]); | ||
|
|
||
| // Determine target type for dynamic text | ||
| const getTargetType = () => { | ||
| if (activeTab === 'extension') return 'extension'; | ||
| return selectedPathIndex < segments.length - 1 ? 'folder' : 'file'; | ||
| }; | ||
| const targetType = getTargetType(); | ||
|
|
||
| const getSkipPattern = (): string => { | ||
| if (activeTab === 'extension') { | ||
| return `*.${extension}`; | ||
| } | ||
| return buildPath(segments, selectedPathIndex); | ||
| }; | ||
|
|
||
| const getWarningMessage = (): string => { | ||
| if (activeTab === 'extension') { | ||
| return `All .${extension} files will be skipped from future scans in this project.`; | ||
| } | ||
| if (selectedPathIndex < segments.length - 1) { | ||
| return 'This folder and all its contents will be skipped from future scans.'; | ||
| } | ||
| return 'This file will be skipped from future scans.'; | ||
| }; |
There was a problem hiding this comment.
Guard “extension” selection when no extension exists.
If initialSelection === 'extension' but the file has no extension, activeTab still becomes extension, the tab isn’t rendered, and getSkipPattern() produces *.null. Add a guard to fall back to the path tab and avoid invalid patterns.
🐛 Suggested fix
@@
- const extension = getFileExtension(filename);
+ const extension = getFileExtension(filename);
+ const hasExtension = !!extension;
@@
- useEffect(() => {
- if (open) {
- if (initialSelection === 'extension') {
- setActiveTab('extension');
- setSelectedPathIndex(segments.length - 1);
- } else {
- setActiveTab('path');
- if (initialSelection === 'folder') {
- // Select parent folder (one level up from file)
- setSelectedPathIndex(Math.max(0, segments.length - 2));
- } else {
- setSelectedPathIndex(segments.length - 1);
- }
- }
- }
- }, [open, segments.length, initialSelection]);
+ useEffect(() => {
+ if (open) {
+ if (initialSelection === 'extension' && hasExtension) {
+ setActiveTab('extension');
+ setSelectedPathIndex(segments.length - 1);
+ } else {
+ setActiveTab('path');
+ if (initialSelection === 'folder') {
+ // Select parent folder (one level up from file)
+ setSelectedPathIndex(Math.max(0, segments.length - 2));
+ } else {
+ setSelectedPathIndex(segments.length - 1);
+ }
+ }
+ }
+ }, [open, segments.length, initialSelection, hasExtension]);
@@
- const getSkipPattern = (): string => {
- if (activeTab === 'extension') {
- return `*.${extension}`;
- }
- return buildPath(segments, selectedPathIndex);
- };
+ const getSkipPattern = (): string => {
+ if (activeTab === 'extension' && hasExtension) {
+ return `*.${extension}`;
+ }
+ return buildPath(segments, selectedPathIndex);
+ };
@@
- const getWarningMessage = (): string => {
- if (activeTab === 'extension') {
- return `All .${extension} files will be skipped from future scans in this project.`;
- }
+ const getWarningMessage = (): string => {
+ if (activeTab === 'extension' && hasExtension) {
+ return `All .${extension} files will be skipped from future scans in this project.`;
+ }
if (selectedPathIndex < segments.length - 1) {
return 'This folder and all its contents will be skipped from future scans.';
}
return 'This file will be skipped from future scans.';
};🤖 Prompt for AI Agents
In `@frontend/src/components/SkipActionModal.tsx` around lines 88 - 128, When
initializing/resetting in the useEffect that checks initialSelection, guard the
'extension' branch so that if initialSelection === 'extension' but extension is
falsy or empty you instead setActiveTab('path') and setSelectedPathIndex to the
appropriate value (e.g., segments.length - 1 or parent folder for 'folder') to
avoid selecting a non-rendered tab; also update getSkipPattern (and optionally
getTargetType/getWarningMessage) to treat a missing extension by returning the
path-based pattern (via buildPath(segments, selectedPathIndex)) so you never
generate patterns like '*.null' and UI text stays consistent.
| const MenubarShortcut = ({ | ||
| className, | ||
| ...props | ||
| }: React.HTMLAttributes<HTMLSpanElement>) => { | ||
| return ( | ||
| <span | ||
| className={cn( | ||
| "ml-auto text-xs tracking-widest text-muted-foreground", | ||
| className | ||
| )} | ||
| {...props} | ||
| /> | ||
| ) | ||
| } | ||
| MenubarShortcut.displayname = "MenubarShortcut" |
There was a problem hiding this comment.
Fix displayName casing for MenubarShortcut.
displayname is a typo, so the component name won’t show correctly in React DevTools.
🔧 Fix
-MenubarShortcut.displayname = "MenubarShortcut"
+MenubarShortcut.displayName = "MenubarShortcut"📝 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.
| const MenubarShortcut = ({ | |
| className, | |
| ...props | |
| }: React.HTMLAttributes<HTMLSpanElement>) => { | |
| return ( | |
| <span | |
| className={cn( | |
| "ml-auto text-xs tracking-widest text-muted-foreground", | |
| className | |
| )} | |
| {...props} | |
| /> | |
| ) | |
| } | |
| MenubarShortcut.displayname = "MenubarShortcut" | |
| const MenubarShortcut = ({ | |
| className, | |
| ...props | |
| }: React.HTMLAttributes<HTMLSpanElement>) => { | |
| return ( | |
| <span | |
| className={cn( | |
| "ml-auto text-xs tracking-widest text-muted-foreground", | |
| className | |
| )} | |
| {...props} | |
| /> | |
| ) | |
| } | |
| MenubarShortcut.displayName = "MenubarShortcut" |
🤖 Prompt for AI Agents
In `@frontend/src/components/ui/menubar.tsx` around lines 221 - 235, The component
uses a misspelled property name "displayname" so React DevTools won't show the
correct name; update the component's static property to
MenubarShortcut.displayName = "MenubarShortcut" (capital D) replacing the
existing MenubarShortcut.displayname assignment to ensure the component name
appears properly in debugging tools.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/components/FilterActionButton.tsx (1)
103-178: Component shortcut labels aren’t wired to handlers.The menu shows component shortcuts (e.g., Shift+R), but this component doesn’t register a component-specific shortcut handler. Either bind the component shortcut (e.g.,
KEYBOARD_SHORTCUTS.replaceComponent) or remove the label to avoid misleading hints.✅ Optional wiring for Replace component
+ const componentShortcutKeys = useMemo(() => { + if (action === FilterAction.Replace) return KEYBOARD_SHORTCUTS.replaceComponent?.keys; + return undefined; + }, [action]); + + useKeyboardShortcut(componentShortcutKeys ?? '', handleOpenModalComponent, { + enabled: !isCompletedResult && !!selectedResult && !!componentShortcutKeys, + });
🤖 Fix all issues with AI agents
In `@frontend/src/components/FilterActionButton.tsx`:
- Around line 79-87: handleDirectAction currently calls onAdd for all actions
(including "Replace") without ensuring a replacement component (replaceWith)
exists; update handleDirectAction to guard against Replace by either routing it
through the modal or returning early: check the action value inside
handleDirectAction (referencing action, selectedResult, isCompletedResult) and
if action === 'replace' do not call onAdd (instead trigger the modal flow or
perform the replaceWith validation), otherwise proceed to call onAdd with the
existing filterBy and purl fields.
In `@frontend/src/components/FilterComponentActions.tsx`:
- Around line 288-324: The displayed shortcuts for "Component" (Shift+I /
Shift+D) are inconsistent with the actual behavior; either wire the keyboard
shortcuts to the component handlers or change the labels. Update the keyboard
binding logic so Shift+I triggers handleIncludeComponent and Shift+D triggers
handleDismissComponent (instead of opening the modal with file preselected), or
if you want the current file-first behavior, change the MenubarShortcut text for
the "Component" items to the actual keys that invoke the file-first action;
reference the MenubarItem onSelect handlers handleIncludeFile,
handleIncludeFolder, handleIncludeComponent and handleDismissFile,
handleDismissFolder, handleDismissComponent when making the change.
♻️ Duplicate comments (3)
app.go (1)
138-184: Missing keyboard accelerators for Actions menu items.All submenu items pass
nilas the accelerator parameter, which is inconsistent with the File and View menus that correctly extract accelerators fromgroupedShortcuts. This prevents shortcut hints from appearing in menu items and disables OS-level menu bindings when the webview isn't focused.Since
groupedShortcutsis already available (line 117), extract accelerators for the Actions group:Suggested approach
// Actions menu with submenus ActionsMenu := AppMenu.AddSubmenu("Actions") + actionAccelerators := map[entities.Action]*keys.Accelerator{} + for _, sc := range groupedShortcuts[entities.GroupActions] { + actionAccelerators[sc.Action] = sc.Accelerator + } // Include submenu IncludeMenu := ActionsMenu.AddSubmenu("Include") - IncludeMenu.AddText("Include file", nil, func(cd *menu.CallbackData) { + IncludeMenu.AddText("Include file", actionAccelerators[entities.ActionInclude], func(cd *menu.CallbackData) { runtime.EventsEmit(a.ctx, string(entities.ActionInclude)) }) - IncludeMenu.AddText("Include folder", nil, func(cd *menu.CallbackData) { + IncludeMenu.AddText("Include folder", actionAccelerators[entities.ActionIncludeFolder], func(cd *menu.CallbackData) { runtime.EventsEmit(a.ctx, string(entities.ActionIncludeFolder)) }) // ... apply same pattern to remaining itemsActions without defined shortcuts will naturally receive
nilaccelerators via the map lookup.backend/entities/keyboard.go (1)
100-111: AllShortcutActions includesReplaceComponentandSkipExtension, but they have no corresponding entries inDefaultShortcuts.These actions are registered for frontend binding but won't appear in the keyboard shortcuts UI or have associated keybindings. If these actions are intentionally UI-only (triggered via buttons/menus), consider adding a comment to clarify. Otherwise, add the missing shortcut entries.
frontend/src/components/ui/menubar.tsx (1)
221-235: FixMenubarShortcut.displayNamecasing.React DevTools ignores
displayname, so the component name won’t show correctly.🔧 Proposed fix
-MenubarShortcut.displayname = "MenubarShortcut" +MenubarShortcut.displayName = "MenubarShortcut"
| const handleDirectAction = useCallback(() => { | ||
| if (!isCompletedResult && selectedResult) { | ||
| onAdd({ | ||
| action, | ||
| filterBy: 'by_file', | ||
| purl: selectedResult.detected_purl ?? '', | ||
| }); | ||
|
|
||
| if (!confirmed) return; | ||
| } | ||
| }, [isCompletedResult, selectedResult, action, onAdd]); |
There was a problem hiding this comment.
Prevent Replace direct action without a replacement component.
handleDirectAction invokes onAdd for Replace without a replaceWith, which can yield an invalid request. Route Replace through the modal (or guard it) to ensure a replacement component is provided.
🐛 Suggested fix
- const handleDirectAction = useCallback(() => {
- if (!isCompletedResult && selectedResult) {
- onAdd({
- action,
- filterBy: 'by_file',
- purl: selectedResult.detected_purl ?? '',
- });
- }
- }, [isCompletedResult, selectedResult, action, onAdd]);
+ const handleDirectAction = useCallback(() => {
+ if (!isCompletedResult && selectedResult) {
+ if (action === FilterAction.Replace) {
+ handleOpenModal('file');
+ return;
+ }
+ onAdd({
+ action,
+ filterBy: 'by_file',
+ purl: selectedResult.detected_purl ?? '',
+ });
+ }
+ }, [isCompletedResult, selectedResult, action, onAdd, handleOpenModal]);🤖 Prompt for AI Agents
In `@frontend/src/components/FilterActionButton.tsx` around lines 79 - 87,
handleDirectAction currently calls onAdd for all actions (including "Replace")
without ensuring a replacement component (replaceWith) exists; update
handleDirectAction to guard against Replace by either routing it through the
modal or returning early: check the action value inside handleDirectAction
(referencing action, selectedResult, isCompletedResult) and if action ===
'replace' do not call onAdd (instead trigger the modal flow or perform the
replaceWith validation), otherwise proceed to call onAdd with the existing
filterBy and purl fields.
🔍 SCANOSS Code Similarity Detected📋 1 full file matches found 🔗 View detailed findings on commit 617c505 Files with similarities:
💡 Click the commit link above to see detailed annotations for each match. |
SCANOSS SCAN Completed 🚀
View more details on SCANOSS Action Summary |
🔍 SCANOSS Code Similarity Detected📋 1 full file matches found 🔗 View detailed findings on commit c6ec070 Files with similarities:
💡 Click the commit link above to see detailed annotations for each match. |
SCANOSS SCAN Completed 🚀
View more details on SCANOSS Action Summary |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@frontend/src/components/FilterComponentActions.tsx`:
- Around line 107-115: The Skip action is currently allowed for completed
results; update the createModalSkipHandler to early-return when
isCompletedResult is true (same style as other handlers) so it won't open the
modal for completed items, add the !isCompletedResult condition into the
skipEnabled computation so the keyboard shortcut respects immutability, and
change the Skip menu trigger to use the existing isDisabled boolean (instead of
its current enablement expression) so the UI button is disabled consistently;
reference the createModalSkipHandler function, the skipEnabled variable, the
isCompletedResult flag, and the isDisabled variable when making these changes.
♻️ Duplicate comments (1)
frontend/src/components/FilterComponentActions.tsx (1)
150-157: Align “Component” shortcut hints with actual bindings.Line 150–157 binds Shift+I/Shift+D to include/dismiss with modal (file-initial selection), while Line 212–240 labels those keys on the Component items. Either wire Shift+I/Shift+D to
handlers.includeComponent/handlers.dismissComponentor update the labels to reflect the real shortcut target.Also applies to: 212-240
| // Creates handler that opens skip modal with given selection | ||
| const createModalSkipHandler = useCallback( | ||
| (selection: SkipInitialSelection) => () => { | ||
| if (!selectedResult) return; | ||
| setSkipModalInitialSelection(selection); | ||
| setSkipModalOpen(true); | ||
| }, | ||
| [selectedResult] | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd FilterComponentActions.tsxRepository: scanoss/scanoss.cc
Length of output: 112
🏁 Script executed:
wc -l frontend/src/components/FilterComponentActions.tsxRepository: scanoss/scanoss.cc
Length of output: 116
🏁 Script executed:
cat -n frontend/src/components/FilterComponentActions.tsx | head -150Repository: scanoss/scanoss.cc
Length of output: 7401
🏁 Script executed:
cat -n frontend/src/components/FilterComponentActions.tsx | sed -n '145,280p'Repository: scanoss/scanoss.cc
Length of output: 7014
Skip action should be blocked for completed results to match filter action immutability.
The Skip handler and keyboard shortcuts are currently enabled for completed results, while Include, Dismiss, and Replace are properly blocked. This creates an inconsistency—if completed results should be immutable, the Skip action should follow the same pattern.
The fix requires three changes:
- Add
isCompletedResultcheck tocreateModalSkipHandler(line 110) - Update
skipEnabled(line 147) to include the!isCompletedResultcheck - Update the Skip menu trigger (line 275) to use the existing
isDisabledvariable
Proposed adjustment
const createModalSkipHandler = useCallback(
(selection: SkipInitialSelection) => () => {
- if (!selectedResult) return;
+ if (!selectedResult || isCompletedResult) return;
setSkipModalInitialSelection(selection);
setSkipModalOpen(true);
},
- [selectedResult]
+ [selectedResult, isCompletedResult]
);
- const skipEnabled = !!selectedResult;
+ const skipEnabled = !isCompletedResult && !!selectedResult;
<MenubarTrigger
- disabled={!selectedResult}
+ disabled={isDisabled}📝 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.
| // Creates handler that opens skip modal with given selection | |
| const createModalSkipHandler = useCallback( | |
| (selection: SkipInitialSelection) => () => { | |
| if (!selectedResult) return; | |
| setSkipModalInitialSelection(selection); | |
| setSkipModalOpen(true); | |
| }, | |
| [selectedResult] | |
| ); | |
| // Creates handler that opens skip modal with given selection | |
| const createModalSkipHandler = useCallback( | |
| (selection: SkipInitialSelection) => () => { | |
| if (!selectedResult || isCompletedResult) return; | |
| setSkipModalInitialSelection(selection); | |
| setSkipModalOpen(true); | |
| }, | |
| [selectedResult, isCompletedResult] | |
| ); |
🤖 Prompt for AI Agents
In `@frontend/src/components/FilterComponentActions.tsx` around lines 107 - 115,
The Skip action is currently allowed for completed results; update the
createModalSkipHandler to early-return when isCompletedResult is true (same
style as other handlers) so it won't open the modal for completed items, add the
!isCompletedResult condition into the skipEnabled computation so the keyboard
shortcut respects immutability, and change the Skip menu trigger to use the
existing isDisabled boolean (instead of its current enablement expression) so
the UI button is disabled consistently; reference the createModalSkipHandler
function, the skipEnabled variable, the isCompletedResult flag, and the
isDisabled variable when making these changes.
🔍 SCANOSS Code Similarity Detected📋 1 full file matches found 🔗 View detailed findings on commit 93fd741 Files with similarities:
💡 Click the commit link above to see detailed annotations for each match. |
SCANOSS SCAN Completed 🚀
View more details on SCANOSS Action Summary |
isasmendiagus
force-pushed
the
feat/SP-2883/add-folder-Actions
branch
from
e35bb53 to
43df69a
Compare
SCANOSS SCAN Completed 🚀
View more details on SCANOSS Action Summary |
1 participant
Changes:
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.