refactor(network): floating IPs UI/UX Improvements#703
Draft
vlad-schur-external-sap wants to merge 11 commits intomainfrom
Draft
refactor(network): floating IPs UI/UX Improvements#703vlad-schur-external-sap wants to merge 11 commits intomainfrom
vlad-schur-external-sap wants to merge 11 commits intomainfrom
Conversation
📝 WalkthroughWalkthroughRefactors ListToolbar search to use Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Router as Route.beforeLoad
participant TRPC as trpcClient.auth
participant App as Component
User->>Router: navigate to /network/(securitygroups|floatingips)
Router->>TRPC: getAvailableServices.query()
TRPC-->>Router: availableServices
alt network service missing
Router->>User: redirect to /accounts/$accountId/projects
else neutron missing
Router->>User: redirect to /accounts/$accountId/projects/$projectId/network/overview
else
Router->>App: allow route load → render component
end
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly Related PRs
Suggested Labels
Suggested Reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
12 tasks
Contributor
There was a problem hiding this comment.
Pull request overview
UI/UX refactor for Network-related views, consolidating sort/search/filter controls into the shared ListToolbar and updating Floating IP list/detail presentation accordingly.
Changes:
- Refactored
ListToolbarto inline sort controls and debounce search via@tanstack/react-form; updated filter label to “Filter by”. - Updated Floating IP list/detail UI (primary allocate button, clickable rows, details view switched from DataGrid to DescriptionList; details route breadcrumb removed).
- Updated/added tests to align with the new toolbar and UI labels/structures.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/aurora-portal/src/server/Compute/helpers/imageHelpers.test.ts | Updates Zod error expectation shape (received vs input). |
| apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-components/-details/SecurityGroupRulesTable.test.tsx | Updates test label to match “Filter by”. |
| apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/floatingips/-components/FloatingIps.tsx | Primary styling for Allocate button. |
| apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/floatingips/-components/-table/FloatingIpTableRow.tsx | Makes rows clickable to navigate; adjusts actions cell styling. |
| apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/floatingips/-components/-modals/AllocateFloatingIpModal.tsx | Tweaks select disabled/required behavior for allocation form. |
| apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/floatingips/-components/-details/FloatingIpDetailsView.tsx | Switches details layout to DescriptionList-based presentation. |
| apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/floatingips/-components/-details/FloatingIpDetailsView.test.tsx | Updates tests for DescriptionList layout and em-dash fallback. |
| apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/floatingips/$floatingIpId.tsx | Removes breadcrumb wrapper; keeps loading/error states. |
| apps/aurora-portal/src/client/components/navigation/MainNavigation.tsx | Tailwind class normalization (flex-shrink-0 → shrink-0). |
| apps/aurora-portal/src/client/components/ListToolbar/index.tsx | Major refactor: inline sort UI + debounced search via react-form; removes searchInputProps. |
| apps/aurora-portal/src/client/components/ListToolbar/index.test.tsx | Adjusts tests for debounce + adds coverage for special-character sort keys. |
| apps/aurora-portal/src/client/components/ListToolbar/SortInput.tsx | Removes standalone SortInput component (functionality moved into ListToolbar). |
| apps/aurora-portal/src/client/components/ListToolbar/SortInput.test.tsx | Removes SortInput tests (now covered via ListToolbar tests). |
| apps/aurora-portal/src/client/components/ListToolbar/FiltersInput.tsx | Renames filter select label to “Filter by”. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+55
to
+59
| const form = useForm({ | ||
| defaultValues: { | ||
| search: searchTerm ?? "", | ||
| }, | ||
| }) |
There was a problem hiding this comment.
useForm({ defaultValues: { search: searchTerm ?? "" } }) only seeds the initial value; when the searchTerm prop changes (e.g., URL-driven search params in Swift Containers), the SearchInput will stay stale because the form state is not synchronized. Consider adding a useEffect that updates/resets the search field when searchTerm changes (without triggering onSearch again) so the toolbar remains controlled from the outside.
Comment on lines
+173
to
+181
| <SearchInput | ||
| id={field.name} | ||
| name={field.name} | ||
| value={field.state.value} | ||
| onInput={(e) => field.handleChange(e.currentTarget.value)} | ||
| onClear={handleSearchClear} | ||
| placeholder={t`Search...`} | ||
| data-testid="searchbar" | ||
| /> |
There was a problem hiding this comment.
The toolbar no longer passes a search-submit handler to SearchInput (previously onSearch existed). As a result, the Search button/Enter action may no longer trigger an immediate search (only the debounced field-change does). Please wire SearchInput’s submit/search event (or wrap it in a form with onSubmit) so explicit user search actions still call onSearch immediately.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/aurora-portal/src/client/components/ListToolbar/index.tsx`:
- Around line 164-184: The SearchInput in the form.Field children render is
missing the onSearch prop so clicking its search button doesn't trigger an
immediate lookup; update the SearchInput invocation inside the children callback
to pass an onSearch handler that calls the existing onSearch prop with the
current field value (use field.state.value) so a button click invokes onSearch
immediately (in addition to the existing debounced onChange and
onClear/handleSearchClear wiring).
- Around line 55-59: The form's defaultValues (useForm) only initialize on mount
so add a syncing effect: create a useEffect that watches the searchTerm prop and
calls form.setValue('search', searchTerm ?? '') (or form.reset({ search:
searchTerm ?? "" }) if you want to reset other fields) to update the TanStack
form state when searchTerm changes; ensure you import React's useEffect and
reference the existing form and searchTerm identifiers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 95383667-4819-4bcb-b2f4-8608a14be54b
📒 Files selected for processing (14)
apps/aurora-portal/src/client/components/ListToolbar/FiltersInput.tsxapps/aurora-portal/src/client/components/ListToolbar/SortInput.test.tsxapps/aurora-portal/src/client/components/ListToolbar/SortInput.tsxapps/aurora-portal/src/client/components/ListToolbar/index.test.tsxapps/aurora-portal/src/client/components/ListToolbar/index.tsxapps/aurora-portal/src/client/components/navigation/MainNavigation.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/floatingips/$floatingIpId.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/floatingips/-components/-details/FloatingIpDetailsView.test.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/floatingips/-components/-details/FloatingIpDetailsView.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/floatingips/-components/-modals/AllocateFloatingIpModal.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/floatingips/-components/-table/FloatingIpTableRow.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/floatingips/-components/FloatingIps.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-components/-details/SecurityGroupRulesTable.test.tsxapps/aurora-portal/src/server/Compute/helpers/imageHelpers.test.ts
💤 Files with no reviewable changes (2)
- apps/aurora-portal/src/client/components/ListToolbar/SortInput.test.tsx
- apps/aurora-portal/src/client/components/ListToolbar/SortInput.tsx
Comment on lines
+55
to
+59
| const form = useForm({ | ||
| defaultValues: { | ||
| search: searchTerm ?? "", | ||
| }, | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if any callers update searchTerm after initial render (e.g., from URL params)
rg -n "searchTerm.*=.*useSearch|searchTerm.*=.*useState|setSearchTerm" --type=ts --type=tsx -g '!*.test.*' apps/aurora-portal/src/client/routes/Repository: cobaltcore-dev/aurora-dashboard
Length of output: 102
🏁 Script executed:
#!/bin/bash
# First, let's find the ListToolbar component and see its prop definition
cat -n apps/aurora-portal/src/client/components/ListToolbar/index.tsx | head -80Repository: cobaltcore-dev/aurora-dashboard
Length of output: 2856
🏁 Script executed:
#!/bin/bash
# Search for where ListToolbar is imported and used
rg -n "ListToolbar" --type=ts apps/aurora-portal/src/client/ -g '!*.test.*' | head -20Repository: cobaltcore-dev/aurora-dashboard
Length of output: 3680
🏁 Script executed:
#!/bin/bash
# Look for searchTerm prop usage/definition in route files
rg -n "searchTerm" --type=ts apps/aurora-portal/src/client/routes/ -g '!*.test.*' -B 2 -A 2Repository: cobaltcore-dev/aurora-dashboard
Length of output: 43123
🏁 Script executed:
#!/bin/bash
# Get more of the ListToolbar component to see how form fields are used
cat -n apps/aurora-portal/src/client/components/ListToolbar/index.tsx | sed -n '100,200p'Repository: cobaltcore-dev/aurora-dashboard
Length of output: 4621
🏁 Script executed:
#!/bin/bash
# Check if there are any useEffect hooks in ListToolbar that handle prop changes
cat -n apps/aurora-portal/src/client/components/ListToolbar/index.tsx | grep -A 10 "useEffect"Repository: cobaltcore-dev/aurora-dashboard
Length of output: 57
🏁 Script executed:
#!/bin/bash
# Check the entire ListToolbar component for any effect hooks or prop sync logic
wc -l apps/aurora-portal/src/client/components/ListToolbar/index.tsxRepository: cobaltcore-dev/aurora-dashboard
Length of output: 141
🏁 Script executed:
#!/bin/bash
# Get the complete file to check for useEffect
cat apps/aurora-portal/src/client/components/ListToolbar/index.tsx | tail -100Repository: cobaltcore-dev/aurora-dashboard
Length of output: 3676
🏁 Script executed:
#!/bin/bash
# Look at a parent component that passes dynamic searchTerm to see the actual usage pattern
cat -n apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Containers/index.tsx | grep -B 5 -A 5 "searchParam"Repository: cobaltcore-dev/aurora-dashboard
Length of output: 216
🏁 Script executed:
#!/bin/bash
# Find the Containers component file
find apps/aurora-portal/src/client/routes -name "Containers/index.tsx" | head -1Repository: cobaltcore-dev/aurora-dashboard
Length of output: 57
🏁 Script executed:
#!/bin/bash
# Check where searchParam is defined and how it's set
rg -n "searchParam" apps/aurora-portal/src/client/routes/_auth/accounts -A 3 -B 3 | head -80Repository: cobaltcore-dev/aurora-dashboard
Length of output: 12186
🏁 Script executed:
#!/bin/bash
# Check if TanStack Form's useForm initializes defaultValues only once
# by looking at how it handles prop changes - search for any existing patterns
rg -n "defaultValues" apps/aurora-portal/src/client/components/ -A 5 -B 2 | head -100Repository: cobaltcore-dev/aurora-dashboard
Length of output: 724
🏁 Script executed:
#!/bin/bash
# Check the actual route handler to see if it's truly dynamic (URL changes trigger re-render)
cat -n apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage/-components/Swift/Objects/index.tsx | sed -n '160,180p'Repository: cobaltcore-dev/aurora-dashboard
Length of output: 213
🏁 Script executed:
#!/bin/bash
# Find the actual Swift Objects component path
find apps/aurora-portal/src/client/routes -path "*Swift/Objects/index.tsx" -type fRepository: cobaltcore-dev/aurora-dashboard
Length of output: 201
🏁 Script executed:
#!/bin/bash
# Check how handleSearchChange is implemented in a parent component
rg -n "handleSearchChange" apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/storage -A 3 -B 1 | head -50Repository: cobaltcore-dev/aurora-dashboard
Length of output: 185
🏁 Script executed:
#!/bin/bash
# Get the Swift Objects component and look for handleSearchChange
cat "apps/aurora-portal/src/client/routes/_auth/accounts/\$accountId/projects/\$projectId/storage/-components/Swift/Objects/index.tsx" | grep -n "handleSearchChange" -A 3 -B 1Repository: cobaltcore-dev/aurora-dashboard
Length of output: 514
🏁 Script executed:
#!/bin/bash
# Let's check if TanStack React Form handles dynamic defaultValues
# by looking at the library documentation or usage patterns
rg -n "useForm" apps/aurora-portal/src/client/components/ListToolbar/index.tsx -A 10 -B 2Repository: cobaltcore-dev/aurora-dashboard
Length of output: 772
🏁 Script executed:
#!/bin/bash
# Check if there's any reset or field update logic when props change
rg -n "form\.reset|form\.setFieldValue|form\.getFieldValue" apps/aurora-portal/src/client/components/ListToolbar/index.tsxRepository: cobaltcore-dev/aurora-dashboard
Length of output: 115
🏁 Script executed:
#!/bin/bash
# Check TanStack Router behavior - does it remount or rerender when params change?
# Look at how the component is structured
cat "apps/aurora-portal/src/client/routes/_auth/accounts/\$accountId/projects/\$projectId/storage/-components/Swift/Objects/index.tsx" | head -50Repository: cobaltcore-dev/aurora-dashboard
Length of output: 1999
🏁 Script executed:
#!/bin/bash
# Verify: Does the parent actually update searchTerm after initial mount?
# Check if there's any scenario where searchTerm changes without remounting
rg -n "Route.useSearch" apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects -A 5 | head -40Repository: cobaltcore-dev/aurora-dashboard
Length of output: 176
🏁 Script executed:
#!/bin/bash
# Check if TanStack React Form handles defaultValues updates
# by examining the actual field rendering and form behavior
cat "apps/aurora-portal/src/client/routes/_auth/accounts/\$accountId/projects/\$projectId/storage/-components/Swift/Objects/index.tsx" | sed -n '160,175p'Repository: cobaltcore-dev/aurora-dashboard
Length of output: 832
🏁 Script executed:
#!/bin/bash
# Check if component is remounted or just re-rendered on URL change
# Look for key prop or other remount indicators
rg -n "ListToolbar|Form|useForm|defaultValues" apps/aurora-portal/src/client/components/ListToolbar/ -B 2 -A 2Repository: cobaltcore-dev/aurora-dashboard
Length of output: 5115
🏁 Script executed:
#!/bin/bash
# Test: Check TanStack Form documentation behavior with defaultValues
# by looking at actual implementation - does it update on prop changes?
python3 << 'EOF'
# TanStack React Form behavior: defaultValues only initialize once on mount
# They do NOT automatically update when the prop changes
# This is standard form library behavior
print("TanStack React Form behavior:")
print("- useForm({ defaultValues }) initializes form state once on mount")
print("- Changing defaultValues prop does NOT update the form's internal state")
print("- Must explicitly use form.setFieldValue() or form.reset() to sync prop changes")
print("\nConclusion: The concern is VALID - form state can become stale if searchTerm prop changes")
EOFRepository: cobaltcore-dev/aurora-dashboard
Length of output: 413
Form's search field state won't sync when searchTerm prop updates after mount.
TanStack React Form's defaultValues only initialize once on component mount. If the parent component updates the searchTerm prop (e.g., from URL params via external navigation), the form's internal state won't reflect the new value. This causes the search input to display stale data when the URL changes externally (browser back/forward, navigation from another page, etc.).
Add a useEffect to sync prop changes to the form:
🛠️ Suggested approach
+ import { useEffect } from "react"
// ...
const form = useForm({
defaultValues: {
search: searchTerm ?? "",
},
})
+ useEffect(() => {
+ if (searchTerm !== undefined && searchTerm !== form.getFieldValue("search")) {
+ form.setFieldValue("search", searchTerm)
+ }
+ }, [searchTerm, form])📝 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.
Suggested change
| const form = useForm({ | |
| defaultValues: { | |
| search: searchTerm ?? "", | |
| }, | |
| }) | |
| import { useCallback } from "react" | |
| import { useEffect } from "react" | |
| // ... other imports ... | |
| const form = useForm({ | |
| defaultValues: { | |
| search: searchTerm ?? "", | |
| }, | |
| }) | |
| useEffect(() => { | |
| if (searchTerm !== undefined && searchTerm !== form.getFieldValue("search")) { | |
| form.setFieldValue("search", searchTerm) | |
| } | |
| }, [searchTerm, form]) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/aurora-portal/src/client/components/ListToolbar/index.tsx` around lines
55 - 59, The form's defaultValues (useForm) only initialize on mount so add a
syncing effect: create a useEffect that watches the searchTerm prop and calls
form.setValue('search', searchTerm ?? '') (or form.reset({ search: searchTerm ??
"" }) if you want to reset other fields) to update the TanStack form state when
searchTerm changes; ensure you import React's useEffect and reference the
existing form and searchTerm identifiers.
Comment on lines
+164
to
184
| {onSearch && ( | ||
| <div className="w-full md:ml-auto md:w-auto md:min-w-25"> | ||
| <SearchInput {...searchProps} /> | ||
| <form.Field | ||
| name="search" | ||
| listeners={{ | ||
| onChangeDebounceMs: 500, | ||
| onChange: ({ value }) => onSearch?.(value), | ||
| }} | ||
| children={(field) => ( | ||
| <SearchInput | ||
| id={field.name} | ||
| name={field.name} | ||
| value={field.state.value} | ||
| onInput={(e) => field.handleChange(e.currentTarget.value)} | ||
| onClear={handleSearchClear} | ||
| placeholder={t`Search...`} | ||
| data-testid="searchbar" | ||
| /> | ||
| )} | ||
| /> | ||
| </div> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check SearchInput component API for onSearch prop
ast-grep --pattern 'export $_ SearchInput = $_($$$)'
rg -n "onSearch" --type=ts -A3 -B3 "juno-ui-components"Repository: cobaltcore-dev/aurora-dashboard
Length of output: 133
🏁 Script executed:
# First, find the ListToolbar file and see the imports
grep -n "SearchInput" /dev/stdin << 'EOF'
$(head -50 apps/aurora-portal/src/client/components/ListToolbar/index.tsx)
EOF
# Let's read the file directly to see imports
head -100 apps/aurora-portal/src/client/components/ListToolbar/index.tsxRepository: cobaltcore-dev/aurora-dashboard
Length of output: 3106
🏁 Script executed:
# Find SearchInput import in the ListToolbar file
rg -n "import.*SearchInput" apps/aurora-portal/src/client/components/ListToolbar/index.tsxRepository: cobaltcore-dev/aurora-dashboard
Length of output: 57
🏁 Script executed:
# Find SearchInput component definition in the project
fd -e tsx -e ts | xargs rg -l "export.*SearchInput" 2>/dev/null | head -5Repository: cobaltcore-dev/aurora-dashboard
Length of output: 57
🏁 Script executed:
# Search for SearchInput component definition
rg -n "const SearchInput|function SearchInput|export.*SearchInput" --type=tsx --type=ts -A 10Repository: cobaltcore-dev/aurora-dashboard
Length of output: 102
🏁 Script executed:
# Read the ListToolbar file to see the SearchInput usage in context (around lines 164-184)
sed -n '150,200p' apps/aurora-portal/src/client/components/ListToolbar/index.tsxRepository: cobaltcore-dev/aurora-dashboard
Length of output: 1974
🏁 Script executed:
# Find handleSearchClear implementation
rg -n "handleSearchClear" apps/aurora-portal/src/client/components/ListToolbar/index.tsx -A 5Repository: cobaltcore-dev/aurora-dashboard
Length of output: 460
🏁 Script executed:
# Check if there are other usages of SearchInput in the codebase
rg -n "SearchInput" --type=ts --type=tsx -B 2 -A 10 | grep -E "(SearchInput|onSearch)" | head -30Repository: cobaltcore-dev/aurora-dashboard
Length of output: 102
🏁 Script executed:
# Check if SearchInput is documented or has type definitions in the project
find . -name "*.d.ts" -o -name "*.ts" -o -name "*.tsx" | xargs grep -l "SearchInput" | grep -v node_modules | head -10Repository: cobaltcore-dev/aurora-dashboard
Length of output: 251
🏁 Script executed:
# Check how SearchInput is used in ProjectOverviewNavBar
grep -n "SearchInput" apps/aurora-portal/src/client/routes/_auth/accounts/\$accountId/projects/-components/ProjectOverviewNavBar.tsx -B 3 -A 10Repository: cobaltcore-dev/aurora-dashboard
Length of output: 1163
🏁 Script executed:
# Check node_modules for SearchInput component definition or try to find type definitions
find . -path "*/node_modules/*juno*" -name "*.d.ts" 2>/dev/null | head -5Repository: cobaltcore-dev/aurora-dashboard
Length of output: 57
🏁 Script executed:
# Look for package.json to find the juno-ui-components version
grep -r "juno-ui-components" package.json package-lock.json pnpm-lock.yaml 2>/dev/null | head -5Repository: cobaltcore-dev/aurora-dashboard
Length of output: 301
🏁 Script executed:
# Check if there are tests for ListToolbar that show SearchInput usage
find . -name "*ListToolbar*test*" -o -name "*ListToolbar*spec*" 2>/dev/null | head -5Repository: cobaltcore-dev/aurora-dashboard
Length of output: 57
🏁 Script executed:
# Look for test files for ListToolbar to understand the expected behavior
find . -path "*/ListToolbar*" -name "*.test.*" -o -path "*/ListToolbar*" -name "*.spec.*" 2>/dev/nullRepository: cobaltcore-dev/aurora-dashboard
Length of output: 301
🏁 Script executed:
# Check if there's a SearchInput component locally defined
fd -e tsx -e ts -type f | xargs grep -l "export.*const.*SearchInput\|export.*function.*SearchInput" 2>/dev/null | grep -v node_modulesRepository: cobaltcore-dev/aurora-dashboard
Length of output: 245
🏁 Script executed:
# Search for how SearchInput is documented or used with onChange vs onInput
rg -n "SearchInput" --type=tsx --type=ts -B 2 -A 15 | grep -E "(onChange|onInput|onSearch)" -B 2 -A 2Repository: cobaltcore-dev/aurora-dashboard
Length of output: 102
🏁 Script executed:
# Let's check the juno-ui-components in node_modules for SearchInput definition
find node_modules/@cloudoperators/juno-ui-components -name "*.d.ts" 2>/dev/null | xargs grep -l "SearchInput" | head -3Repository: cobaltcore-dev/aurora-dashboard
Length of output: 57
🏁 Script executed:
# Read the ListToolbar test file to understand the expected behavior
cat apps/aurora-portal/src/client/components/ListToolbar/index.test.tsxRepository: cobaltcore-dev/aurora-dashboard
Length of output: 8072
🏁 Script executed:
# Also check if there's documentation or examples showing SearchInput with onChange
rg -n "onChange.*SearchInput|SearchInput.*onChange" -A 5 -B 5Repository: cobaltcore-dev/aurora-dashboard
Length of output: 57
🏁 Script executed:
# Check the full ListToolbar file to understand the complete context
wc -l apps/aurora-portal/src/client/components/ListToolbar/index.tsxRepository: cobaltcore-dev/aurora-dashboard
Length of output: 141
🏁 Script executed:
# Read the full ListToolbar file to see all imports and the complete context
cat -n apps/aurora-portal/src/client/components/ListToolbar/index.tsx | head -120Repository: cobaltcore-dev/aurora-dashboard
Length of output: 4600
🏁 Script executed:
# Check the test more carefully - look for how the SearchInput behaves with the search button
grep -n "Search" apps/aurora-portal/src/client/components/ListToolbar/index.test.tsx -B 5 -A 5Repository: cobaltcore-dev/aurora-dashboard
Length of output: 5385
🏁 Script executed:
# Check if SearchInput is documented anywhere in the juno-ui-components
find node_modules/@cloudoperators/juno-ui-components -type f -name "*.d.ts" -o -name "*.ts" 2>/dev/null | head -10Repository: cobaltcore-dev/aurora-dashboard
Length of output: 57
🏁 Script executed:
# Try a simpler approach - list what's in juno-ui-components
ls -la node_modules/@cloudoperators/juno-ui-components/ 2>/dev/null | head -20Repository: cobaltcore-dev/aurora-dashboard
Length of output: 57
🏁 Script executed:
# Read more of the ListToolbar file to see if there's anything else relevant
sed -n '165,200p' apps/aurora-portal/src/client/components/ListToolbar/index.tsxRepository: cobaltcore-dev/aurora-dashboard
Length of output: 1462
🏁 Script executed:
# Check if SearchInput is exported from juno-ui-components and what props it accepts
python3 << 'EOF'
import json
# Try to find and read type definitions if they exist
try:
with open('node_modules/@cloudoperators/juno-ui-components/package.json', 'r') as f:
package_info = json.load(f)
print("Package info found:")
print(f"Main: {package_info.get('main')}")
print(f"Types: {package_info.get('types')}")
print(f"Version: {package_info.get('version')}")
except Exception as e:
print(f"Error: {e}")
EOFRepository: cobaltcore-dev/aurora-dashboard
Length of output: 181
🏁 Script executed:
# Check the exact test behavior - does it expect onSearch to fire when button clicked?
sed -n '109,120p' apps/aurora-portal/src/client/components/ListToolbar/index.test.tsxRepository: cobaltcore-dev/aurora-dashboard
Length of output: 580
🏁 Script executed:
# Look at ProjectOverviewNavBar to see if it shows SearchInput usage pattern
sed -n '57,65p' apps/aurora-portal/src/client/routes/_auth/accounts/\$accountId/projects/-components/ProjectOverviewNavBar.tsxRepository: cobaltcore-dev/aurora-dashboard
Length of output: 342
🏁 Script executed:
# Check if the test is currently passing by looking at the test more holistically
# Check if there's a mock setup that might make the button click trigger onSearch
grep -n "vi.fn\|Mock\|jest" apps/aurora-portal/src/client/components/ListToolbar/index.test.tsx | head -20Repository: cobaltcore-dev/aurora-dashboard
Length of output: 1075
🏁 Script executed:
# Look at the complete test setup to understand how it works
sed -n '109,125p' apps/aurora-portal/src/client/components/ListToolbar/index.test.tsxRepository: cobaltcore-dev/aurora-dashboard
Length of output: 793
🏁 Script executed:
# Check if SearchInput from juno-ui-components passes through onClick to a button
# by searching for other components that might show the pattern
rg -n "onClick.*SearchInput|SearchInput.*onClick" -B 3 -A 3Repository: cobaltcore-dev/aurora-dashboard
Length of output: 57
🏁 Script executed:
# Check if the ListToolbar tests are currently passing
# by running them or checking test results
cd apps/aurora-portal && npm test -- ListToolbar/index.test.tsx --run 2>&1 | head -100Repository: cobaltcore-dev/aurora-dashboard
Length of output: 190
🏁 Script executed:
# If tests can't run, check the git history to see if this test was recently added
git log --oneline apps/aurora-portal/src/client/components/ListToolbar/index.test.tsx | head -5Repository: cobaltcore-dev/aurora-dashboard
Length of output: 124
🏁 Script executed:
# Check if there's a way SearchInput receives the onSearch callback through context or parent props
grep -n "SearchInput" apps/aurora-portal/src/client/components/ListToolbar/index.tsx -B 5 -A 15Repository: cobaltcore-dev/aurora-dashboard
Length of output: 1623
Add onSearch handler to SearchInput for immediate search on button click.
The SearchInput component has a search button that should trigger an immediate search, independent of the debounced input changes. Currently, only onInput and onClear are wired. Without onSearch, clicking the button before the 500ms debounce completes won't trigger the search.
🛠️ Suggested fix
<SearchInput
id={field.name}
name={field.name}
value={field.state.value}
onInput={(e) => field.handleChange(e.currentTarget.value)}
+ onSearch={() => onSearch?.(field.state.value)}
onClear={handleSearchClear}
placeholder={t`Search...`}
data-testid="searchbar"
/>📝 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.
Suggested change
| {onSearch && ( | |
| <div className="w-full md:ml-auto md:w-auto md:min-w-25"> | |
| <SearchInput {...searchProps} /> | |
| <form.Field | |
| name="search" | |
| listeners={{ | |
| onChangeDebounceMs: 500, | |
| onChange: ({ value }) => onSearch?.(value), | |
| }} | |
| children={(field) => ( | |
| <SearchInput | |
| id={field.name} | |
| name={field.name} | |
| value={field.state.value} | |
| onInput={(e) => field.handleChange(e.currentTarget.value)} | |
| onClear={handleSearchClear} | |
| placeholder={t`Search...`} | |
| data-testid="searchbar" | |
| /> | |
| )} | |
| /> | |
| </div> | |
| {onSearch && ( | |
| <div className="w-full md:ml-auto md:w-auto md:min-w-25"> | |
| <form.Field | |
| name="search" | |
| listeners={{ | |
| onChangeDebounceMs: 500, | |
| onChange: ({ value }) => onSearch?.(value), | |
| }} | |
| children={(field) => ( | |
| <SearchInput | |
| id={field.name} | |
| name={field.name} | |
| value={field.state.value} | |
| onInput={(e) => field.handleChange(e.currentTarget.value)} | |
| onSearch={() => onSearch?.(field.state.value)} | |
| onClear={handleSearchClear} | |
| placeholder={t`Search...`} | |
| data-testid="searchbar" | |
| /> | |
| )} | |
| /> | |
| </div> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/aurora-portal/src/client/components/ListToolbar/index.tsx` around lines
164 - 184, The SearchInput in the form.Field children render is missing the
onSearch prop so clicking its search button doesn't trigger an immediate lookup;
update the SearchInput invocation inside the children callback to pass an
onSearch handler that calls the existing onSearch prop with the current field
value (use field.state.value) so a button click invokes onSearch immediately (in
addition to the existing debounced onChange and onClear/handleSearchClear
wiring).
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/index.tsx (1)
9-29: Extract this network-service guard into a shared helper.
apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network.tsxalready owns the top-level"network"gate, and this same child-level block now exists here and inapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/floatingips/index.tsx. Pulling it into one helper will keep the redirect rules from drifting.♻️ Possible extraction
+// apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/-helpers/requireNeutron.ts +import { redirect } from "@tanstack/react-router" +import { getServiceIndex } from "@/server/Authentication/helpers" + +export async function requireNeutron({ + trpcClient, + accountId, + projectId, +}: { + trpcClient: { auth: { getAvailableServices: { query: () => Promise<{ name: string; type: string }[]> } } } | undefined + accountId: string + projectId: string +}) { + const availableServices = (await trpcClient?.auth.getAvailableServices.query()) || [] + const serviceIndex = getServiceIndex(availableServices) + + if (!serviceIndex["network"]) { + throw redirect({ + to: "/accounts/$accountId/projects", + params: { accountId }, + }) + } + + if (!serviceIndex["network"]["neutron"]) { + throw redirect({ + to: "/accounts/$accountId/projects/$projectId/network/overview", + params: { accountId, projectId }, + }) + } +}- beforeLoad: async ({ context, params }) => { - const { trpcClient } = context - const { accountId, projectId } = params - - const availableServices = (await trpcClient?.auth.getAvailableServices.query()) || [] - const serviceIndex = getServiceIndex(availableServices) - - if (!serviceIndex["network"]) { - throw redirect({ - to: "/accounts/$accountId/projects", - params: { accountId }, - }) - } - - if (!serviceIndex["network"]["neutron"]) { - throw redirect({ - to: "/accounts/$accountId/projects/$projectId/network/overview", - params: { accountId, projectId }, - }) - } - }, + beforeLoad: async ({ context, params }) => + requireNeutron({ + trpcClient: context.trpcClient, + accountId: params.accountId, + projectId: params.projectId, + }),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/network/securitygroups/index.tsx around lines 9 - 29, Extract the duplicated network-service guard into a shared helper (e.g., ensureNeutronServiceAvailable) that accepts ({ context, params }) and runs the same logic currently in beforeLoad: call trpcClient.auth.getAvailableServices.query(), compute getServiceIndex(availableServices), and if network or network.neutron is missing perform the same redirect calls (use the same redirect destinations and params: { accountId } or { accountId, projectId }). Replace the duplicated beforeLoad blocks in beforeLoad (this file) and in the floatingips route with a single import and call to ensureNeutronServiceAvailable, and keep the top-level network.tsx gate as-is so the helper only enforces the child-level neutron redirect.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/network/securitygroups/index.tsx:
- Around line 9-29: Extract the duplicated network-service guard into a shared
helper (e.g., ensureNeutronServiceAvailable) that accepts ({ context, params })
and runs the same logic currently in beforeLoad: call
trpcClient.auth.getAvailableServices.query(), compute
getServiceIndex(availableServices), and if network or network.neutron is missing
perform the same redirect calls (use the same redirect destinations and params:
{ accountId } or { accountId, projectId }). Replace the duplicated beforeLoad
blocks in beforeLoad (this file) and in the floatingips route with a single
import and call to ensureNeutronServiceAvailable, and keep the top-level
network.tsx gate as-is so the helper only enforces the child-level neutron
redirect.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0e29172d-62b3-471e-893e-ac4448fe4ccf
📒 Files selected for processing (20)
apps/aurora-portal/src/client/routeTree.gen.tsapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/floatingips.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/floatingips/$floatingIpId.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/floatingips/index.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/index.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/-components/-modals/CreateSecurityGroupModal.test.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/-components/-modals/CreateSecurityGroupModal.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/-components/-modals/DeleteSecurityGroupDialog.test.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/-components/-modals/DeleteSecurityGroupDialog.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/-components/-modals/EditSecurityGroupModal.test.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/-components/-modals/EditSecurityGroupModal.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/-components/-table/List.test.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/-components/-table/List.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/-components/-table/SecurityGroupListContainer.test.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/-components/-table/SecurityGroupListContainer.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/-components/-table/SecurityGroupTableRow.test.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/-components/-table/SecurityGroupTableRow.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/index.tsxapps/aurora-portal/src/server/Compute/helpers/imageHelpers.test.ts
💤 Files with no reviewable changes (4)
- apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/-components/-modals/CreateSecurityGroupModal.test.tsx
- apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/-components/-modals/EditSecurityGroupModal.test.tsx
- apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups.tsx
- apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/floatingips.tsx
✅ Files skipped from review due to trivial changes (4)
- apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/-components/-table/SecurityGroupListContainer.tsx
- apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/index.tsx
- apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/-components/-table/List.tsx
- apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/floatingips/$floatingIpId.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/aurora-portal/src/server/Compute/helpers/imageHelpers.test.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/aurora-portal/src/server/Network/types/network.ts (1)
96-104: Add explicit null-focused tests for the newly nullable fields.Lines 96-97 and Line 104 now allow
null, but current tests only prove omission-based success. Please addsafeParsecases with explicitnullforipv4_address_scope,ipv6_address_scope, and"provider:physical_network"(and omission forport_security_enabledon Line 101) to lock in this behavior.Proposed test additions
diff --git a/apps/aurora-portal/src/server/Network/types/network.test.ts b/apps/aurora-portal/src/server/Network/types/network.test.ts @@ it("should validate with only required fields", () => { @@ expect(result.success).toBe(true) }) + + it("should validate nullable address scopes and provider physical network", () => { + const result = NetworkSchema.safeParse({ + admin_state_up: true, + created_at: "2026-03-10T10:00:00Z", + id: "network-3", + mtu: 1450, + name: "private-network-nullables", + project_id: "project-1", + "router:external": false, + shared: false, + status: "DOWN", + tenant_id: "tenant-1", + ipv4_address_scope: null, + ipv6_address_scope: null, + "provider:physical_network": null, + }) + + expect(result.success).toBe(true) + })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/aurora-portal/src/server/Network/types/network.ts` around lines 96 - 104, Tests don't assert explicit nulls for the newly nullable fields ipv4_address_scope, ipv6_address_scope and "provider:physical_network" and only assert omission; add safeParse-based tests that pass explicit null for these three fields to the existing schema tests (use the same schema used in network.ts), and ensure there remains a test that omits port_security_enabled to confirm omission still validates; reference the field names ipv4_address_scope, ipv6_address_scope, "provider:physical_network" and the schema under test when adding these safeParse cases.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/aurora-portal/src/server/Network/types/network.ts`:
- Around line 96-104: Tests don't assert explicit nulls for the newly nullable
fields ipv4_address_scope, ipv6_address_scope and "provider:physical_network"
and only assert omission; add safeParse-based tests that pass explicit null for
these three fields to the existing schema tests (use the same schema used in
network.ts), and ensure there remains a test that omits port_security_enabled to
confirm omission still validates; reference the field names ipv4_address_scope,
ipv6_address_scope, "provider:physical_network" and the schema under test when
adding these safeParse cases.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3fe9b897-7803-4c0e-b56e-09e5eba6e035
📒 Files selected for processing (5)
apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/-components/-table/List.test.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/-components/-table/SecurityGroupListContainer.test.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/-components/-table/SecurityGroupListContainer.tsxapps/aurora-portal/src/server/Network/types/network.test.tsapps/aurora-portal/src/server/Network/types/network.ts
💤 Files with no reviewable changes (1)
- apps/aurora-portal/src/server/Network/types/network.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/-components/-table/SecurityGroupListContainer.tsx
vlad-schur-external-sap
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 28 out of 34 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <InputGroup className="flex w-full items-end sm:w-auto"> | ||
| <Select | ||
| value={sortSettings.sortBy} | ||
| onChange={(value) => onSort({ ...sortSettings, sortBy: value ?? "", sortDirection })} |
There was a problem hiding this comment.
SortSettings.sortBy is optional, but the new handler coerces an undefined selection to an empty string (value ?? ""). Passing "" as a sort key can produce invalid API requests or inconsistent behavior; it’s safer to propagate undefined (or keep the previous sortBy) when no value is selected.
Suggested change
| onChange={(value) => onSort({ ...sortSettings, sortBy: value ?? "", sortDirection })} | |
| onChange={(value) => onSort({ ...sortSettings, sortBy: value, sortDirection })} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Addressed a set of UI improvements and interaction fixes for the Floating IPs list and detail pages, including layout corrections, accessibility enhancements, and DataGrid row interactivity.
Changes Made
Related Issues
Testing Instructions
pnpm ipnpm run testChecklist
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Style
Accessibility