fix(console): ui audit fixes — forms, VNC, events, status badges#4
Open
IvanHunters wants to merge 13 commits into
Open
fix(console): ui audit fixes — forms, VNC, events, status badges#4IvanHunters wants to merge 13 commits into
IvanHunters wants to merge 13 commits into
Conversation
added 12 commits
May 11, 2026 20:19
In edit mode, Back and Cancel navigate to the resource detail page instead of relying on browser history, which is unstable on direct links. In detail view, Back navigates to the resource list page. Signed-off-by: ohotnikov.ivan <ohotnikov.ivan@e-queo.net>
Clicking a disabled module previously navigated to the marketplace, which created a standalone ApplicationInstance bypassing the tenant reconciler. Now it navigates to the tenant edit form where the user can enable the module via the appropriate spec flag. Signed-off-by: ohotnikov.ivan <ohotnikov.ivan@e-queo.net>
Fetches /oauth2/userinfo on startup alongside the app config and passes the email/user field to AppShell. Falls back gracefully when the endpoint is unavailable (dev mode without auth). Signed-off-by: ohotnikov.ivan <ohotnikov.ivan@e-queo.net>
Covers BootBox, ExternalDNS, FoundationDB, Info, OpenSearch, SeaweedFS, and TCPBalancer. TCPBalancer and OpenSearch use Simple Icons (haproxy, opensearch); the rest use Lucide fallbacks. Signed-off-by: ohotnikov.ivan <ohotnikov.ivan@e-queo.net>
loadConfig and loadUsername now abort after 5 seconds so the app always mounts even when the K8s API or oauth2-proxy is unreachable. Previously a hanging fetch would block Promise.all indefinitely, leaving the root div empty. Signed-off-by: ohotnikov.ivan <ohotnikov.ivan@e-queo.net>
…etwork for TCPBalancer haproxy does not exist in the Simple Icons package (cdn.jsdelivr.net returns 404), causing a broken mask-image in the sidebar. Move TCPBalancer to the Lucide fallback using the Network icon. Signed-off-by: ohotnikov.ivan <ohotnikov.ivan@e-queo.net>
SourceField now fetches existing VMDisks from the tenant namespace and available images from cozy-public (PVCs with vm-default-images- prefix), rendering them as native selects instead of plain text inputs. Signed-off-by: ohotnikov.ivan <ohotnikov.ivan@e-queo.net>
- Add cursor-pointer to toolbar buttons (Tailwind resets cursor to auto) - Enable resizeSession so noVNC sends SetDesktopSize to the VM guest, matching the container resolution instead of upscaling a low-res framebuffer Signed-off-by: ohotnikov.ivan <ohotnikov.ivan@e-queo.net>
Resources with deletionTimestamp set now show a "Terminating" muted badge instead of the Ready/NotReady condition badge in both the list and detail views. Signed-off-by: ohotnikov.ivan <ohotnikov.ivan@e-queo.net>
Events tab now also loads pods and PVCs by the Helm release instance label (app.kubernetes.io/instance) so resources like DataVolume that share the HelmRelease name appear in the event stream alongside app-label resources. Signed-off-by: ohotnikov.ivan <ohotnikov.ivan@e-queo.net>
- VMDiskWidget: remove `disks` from useEffect deps so k8s watch events do not re-trigger auto-select and overwrite a user's disk selection; also fixes first-click-does-nothing on the disks array AddButton - SchemaForm: emit getDefaultFormState on schema load so the parent spec is never empty when the user submits without touching any field (#5) - rjsf-templates: add cursor-pointer to AddButton, strip non-button RJSF props (uiSchema, registry, iconType) from IconButton to avoid React warnings Signed-off-by: ohotnikov.ivan <ohotnikov.ivan@e-queo.net>
…Selector Previously all services were fetched and filtered on the client. Now uses fieldSelector=spec.type=LoadBalancer matching the upstream dashboard behavior. Signed-off-by: ohotnikov.ivan <ohotnikov.ivan@e-queo.net>
📝 WalkthroughWalkthroughThe PR introduces user identity integration into the console app, fixes form state synchronization issues across multiple components, adds dynamic dropdown selection for disk and image fields sourced from Kubernetes resources, corrects navigation patterns across several pages, enhances status display to show termination state, expands resource event scoping to include Helm-managed resources, and applies UI polish to VNC connectivity and button styling. ChangesConsole App Enhancements
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces several enhancements to the console, including user identity fetching, improved navigation logic for edit modes, and more robust resource status reporting, such as the addition of a 'Terminating' state. Key technical improvements include dynamic disk and image selection in the SourceField component, optimized Kubernetes service listing using field selectors, and expanded event tracking for Helm-managed resources. Feedback provided identifies a potential data loss issue in SchemaForm where a stale ref could overwrite asynchronously loaded form data, and suggests refining the useEffect dependencies in VMDiskWidget to ensure reliable auto-selection when the disk list is populated after the initial render.
|
|
||
| const onChangeRef = useRef(onChange) | ||
| onChangeRef.current = onChange | ||
| const initialFormDataRef = useRef(formData) |
There was a problem hiding this comment.
The initialFormDataRef is initialized only once during the component's mount and is never updated. If formData is provided asynchronously by the parent (for example, after an API fetch), this ref will remain empty. When the schema is loaded and the useEffect runs, getDefaultFormState will use the stale empty object as a base, and the subsequent onChange(defaults) call will overwrite the parent's newly loaded data with just the schema defaults.
To fix this, ensure the ref is updated on every render so that the effect always uses the most recent formData when calculating the default state.
Suggested change
| const initialFormDataRef = useRef(formData) | |
| const initialFormDataRef = useRef(formData) | |
| initialFormDataRef.current = formData |
| } | ||
| }, [required, value, disks, isLoading, onChange]) | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [required, value, isLoading]) |
There was a problem hiding this comment.
The disks array should be included in the dependency array of this useEffect. If the disk list is initially empty and then populated via a background update (while isLoading remains false), the auto-selection logic will not trigger because the effect won't re-run. The concern mentioned in the comments about overwriting user selection is already mitigated by the !value check; if a user has already made a selection, the effect will simply do nothing. Including disks ensures that the first disk is automatically selected as soon as the list becomes non-empty.
Suggested change
| }, [required, value, isLoading]) | |
| }, [required, value, isLoading, disks]) |
- VncTab: reset desktopSize on reconnect to avoid stale aspect-ratio locking the layout behind the loading overlay - VMDiskWidget: replace omitted-dep workaround with an autoSelectedRef flag so auto-select fires on initial disk load while respecting user changes; restores all proper deps so the linter is satisfied - SchemaForm: guard defaults emission with emittedSchemaRef so an unexpected schema identity change cannot overwrite in-progress user input - ResourceQuotasField: sync KnownRowEditor local state (checked, localNum) with external value changes via expectedValueRef to distinguish own onChange events from parent-driven resets (e.g. yaml/form mode switch) - ModulesPage: remove hardcoded "root" tenant fallback; disable card with aria-disabled when no tenant is selected to prevent silent 404 navigation Signed-off-by: ohotnikov.ivan <ohotnikov.ivan@e-queo.net>
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/console/src/routes/detail/EventsTab.tsx (1)
71-82:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPrevent transient false empty state while Pods/PVCs are still loading.
Line 175 can show “No events found” before related Pod/PVC names arrive, because only
eventsListloading is checked. This can briefly hide valid events.💡 Suggested fix
- const { data: podsList } = useK8sList<K8sPod>( + const { data: podsList, isLoading: isPodsLoading } = useK8sList<K8sPod>( ... - const { data: helmPodsList } = useK8sList<K8sPod>( + const { data: helmPodsList, isLoading: isHelmPodsLoading } = useK8sList<K8sPod>( ... - const { data: pvcList } = useK8sList<K8sPVC>( + const { data: pvcList, isLoading: isPvcLoading } = useK8sList<K8sPVC>( ... - const { data: helmPvcList } = useK8sList<K8sPVC>( + const { data: helmPvcList, isLoading: isHelmPvcLoading } = useK8sList<K8sPVC>( ... + const isRelatedResourcesLoading = + isPodsLoading || isHelmPodsLoading || isPvcLoading || isHelmPvcLoading + - if (isLoading) { + if (isLoading || isRelatedResourcesLoading) {Also applies to: 85-96, 99-110, 113-124, 163-175
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/console/src/routes/detail/EventsTab.tsx` around lines 71 - 82, The empty-state check currently only looks at eventsList loading and can render “No events found” before related pods/pvcs arrive; update the render/conditional logic in EventsTab to consider the loading state of the related useK8sList hooks (podsList, pvcsList) together with eventsList (e.g., use podsList?.isLoading or pvcsList?.isLoading) so you only show the “No events found” message when eventsList is loaded AND neither podsList nor pvcsList are still loading (or when all three are loaded and empty). Adjust the same checks where similar empty-state messages occur (the other blocks referencing useK8sList for pods/pvcs/events) so they combine the loading flags rather than relying on eventsList alone; keep tenantNamespace and labelSelector usage unchanged.
🧹 Nitpick comments (1)
apps/console/src/components/rjsf-templates.tsx (1)
22-23: 💤 Low valueConsider using underscore prefix for intentionally unused variables.
The current pattern works correctly by excluding
uiSchemaandregistryfrom the button props spread. However, using underscore prefixes is more idiomatic for signaling intentionally unused variables:- // eslint-disable-next-line `@typescript-eslint/no-unused-vars` - const { icon, className, uiSchema, registry, iconType, ...btnProps } = props + const { icon, className, uiSchema: _uiSchema, registry: _registry, iconType, ...btnProps } = propsThis convention makes the intent explicit without requiring eslint suppressions.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/console/src/components/rjsf-templates.tsx` around lines 22 - 23, Rename the intentionally unused destructured variables to use an underscore prefix (e.g., change uiSchema and registry to _uiSchema and _registry) in the props destructuring where const { icon, className, uiSchema, registry, iconType, ...btnProps } = props is defined, remove the now-unnecessary eslint-disable-next-line comment, and keep the rest of the spread into btnProps unchanged so the unused intent is explicit and ESLint warnings are avoided.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/console/src/components/SchemaForm.tsx`:
- Around line 171-183: The effect currently uses a one-time captured
initialFormDataRef (set from formData at mount) when calling
getDefaultFormState, which can produce stale defaults; update the effect to use
the latest formData as the base before computing defaults (either assign
initialFormDataRef.current = formData at the start of the useEffect or pass the
current formData directly into getDefaultFormState) so emitted defaults reflect
the latest values; keep the rest of the emittedSchemaRef and onChangeRef usage
the same to avoid extra re-renders.
In `@apps/console/src/routes/ModulesPage.tsx`:
- Around line 117-119: The current card uses a Link with to={canNavigate ?
target : "#"} so when canNavigate is false keyboard users can still
focus/activate it and change the URL hash; update the render so that when
canNavigate is false you do NOT render a navigable Link (render a
non-interactive container like a div or a role="group" element) or render a true
non-focusable element (remove href/to and set tabIndex={-1},
aria-disabled="true", and prevent activation handlers) instead; locate the card
JSX that uses to, target and canNavigate in ModulesPage.tsx and change the
conditional to render a non-link container when !canNavigate, preserving the
same classes and accessible aria-disabled state.
---
Outside diff comments:
In `@apps/console/src/routes/detail/EventsTab.tsx`:
- Around line 71-82: The empty-state check currently only looks at eventsList
loading and can render “No events found” before related pods/pvcs arrive; update
the render/conditional logic in EventsTab to consider the loading state of the
related useK8sList hooks (podsList, pvcsList) together with eventsList (e.g.,
use podsList?.isLoading or pvcsList?.isLoading) so you only show the “No events
found” message when eventsList is loaded AND neither podsList nor pvcsList are
still loading (or when all three are loaded and empty). Adjust the same checks
where similar empty-state messages occur (the other blocks referencing
useK8sList for pods/pvcs/events) so they combine the loading flags rather than
relying on eventsList alone; keep tenantNamespace and labelSelector usage
unchanged.
---
Nitpick comments:
In `@apps/console/src/components/rjsf-templates.tsx`:
- Around line 22-23: Rename the intentionally unused destructured variables to
use an underscore prefix (e.g., change uiSchema and registry to _uiSchema and
_registry) in the props destructuring where const { icon, className, uiSchema,
registry, iconType, ...btnProps } = props is defined, remove the now-unnecessary
eslint-disable-next-line comment, and keep the rest of the spread into btnProps
unchanged so the unused intent is explicit and ESLint warnings are avoided.
🪄 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: df226560-7e7a-410c-b906-0678d7276cf1
📒 Files selected for processing (16)
apps/console/src/App.tsxapps/console/src/components/ResourceQuotasField.tsxapps/console/src/components/SchemaForm.tsxapps/console/src/components/SourceField.tsxapps/console/src/components/VMDiskWidget.tsxapps/console/src/components/rjsf-templates.tsxapps/console/src/lib/config.tsapps/console/src/lib/sidebar-icons.tsxapps/console/src/main.tsxapps/console/src/routes/ApplicationListPage.tsxapps/console/src/routes/ApplicationOrderPage.tsxapps/console/src/routes/ExternalIpsPage.tsxapps/console/src/routes/ModulesPage.tsxapps/console/src/routes/detail/ApplicationDetailPage.tsxapps/console/src/routes/detail/EventsTab.tsxapps/console/src/routes/detail/VncTab.tsx
Comment on lines
+171
to
+183
| const initialFormDataRef = useRef(formData) | ||
| const emittedSchemaRef = useRef<RJSFSchema | null>(null) | ||
|
|
||
| // Emit defaults to parent once per schema so spec is never empty on first submit. | ||
| // Uses initialFormDataRef so edit-mode existing values are preserved as base. | ||
| // emittedSchemaRef prevents re-running on unrelated re-renders and avoids | ||
| // overwriting user data if the schema object changes identity unexpectedly. | ||
| useEffect(() => { | ||
| if (!schema || Object.keys(schema).length === 0) return | ||
| if (emittedSchemaRef.current === schema) return | ||
| emittedSchemaRef.current = schema | ||
| const defaults = getDefaultFormState(validator, schema, initialFormDataRef.current ?? {}, schema) | ||
| onChangeRef.current(defaults) |
There was a problem hiding this comment.
Use the latest formData as the defaults base.
Line 171 captures formData only once, but Line 182 computes defaults later. If formData changes before emission (or schema changes in-place), defaults can be derived from stale data and overwrite expected values.
💡 Suggested fix
- const initialFormDataRef = useRef(formData)
+ const latestFormDataRef = useRef(formData)
+ latestFormDataRef.current = formData
const emittedSchemaRef = useRef<RJSFSchema | null>(null)
@@
- // Uses initialFormDataRef so edit-mode existing values are preserved as base.
+ // Uses latest formData so async-loaded edit values are preserved as base.
@@
- const defaults = getDefaultFormState(validator, schema, initialFormDataRef.current ?? {}, schema)
+ const defaults = getDefaultFormState(validator, schema, latestFormDataRef.current ?? {}, schema)📝 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 initialFormDataRef = useRef(formData) | |
| const emittedSchemaRef = useRef<RJSFSchema | null>(null) | |
| // Emit defaults to parent once per schema so spec is never empty on first submit. | |
| // Uses initialFormDataRef so edit-mode existing values are preserved as base. | |
| // emittedSchemaRef prevents re-running on unrelated re-renders and avoids | |
| // overwriting user data if the schema object changes identity unexpectedly. | |
| useEffect(() => { | |
| if (!schema || Object.keys(schema).length === 0) return | |
| if (emittedSchemaRef.current === schema) return | |
| emittedSchemaRef.current = schema | |
| const defaults = getDefaultFormState(validator, schema, initialFormDataRef.current ?? {}, schema) | |
| onChangeRef.current(defaults) | |
| const latestFormDataRef = useRef(formData) | |
| latestFormDataRef.current = formData | |
| const emittedSchemaRef = useRef<RJSFSchema | null>(null) | |
| // Emit defaults to parent once per schema so spec is never empty on first submit. | |
| // Uses latest formData so async-loaded edit values are preserved as base. | |
| // emittedSchemaRef prevents re-running on unrelated re-renders and avoids | |
| // overwriting user data if the schema object changes identity unexpectedly. | |
| useEffect(() => { | |
| if (!schema || Object.keys(schema).length === 0) return | |
| if (emittedSchemaRef.current === schema) return | |
| emittedSchemaRef.current = schema | |
| const defaults = getDefaultFormState(validator, schema, latestFormDataRef.current ?? {}, schema) | |
| onChangeRef.current(defaults) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/console/src/components/SchemaForm.tsx` around lines 171 - 183, The
effect currently uses a one-time captured initialFormDataRef (set from formData
at mount) when calling getDefaultFormState, which can produce stale defaults;
update the effect to use the latest formData as the base before computing
defaults (either assign initialFormDataRef.current = formData at the start of
the useEffect or pass the current formData directly into getDefaultFormState) so
emitted defaults reflect the latest values; keep the rest of the
emittedSchemaRef and onChangeRef usage the same to avoid extra re-renders.
Comment on lines
+117
to
+119
| to={canNavigate ? target : "#"} | ||
| aria-disabled={!canNavigate} | ||
| className={`flex items-center gap-3 rounded-lg border border-slate-200 bg-white px-4 py-3 transition-shadow ${canNavigate ? "hover:shadow-sm" : "opacity-50 cursor-not-allowed pointer-events-none"}`} |
There was a problem hiding this comment.
Disabled cards are still keyboard-activatable via Link.
Line 117 uses to="#" for disabled cards, so keyboard users can still activate it and change URL hash. Render a non-link container when !canNavigate, or fully disable focus/activation.
Suggested fix
- return (
- <Link
- to={canNavigate ? target : "#"}
- aria-disabled={!canNavigate}
- className={`flex items-center gap-3 rounded-lg border border-slate-200 bg-white px-4 py-3 transition-shadow ${canNavigate ? "hover:shadow-sm" : "opacity-50 cursor-not-allowed pointer-events-none"}`}
- >
+ const cardClassName = `flex items-center gap-3 rounded-lg border border-slate-200 bg-white px-4 py-3 transition-shadow ${canNavigate ? "hover:shadow-sm" : "opacity-50 cursor-not-allowed"}`
+
+ if (!canNavigate) {
+ return (
+ <div
+ aria-disabled="true"
+ className={cardClassName}
+ >
+ {/* content */}
+ </div>
+ )
+ }
+
+ return (
+ <Link
+ to={target}
+ className={cardClassName}
+ >
...
</Link>
)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/console/src/routes/ModulesPage.tsx` around lines 117 - 119, The current
card uses a Link with to={canNavigate ? target : "#"} so when canNavigate is
false keyboard users can still focus/activate it and change the URL hash; update
the render so that when canNavigate is false you do NOT render a navigable Link
(render a non-interactive container like a div or a role="group" element) or
render a true non-focusable element (remove href/to and set tabIndex={-1},
aria-disabled="true", and prevent activation handlers) instead; locate the card
JSX that uses to, target and canNavigate in ModulesPage.tsx and change the
conditional to render a non-link container when !canNavigate, preserving the
same classes and accessible aria-disabled state.
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
1 participant
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
TCPBalancer icon with Lucide Network
detail, not browser history)
first-click-does-nothing on array AddButton; emit schema defaults on
mount so spec is never empty on first submit
proper guest resolution
Test plan
disk from the dropdown persists the selection
non-empty spec with schema defaults
container size
Summary by CodeRabbit
New Features
Bug Fixes