Feat tars first test change#1332
Conversation
|
|
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
PR Reviewer Guide 🔍
|
| }); | ||
| document.addEventListener('mouseout', function(e) { | ||
| // Ignore moves between descendants of the same titled element. | ||
| if (tooltipTarget && !tooltipTarget.contains(e.relatedTarget)) { |
There was a problem hiding this comment.
Critical: e.relatedTarget can be null when the cursor leaves the browser window. tooltipTarget.contains(null) throws a TypeError in strict WebView2 / Firefox builds, leaving the tooltip stuck on screen for the rest of the session.
Fix: if (tooltipTarget && e.relatedTarget && !tooltipTarget.contains(e.relatedTarget)) — AI review
| </ul> | ||
| </div> | ||
| <div class="tree-node-row tree-node-row-info untrusted-action-row"> | ||
| <button type="button" class="untrusted-trust-btn" data-action="trust-folder">Trust folders and continue</button> |
There was a problem hiding this comment.
Critical: This button has data-action="trust-folder" but there is no click handler for that action anywhere in tree.js (or any other JS file). Clicking it is a no-op — snyk.trustWorkspaceFolders is never dispatched. The untrusted-folder feature is visually present but non-functional. — AI review
|
|
||
| // True when this scanner has findings but the active filters hide them all. | ||
| unfilteredCount := len(computeIssueStats(flattenIssues(allByProduct[p])).uniqueIssues) | ||
| hiddenByFilter := totalIssues == 0 && unfilteredCount > 0 |
There was a problem hiding this comment.
Should Fix: AllIssues includes issues suppressed by IssueViewOptions (OpenIssues=false, IgnoredIssues=false). When IVO hides all issues, unfilteredCount > 0 is true, so hiddenByFilter fires and the child node incorrectly says "No issues found with these filters" instead of the IVO-specific message ("Open issues are disabled", etc.). This is a semantic regression. The denominator should be the IVO-filtered-but-not-severity-filtered count, not the raw AllIssues count. — AI review
| } else if scanRegistered && !scanning && totalIssues == 0 { | ||
| // Surfaces the ✅ tick's meaning on hover; the child node carries any | ||
| // filter-aware detail. | ||
| tooltip = "No issues found" |
There was a problem hiding this comment.
Should Fix: When hiddenByFilter == true, the scanner row tooltip unconditionally says "No issues found", but the child info node correctly says "No issues found with these filters". A user hovering the scanner row is misled into thinking there are no issues at all. Add hiddenByFilter to the tooltip condition:
if hiddenByFilter {
tooltip = "No issues found with these filters"
} else if scanRegistered && !scanning && totalIssues == 0 {
tooltip = "No issues found"
}
``` — AI review| // VS Code https://docs.snyk.io/ide-tools/visual-studio-code-extension/workspace-trust | ||
| // Visual Studio https://docs.snyk.io/developer-tools/snyk-ide-plugins-and-extensions/visual-studio-extension/visual-studio-workspace-trust | ||
| // JetBrains https://docs.snyk.io/developer-tools/snyk-ide-plugins-and-extensions/jetbrains-plugin/jetbrains-plugin-folder-trust | ||
| const untrustedFolderLearnMoreURL = "https://docs.snyk.io/ide-tools/visual-studio-code-extension/workspace-trust" |
There was a problem hiding this comment.
Should Fix: This URL is hardcoded to the VS Code workspace-trust doc. JetBrains and Visual Studio users see VS Code–specific docs when the untrusted-folder banner appears. The TODO comment documents the correct fix (config_html.go already does this via INTEGRATION_ENVIRONMENT). Please implement before merging — shipping wrong docs for two of three supported IDEs is a user-facing correctness bug. — AI review
| assert.Contains(t, html, `id="expandAllBtn"`) | ||
| assert.Contains(t, html, `id="collapseAllBtn"`) | ||
| // IDE-1863: Expand/Collapse All buttons were removed from the toolbar in | ||
| // favour of per-scanner chevrons. |
There was a problem hiding this comment.
Should Fix: favour triggers the misspell linter (favour → favor). This will fail make lint. — AI review
| transform: translate(-50%, -75%) rotate(45deg); | ||
| } | ||
| [data-product-id].tree-node-has-children:not(.expanded) > .tree-node-row > .tree-chevron::before { | ||
| transform: translate(-75%, -50%) rotate(-45deg); |
There was a problem hiding this comment.
Suggestion: The collapsed chevron transform translate(-75%, -50%) rotate(-45deg) produces a left-pointing arrow. The comment above says "Points down when expanded, right when collapsed" — a right-pointing caret is the standard convention for a collapsed tree node. Check the rendered result; rotate(45deg) with translate(-50%, -75%) (same as the expanded state rotated to point right) may be the intended transform. — AI review
AI Review Summary — snyk/snyk-ls #1332Verdict: Changes Requested — 2 Critical, 4 Should Fix, 1 Suggestion found in-diff. Critical (block merge)
Should Fix
Suggestion
Also noted (out-of-diff)
SecurityClean. — AI review |
…1343) * fix: hide autofix messages when autofix is not available * test: add test coverage for agent fix summary in tree view
… feat/html-tree-view-ga
4 participants
Disables CLI path input field when 'manage binaries automatically' checkbox is ticked.