feat: add placeholder support to empty lists of applied controls in selector #2882
Conversation
WalkthroughAdded Changes
Sequence Diagram(s)sequenceDiagram
participant Parent
participant AutocompleteSelect
participant MultiSelect
participant User
Parent->>AutocompleteSelect: pass props (may include placeholder)
AutocompleteSelect->>AutocompleteSelect: placeholder ||= m.selectPlaceholder()
AutocompleteSelect->>MultiSelect: forward { value, placeholder, --sms-placeholder-color }
MultiSelect->>User: render input with placeholder (styled italic)
User->>MultiSelect: interact / open / type
sequenceDiagram
participant User
participant ModelTable
participant Search
Note over ModelTable: rows change or search setting toggled
ModelTable->>ModelTable: compute showSearch = searchEnabled && rows.length > 0
alt showSearch true
ModelTable->>Search: render
User->>Search: type query
Search->>ModelTable: emit search event
else showSearch false
Note over ModelTable: Search not rendered
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (3)
⏰ 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). (8)
🔇 Additional comments (7)
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 |
![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/messages/en.json (1)
2673-2675: i18n key added — looks good.The new key selectExistingAppliedControls aligns with usages and keeps wording consistent with “applied controls.”
Consider adding this key to other locales (or confirm fallback behavior is acceptable).
frontend/messages/fr.json (1)
2673-2675: French translation added — looks good.Wording aligns with existing terminology for “Mesures appliquées.”
Optionally replace "..." with the single-character ellipsis “…” for typographic consistency.
frontend/src/lib/components/Forms/AutocompleteSelect.svelte (1)
43-44: Consider usingundefinedinstead ofnullfor optional placeholder prop.The svelte-multiselect version in use is 11.1.1, which supports the placeholder prop. The code correctly forwards the prop to the MultiSelect component (line 462).
However, the current implementation uses
nullas the default (line 92), which explicitly passes a value to the child component. Usingundefinedinstead would omit the prop entirely, allowing MultiSelect's own defaults to apply—a cleaner Svelte pattern:
- Line 43: Change
placeholder?: string | null;toplaceholder?: string | undefined;- Line 92: Change
placeholder = null,toplaceholder = undefined,- Line 462: Change
{placeholder}to{placeholder ?? undefined}(optional, for clarity)This is a minor refinement; the current code functions correctly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
frontend/messages/en.json(1 hunks)frontend/messages/fr.json(1 hunks)frontend/src/lib/components/Forms/AutocompleteSelect.svelte(3 hunks)frontend/src/lib/components/Forms/ModelForm/FindingForm.svelte(1 hunks)frontend/src/lib/components/Forms/ModelForm/SecurityExceptionForm.svelte(1 hunks)frontend/src/lib/components/Forms/ModelForm/StakeholderForm.svelte(1 hunks)frontend/src/lib/components/Forms/ModelForm/TaskTemplateForm.svelte(1 hunks)frontend/src/lib/components/Forms/ModelForm/VulnerabilitiesForm.svelte(1 hunks)frontend/src/routes/(app)/(internal)/risk-scenarios/[id=uuid]/edit/+page.svelte(2 hunks)frontend/src/routes/(app)/(third-party)/requirement-assessments/[id=uuid]/edit/+page.svelte(1 hunks)
⏰ 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). (7)
- GitHub Check: build_enterprise_frontend
- GitHub Check: build_community_frontend
- GitHub Check: startup-docker-compose-test
- GitHub Check: enterprise-startup-docker-compose-test
- GitHub Check: enterprise-startup-functional-test (3.12)
- GitHub Check: startup-functional-test (3.12)
- GitHub Check: Analyze (python)
🔇 Additional comments (7)
frontend/src/lib/components/Forms/ModelForm/SecurityExceptionForm.svelte (1)
157-158: LGTM — consistent placeholder usage.Matches the new i18n key and improves UX for empty selections.
frontend/src/lib/components/Forms/ModelForm/StakeholderForm.svelte (1)
300-301: LGTM — placeholder wired correctly.Consistent with other forms and the new component prop.
frontend/src/lib/components/Forms/ModelForm/TaskTemplateForm.svelte (1)
272-272: LGTM! Nice UX improvement.Adding the placeholder provides helpful guidance for users when selecting applied controls.
frontend/src/routes/(app)/(third-party)/requirement-assessments/[id=uuid]/edit/+page.svelte (1)
533-533: LGTM! Placeholder improves user experience.The placeholder text provides clear guidance for users when selecting applied controls in the requirement assessment form.
frontend/src/lib/components/Forms/ModelForm/VulnerabilitiesForm.svelte (1)
77-77: LGTM! Consistent with the placeholder enhancement.The placeholder provides helpful context for users selecting applied controls in the vulnerabilities form.
frontend/src/lib/components/Forms/ModelForm/FindingForm.svelte (1)
160-160: LGTM! Placeholder enhances usability.The placeholder text helps guide users when selecting applied controls for findings.
frontend/src/routes/(app)/(internal)/risk-scenarios/[id=uuid]/edit/+page.svelte (1)
307-307: LGTM! Placeholders improve the risk scenario editing experience.Both applied controls fields (existing controls and extra controls) now have helpful placeholder text to guide users when the fields are empty.
Also applies to: 384-384
nas-tabchiche
changed the title
…applied-controls-in-selectors
…applied-controls-in-selectors
…trols-in-selectors' of https://github.com/intuitem/ciso-assistant-community into CA-1383-add-a-placeholder-in-empty-lists-of-applied-controls-in-selectors
There was a problem hiding this comment.
Here are a few changes to complete this:
1/ Do it for all similar fields, using a generic message "Select..."
2/ The message should be in italics and/or gray, to make clear it's a placeholder
3/ Hide the "search" field when the list is empty (most users imagine the search field can be used to search for candidate objects).
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
frontend/src/lib/components/Forms/AutocompleteSelect.svelte (1)
462-463: Consider using a theme variable for placeholder color.The hardcoded color
#9ca3af(gray-400) works but could become inconsistent if the theme changes. Consider using a CSS variable from your design system.For example:
- --sms-placeholder-color="#9ca3af" + --sms-placeholder-color="var(--color-surface-500)"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
frontend/messages/en.json(1 hunks)frontend/messages/fr.json(1 hunks)frontend/src/lib/components/Forms/AutocompleteSelect.svelte(5 hunks)frontend/src/lib/components/ModelTable/ModelTable.svelte(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/messages/en.json
- frontend/messages/fr.json
⏰ 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). (7)
- GitHub Check: build_community_frontend
- GitHub Check: build_enterprise_frontend
- GitHub Check: enterprise-startup-functional-test (3.12)
- GitHub Check: startup-functional-test (3.12)
- GitHub Check: enterprise-startup-docker-compose-test
- GitHub Check: startup-docker-compose-test
- GitHub Check: Analyze (python)
🔇 Additional comments (4)
frontend/src/lib/components/Forms/AutocompleteSelect.svelte (2)
43-43: LGTM! Placeholder prop properly implemented.The placeholder prop is correctly typed as optional with null support, and the default value uses the translation function for proper internationalization.
Also applies to: 92-92
376-384: Review the behavioral change fromrun()to$effect()in the value synchronization effect.The change from
run()to$effect()represents a fundamental shift in timing:run()executes as a pre-update effect during initialization, while$effect()runs during the normal effect phase after state changes. This affects whenselectedValuesare synchronized to$value.The concern is valid:
- Initial value setting may be delayed or not occur before first render
- External value changes (e.g., form reset) may not propagate correctly if the effect doesn't run at the expected time
- The internal update gate (
isInternalUpdate) prevents loops, but the new timing could introduce edge casesConfirm through manual testing:
- Initial form load with pre-selected values
- Form reset / external value changes
- Multiple selections with dynamic option loading
- All current usage contexts work as expected
frontend/src/lib/components/ModelTable/ModelTable.svelte (2)
229-230: LGTM! Clear improvement to UX.Hiding the search field when the table is empty prevents user confusion. The derived value correctly combines the search flag with row presence, and the comment clearly explains the intent.
477-479: LGTM! Conditional rendering correctly uses the derived value.The change from
{#if search}to{#if showSearch}correctly implements the new behavior of hiding search on empty tables.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/src/lib/components/ModelTable/Search.svelte (1)
32-37: Consider using a CSS variable or theme token instead of a hardcoded color.The hardcoded color
#9ca3afmay break theme consistency, especially in dark mode. Since the project uses design system classes (e.g.,bg-surface-50) and AutocompleteSelect uses a CSS variable (--sms-placeholder-color) for placeholder styling, consider following the same pattern here.Apply this diff to use a more maintainable approach:
<style> #search-input::placeholder { font-style: italic; - color: #9ca3af; + color: var(--placeholder-color, #9ca3af); } </style>Alternatively, if your design system provides a semantic color token (e.g.,
text-gray-400equivalent as a CSS variable), use that instead.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/src/lib/components/Forms/AutocompleteSelect.svelte(5 hunks)frontend/src/lib/components/ModelTable/Search.svelte(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/lib/components/Forms/AutocompleteSelect.svelte
⏰ 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). (7)
- GitHub Check: build_enterprise_frontend
- GitHub Check: build_community_frontend
- GitHub Check: enterprise-startup-docker-compose-test
- GitHub Check: enterprise-startup-functional-test (3.12)
- GitHub Check: startup-functional-test (3.12)
- GitHub Check: startup-docker-compose-test
- GitHub Check: Analyze (python)
…applied-controls-in-selectors
…applied-controls-in-selectors
tchoumi313
force-pushed
the
CA-1383-add-a-placeholder-in-empty-lists-of-applied-controls-in-selectors
branch
from
d2d4721 to
cb11fd8
Compare
Summary by CodeRabbit
New Features
Improvements
Style
✏️ Tip: You can customize this high-level summary in your review settings.