fix: clean client warnings on audit pages #1399
Conversation
WalkthroughThe pull request introduces modifications to three frontend files: Changes
Poem
🪧 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: 0
🧹 Nitpick comments (3)
frontend/src/lib/components/TreeView/RecursiveTreeViewItem.svelte (1)
137-137: Improve comment formatting and clarity.The comment has formatting and clarity issues:
- Contains a typo: "has" should be "as"
- Uses informal language: "this has to be"
- Lacks proper spacing around the comment text
Apply this diff to improve the comment:
- <!-- class={node.contentProps.hidden === true ? 'hidden' : null} this has to be updated to see only assessable requirements has class prop is not used anymore in TreeViewItem --> + <!-- Removed: class={node.contentProps.hidden === true ? 'hidden' : null} + TODO: Update logic to show only assessable requirements as the class property is no longer used in TreeViewItem -->🧰 Tools
🪛 GitHub Actions: Frontend Linters
[warning] Code formatting does not meet Prettier standards. Run 'prettier --write' to fix formatting issues.
frontend/src/routes/(app)/(third-party)/compliance-assessments/[id=uuid]/table-mode/+page.svelte (2)
385-385: Standardize form ID naming convention.The form IDs use inconsistent naming patterns:
score-uses hyphen-caseis_scored-uses snake_casedocumentationScore-uses camelCaseApply this diff to standardize the naming convention using kebab-case:
- form={superForm(requirementAssessment.scoreForm, { id: `score-${requirementAssessment.id}` })} + form={superForm(requirementAssessment.scoreForm, { id: `requirement-score-${requirementAssessment.id}` })} - id: `is_scored-${requirementAssessment.id}` + id: `requirement-is-scored-${requirementAssessment.id}` - id: `documentationScore-${requirementAssessment.id}` + id: `requirement-documentation-score-${requirementAssessment.id}`Also applies to: 402-402, 420-420
🧰 Tools
🪛 GitHub Actions: Frontend Linters
[warning] Code formatting does not meet Prettier standards. Run 'prettier --write' to fix formatting issues.
Line range hint
385-420: Fix code formatting issues.The code has inconsistent indentation that needs to be fixed according to the Prettier standards.
Run the following command to fix the formatting:
prettier --write "frontend/src/routes/(app)/(third-party)/compliance-assessments/[id=uuid]/table-mode/+page.svelte"🧰 Tools
🪛 GitHub Actions: Frontend Linters
[warning] Code formatting does not meet Prettier standards. Run 'prettier --write' to fix formatting issues.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/src/lib/components/TreeView/RecursiveTreeViewItem.svelte(1 hunks)frontend/src/routes/(app)/(third-party)/compliance-assessments/[id=uuid]/table-mode/+page.svelte(3 hunks)
🧰 Additional context used
🪛 GitHub Actions: Frontend Linters
frontend/src/lib/components/TreeView/RecursiveTreeViewItem.svelte
[warning] Code formatting does not meet Prettier standards. Run 'prettier --write' to fix formatting issues.
frontend/src/routes/(app)/(third-party)/compliance-assessments/[id=uuid]/table-mode/+page.svelte
[warning] Code formatting does not meet Prettier standards. Run 'prettier --write' to fix formatting issues.
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: enterprise-startup-functional-test (3.12)
- GitHub Check: enterprise-startup-docker-compose-test
- GitHub Check: startup-functional-test (3.12)
- GitHub Check: enterprise-functional-tests (3.12, chromium)
- GitHub Check: migrations-check (3.12)
- GitHub Check: functional-tests (3.12, chromium)
- GitHub Check: startup-docker-compose-test
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
frontend/src/lib/components/TreeView/RecursiveTreeViewItem.svelte (1)
137-137: Verify the impact of removing the visibility logic.The removal of the
hiddenclass might affect how non-assessable requirements are displayed in the tree view. Please ensure that:
- Non-assessable requirements are properly handled
- The visibility state is managed correctly
Let's verify the usage of this component and the potential impact:
✅ Verification successful
The visibility logic removal is safe to proceed with
The commented code was already deprecated as indicated by the comment itself, and no other components in the codebase rely on the
contentProps.hiddenproperty for visibility management. The comment accurately reflects that this visibility mechanism is no longer used in TreeViewItem.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other components that might rely on the hidden class rg -l "class.*hidden.*null" --type svelte # Search for components that use contentProps.hidden ast-grep --pattern 'contentProps.hidden'Length of output: 115
Script:
#!/bin/bash # Search for files containing contentProps.hidden rg -t js -t jsx -t ts -t tsx -t svelte "contentProps\.hidden" # Search for TreeViewItem usage and implementation rg -t js -t jsx -t ts -t tsx -t svelte "TreeViewItem" # Search for any hidden class or visibility management rg -t js -t jsx -t ts -t tsx -t svelte "class=.*hidden|visibility|display:\s*none"Length of output: 286
Script:
#!/bin/bash # Find all TreeView related files fd "TreeView" # Search for contentProps.hidden without file type restrictions rg "contentProps\.hidden" # Search for visibility-related patterns rg "class=\".*hidden|style=\".*display:\s*none"Length of output: 5899
🧰 Tools
🪛 GitHub Actions: Frontend Linters
[warning] Code formatting does not meet Prettier standards. Run 'prettier --write' to fix formatting issues.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/src/lib/components/TreeView/TreeViewItem.svelte (1)
245-246: Consider using Svelte's built-in class directive instead of a custom prop.While exporting
classPropworks, Svelte provides a more idiomatic way to handle class forwarding using the built-inclassdirective. This would better integrate with Svelte's reactivity system and maintain consistency with other Svelte components.Consider this alternative approach:
-export let classProp = ''; // Replacing $$props.class -$: classesBase = `${cBase} ${classProp}`; +let className = ''; // Internal class name +export { className as class }; // Export as 'class' +$: classesBase = `${cBase} ${className}`;This approach:
- Better aligns with Svelte's class handling conventions
- Maintains compatibility with standard HTML class attributes
- Preserves reactivity while being more maintainable
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/src/lib/components/TreeView/RecursiveTreeViewItem.svelte(1 hunks)frontend/src/lib/components/TreeView/TreeViewItem.svelte(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/lib/components/TreeView/RecursiveTreeViewItem.svelte
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: startup-functional-test (3.12)
- GitHub Check: enterprise-startup-functional-test (3.12)
- GitHub Check: startup-docker-compose-test
- GitHub Check: enterprise-functional-tests (3.12, chromium)
- GitHub Check: enterprise-startup-docker-compose-test
- GitHub Check: functional-tests (3.12, chromium)
🔇 Additional comments (1)
frontend/src/lib/components/TreeView/TreeViewItem.svelte (1)
245-246: Verify consistent class prop usage across parent components.Let's ensure this change is consistently applied across all parent components using this TreeViewItem component.
✅ Verification successful
No impact on parent components - change is safe ✅
The change from
$$props.classtoclassPropis an internal implementation detail that doesn't affect any parent components. Current usages in RecursiveTreeViewItem and Dropdown components don't pass any class-related props to TreeViewItem.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for TreeViewItem usage patterns in parent components echo "Searching for TreeViewItem usage patterns..." rg -l "TreeViewItem" frontend/src/lib/components/ | while read -r file; do echo "=== $file ===" # Look for both class and classProp usage rg -A 2 -B 2 "<TreeViewItem.*(?:class|classProp)=" "$file" || true doneLength of output: 1277
Script:
#!/bin/bash # Search for any TreeViewItem usage with more context for file in frontend/src/lib/components/TreeView/RecursiveTreeViewItem.svelte \ frontend/src/lib/components/TreeView/RecursiveTreeView.svelte \ frontend/src/lib/components/Dropdown/Dropdown.svelte; do echo "=== $file ===" rg -A 5 -B 5 "<TreeViewItem" "$file" || true doneLength of output: 1997
ab-smith
changed the title
Summary by CodeRabbit
These changes enhance component visibility and improve form handling while maintaining existing functionality.