made changes related to enhancement: Move Report Issue to sidebar (bottom) #424
Conversation
WalkthroughSmall UX, config, and docs updates: README links adjusted; API version timestamp updated; Docker Compose Redis container_name changed; UI tweaks include container date formatting, sidebar class and moving user actions to bottom, password input autocomplete prop, and minor CSS whitespace. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Sidebar as Sidebar (left)
participant Dropdown as User Dropdown
participant Handlers as Action Handlers
Note over Sidebar,Dropdown: Before — actions lived in Dropdown
User->>Dropdown: open menu
Dropdown->>Handlers: handleReportIssue() / handleHelp() / handleSponsor()
Note over Sidebar,Dropdown: After — actions moved to bottom of Sidebar
User->>Sidebar: Click "Sponsor"
Sidebar->>Handlers: handleSponsor()
User->>Sidebar: Click "Help"
Sidebar->>Handlers: handleHelp()
User->>Sidebar: Click "Report Issue"
Sidebar->>Handlers: handleReportIssue()
note right of Handlers #DFF2E1: Handlers unchanged — only UI placement moved.
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
view/components/layout/nav-user.tsx (1)
215-223: Duplicate actions: Sponsor/Help exist in both dropdown and sidebar.The PR goal is to move Report Issue to sidebar bottom. Remove duplicates from the dropdown (or gate one set).
Apply:
- <DropdownMenuItem onClick={handleSponsor}> - <Heart className="text-red-500" /> - {t('user.menu.sponsor')} - </DropdownMenuItem> - <DropdownMenuItem onClick={handleHelp}> - <HelpCircle /> - {t('user.menu.help')} - </DropdownMenuItem> + {/* Sponsor/Help moved to bottom of sidebar */}Also applies to: 233-245
🧹 Nitpick comments (5)
view/app/containers/components/table.tsx (1)
86-89: Avoid hardcodingen-US; respect user locale and parse robustly.Use default locale (or app locale) and
Number(...)to parse.Apply:
- ? new Intl.DateTimeFormat('en-US', { month: 'short', day: '2-digit', year: 'numeric' }).format( - new Date(parseInt(container.created) * 1000) - ) + ? new Intl.DateTimeFormat(undefined, { month: 'short', day: '2-digit', year: 'numeric' }).format( + new Date(Number(container.created) * 1000) + )view/components/layout/nav-user.tsx (2)
42-52: Bug: wrong localStorage key for sidebar state.
SIDEBAR_STORAGE_KEYhere is a string literal; the actual key used by the sidebar issidebar_state. Current logout won’t clear it.Apply:
- 'SIDEBAR_STORAGE_KEY', + 'sidebar_state',
86-89: Security: addnoopener,noreferrertowindow.open.Prevents
window.openerabuse and referrer leakage.Apply:
- window.open('https://github.com/sponsors/raghavyuva', '_blank'); + window.open('https://github.com/sponsors/raghavyuva', '_blank', 'noopener,noreferrer'); ... - window.open(url, '_blank'); + window.open(url, '_blank', 'noopener,noreferrer'); ... - window.open('https://docs.nixopus.com', '_blank'); + window.open('https://docs.nixopus.com', '_blank', 'noopener,noreferrer');Also applies to: 154-156, 158-160
view/components/ui/password-input-field.tsx (2)
24-31: Minor UX/a11y: keep input focus during toggle and respect disabled state.Prevent focus shift on mousedown and disable the toggle when the input is disabled.
Example (outside changed lines):
// inside component const disabled = props.disabled; <button type="button" onClick={() => setShowPassword((v) => !v)} onMouseDown={(e) => e.preventDefault()} // keep caret/focus in input disabled={disabled} className="text-muted-foreground hover:text-foreground absolute inset-y-0 right-2 my-auto inline-flex h-6 w-6 items-center justify-center" aria-label={showPassword ? 'Hide password' : 'Show password'} aria-pressed={showPassword} > {showPassword ? <EyeOff className="h-5 w-5" aria-hidden /> : <Eye className="h-5 w-5" aria-hidden />} </button>
12-12: autoComplete surfaced — set new-password on creation flowsPasswordInputField defaults to 'current-password'; explicitly set autoComplete="new-password" for fields that create or reset passwords.
- Default: view/components/ui/password-input-field.tsx:22 — autoComplete={autoComplete ?? 'current-password'}.
- Already set: view/app/settings/teams/components/AddMember.tsx:132 — autoComplete="new-password".
- Update to autoComplete="new-password": view/app/register/page.tsx:71,80; view/app/reset-password/components/ResetPasswordForm.tsx:108,125.
- Login flow OK with default: view/components/features/login-form.tsx:67.
- Verify intent for view/app/settings/notifications/components/channelTab.tsx:236 — set new-password only if this is a new/change password field.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
README.md(2 hunks)api/api/versions.json(1 hunks)docker-compose.yml(1 hunks)view/app/containers/components/table.tsx(1 hunks)view/app/globals.css(1 hunks)view/components/layout/app-sidebar.tsx(1 hunks)view/components/layout/nav-user.tsx(1 hunks)view/components/ui/password-input-field.tsx(1 hunks)view/components/ui/sidebar.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
view/components/ui/sidebar.tsx (1)
view/lib/utils.ts (1)
cn(4-6)
view/components/ui/password-input-field.tsx (1)
view/lib/utils.ts (1)
cn(4-6)
🔇 Additional comments (9)
api/api/versions.json (1)
6-6: Release date bump looks good.ISO-8601 format with offset; no issues.
view/app/globals.css (1)
526-527: No functional changes.Extra trailing newlines are fine if your formatter allows them.
README.md (2)
14-16: Roadmap link addition is good.Public docs navigation improved.
84-85: Trailing blank line OK.No impact.
view/components/ui/sidebar.tsx (1)
434-441: Comment-only change acknowledged.Bottom anchoring is already supported via
flex-col+mt-auto.view/components/layout/app-sidebar.tsx (1)
154-154: Verify UX ofcollapsible="icon"on desktop.Switching from default off-canvas to icon-collapse changes affordance widths; ensure no regressions in nav labels and bottom-anchored items.
docker-compose.yml (1)
65-65: No action required — old container name not referenced.
Search shows no occurrences of "nixopus-redis-container" and only references to "nixopus-redis" (docker-compose.yml, helpers/config*.yaml, api/internal/config/config_test.go, docs/contributing/self-hosting.md).view/components/layout/nav-user.tsx (1)
3-3: Translations: ensure keys exist for new sidebar items.
File: view/components/layout/nav-user.tsx — Verifyuser.menu.sponsor,user.menu.help, anduser.menu.reportIssueare present in all supported locale files; add missing translations.view/components/ui/password-input-field.tsx (1)
19-23: Prop spread order is correct; explicit overrides win.Spreading
...propsbeforetypeandautoCompleteenforces invariants (password toggle, default autocomplete) while still honoring consumer‑providedautoComplete. Looks good.
| <SidebarMenuItem className="mt-auto" onClick={handleSponsor}> | ||
| <Heart className="text-red-500" /> | ||
| {t('user.menu.sponsor')} | ||
| </SidebarMenuItem> | ||
| <SidebarMenuItem onClick={handleHelp}> | ||
| <HelpCircle /> | ||
| {t('user.menu.help')} | ||
| </SidebarMenuItem> | ||
| <SidebarMenuItem onClick={handleReportIssue}> | ||
| <AlertCircle /> | ||
| {t('user.menu.reportIssue')} | ||
| </SidebarMenuItem> | ||
| </SidebarMenu> |
There was a problem hiding this comment.
A11y: use button components for interactive list items.
Clickable
SidebarMenuButton.
Apply:
- <SidebarMenuItem className="mt-auto" onClick={handleSponsor}>
- <Heart className="text-red-500" />
- {t('user.menu.sponsor')}
- </SidebarMenuItem>
- <SidebarMenuItem onClick={handleHelp}>
- <HelpCircle />
- {t('user.menu.help')}
- </SidebarMenuItem>
- <SidebarMenuItem onClick={handleReportIssue}>
- <AlertCircle />
- {t('user.menu.reportIssue')}
- </SidebarMenuItem>
+ <SidebarMenuItem className="mt-auto">
+ <SidebarMenuButton onClick={handleSponsor}>
+ <Heart className="text-red-500" />
+ {t('user.menu.sponsor')}
+ </SidebarMenuButton>
+ </SidebarMenuItem>
+ <SidebarMenuItem>
+ <SidebarMenuButton onClick={handleHelp}>
+ <HelpCircle />
+ {t('user.menu.help')}
+ </SidebarMenuButton>
+ </SidebarMenuItem>
+ <SidebarMenuItem>
+ <SidebarMenuButton onClick={handleReportIssue}>
+ <AlertCircle />
+ {t('user.menu.reportIssue')}
+ </SidebarMenuButton>
+ </SidebarMenuItem>📝 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.
| <SidebarMenuItem className="mt-auto" onClick={handleSponsor}> | |
| <Heart className="text-red-500" /> | |
| {t('user.menu.sponsor')} | |
| </SidebarMenuItem> | |
| <SidebarMenuItem onClick={handleHelp}> | |
| <HelpCircle /> | |
| {t('user.menu.help')} | |
| </SidebarMenuItem> | |
| <SidebarMenuItem onClick={handleReportIssue}> | |
| <AlertCircle /> | |
| {t('user.menu.reportIssue')} | |
| </SidebarMenuItem> | |
| </SidebarMenu> | |
| <SidebarMenuItem className="mt-auto"> | |
| <SidebarMenuButton onClick={handleSponsor}> | |
| <Heart className="text-red-500" /> | |
| {t('user.menu.sponsor')} | |
| </SidebarMenuButton> | |
| </SidebarMenuItem> | |
| <SidebarMenuItem> | |
| <SidebarMenuButton onClick={handleHelp}> | |
| <HelpCircle /> | |
| {t('user.menu.help')} | |
| </SidebarMenuButton> | |
| </SidebarMenuItem> | |
| <SidebarMenuItem> | |
| <SidebarMenuButton onClick={handleReportIssue}> | |
| <AlertCircle /> | |
| {t('user.menu.reportIssue')} | |
| </SidebarMenuButton> | |
| </SidebarMenuItem> | |
| </SidebarMenu> |
🤖 Prompt for AI Agents
In view/components/layout/nav-user.tsx around lines 233 to 245, the interactive
SidebarMenuItem list items are implemented as clickable <li> elements which
breaks keyboard/focus semantics; replace each interactive SidebarMenuItem with
the provided SidebarMenuButton (or wrap the existing content in
SidebarMenuButton) so the onClick, className and icons/translations move to the
button element, preserving styling and handlers for handleSponsor, handleHelp,
and handleReportIssue; ensure the button receives the mt-auto class where used
and that tab/keyboard activation works as before.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
helpers/config.dev.yaml (1)
20-20: Use a resolvable hostname (not 127.0.0.1) for CADDY_ENDPOINThelpers/config.dev.yaml:20 currently sets CADDY_ENDPOINT to http://127.0.0.1:2019 — this will be unreachable from the API container if Caddy runs as a separate container. GetCaddyClient reads this value (api/internal/features/deploy/tasks/init.go:46) and production already defaults to http://nixopus-caddy:2019 (helpers/config.prod.yaml:20). Change the dev default to a container hostname (e.g. http://nixopus-caddy:2019) or require host.docker.internal (with host-gateway), and add startup validation/docs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
api/api/versions.json(1 hunks)api/internal/features/deploy/tasks/create.go(2 hunks)helpers/config.dev.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- api/api/versions.json
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-15T22:28:14.549Z
Learnt from: zhravan
PR: raghavyuva/nixopus#0
File: :0-0
Timestamp: 2025-09-15T22:28:14.549Z
Learning: In the nixopus codebase, there is a critical proxy loop issue in api/internal/features/deploy/tasks/create.go where Caddy domain registration uses the same domain as both the domain name and upstream host in AddDomainWithAutoTLS(), causing applications to never be properly served. The fix requires using proper upstream host configuration like "localhost" or "127.0.0.1" instead of the domain name.
Applied to files:
api/internal/features/deploy/tasks/create.go
🧬 Code graph analysis (1)
api/internal/features/deploy/tasks/create.go (1)
api/internal/types/task.go (1)
TaskPayload(3-9)
⏰ 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). (1)
- GitHub Check: GitGuardian Security Checks
🔇 Additional comments (1)
api/internal/features/deploy/tasks/create.go (1)
10-10: AppConfig initialized at startup — ensure other entrypoints call Initconfig.Init() is called in api/main.go (store := config.Init()) before queue.Init(); validateConfig requires SSH.Host, so HandleCreateDockerfileDeployment will have SSH.Host populated on normal startup. Ensure any alternate entrypoints or worker processes call config.Init() before registering/processing tasks.
| upstreamHost := config.AppConfig.SSH.Host | ||
|
|
||
| err = client.AddDomainWithAutoTLS(TaskPayload.Application.Domain, upstreamHost, port, caddygo.DomainOptions{}) | ||
| if err != nil { |
There was a problem hiding this comment.
Add guard + fallback to avoid proxy loop and empty upstream; handle reload error
If config.AppConfig.SSH.Host is empty or equals the app domain, the Caddy proxy loop can reappear or config will be invalid. Also, client.Reload() errors are ignored.
Apply this diff:
port, err := strconv.Atoi(containerResult.AvailablePort)
if err != nil {
taskCtx.LogAndUpdateStatus("Failed to convert port to int: "+err.Error(), shared_types.Failed)
return err
}
- upstreamHost := config.AppConfig.SSH.Host
+ upstreamHost := config.AppConfig.SSH.Host
+ if upstreamHost == "" || upstreamHost == TaskPayload.Application.Domain {
+ taskCtx.AddLog("Upstream host unset or equals domain; defaulting to 127.0.0.1")
+ upstreamHost = "127.0.0.1"
+ }
- err = client.AddDomainWithAutoTLS(TaskPayload.Application.Domain, upstreamHost, port, caddygo.DomainOptions{})
+ err = client.AddDomainWithAutoTLS(TaskPayload.Application.Domain, upstreamHost, port, caddygo.DomainOptions{})
if err != nil {
fmt.Println("Failed to add domain: ", err)
taskCtx.LogAndUpdateStatus("Failed to add domain: "+err.Error(), shared_types.Failed)
return err
}
- client.Reload()
+ if err := client.Reload(); err != nil {
+ taskCtx.LogAndUpdateStatus("Failed to reload Caddy: "+err.Error(), shared_types.Failed)
+ return err
+ }
return nil📝 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.
| upstreamHost := config.AppConfig.SSH.Host | |
| err = client.AddDomainWithAutoTLS(TaskPayload.Application.Domain, upstreamHost, port, caddygo.DomainOptions{}) | |
| if err != nil { | |
| port, err := strconv.Atoi(containerResult.AvailablePort) | |
| if err != nil { | |
| taskCtx.LogAndUpdateStatus("Failed to convert port to int: "+err.Error(), shared_types.Failed) | |
| return err | |
| } | |
| upstreamHost := config.AppConfig.SSH.Host | |
| if upstreamHost == "" || upstreamHost == TaskPayload.Application.Domain { | |
| taskCtx.AddLog("Upstream host unset or equals domain; defaulting to 127.0.0.1") | |
| upstreamHost = "127.0.0.1" | |
| } | |
| err = client.AddDomainWithAutoTLS(TaskPayload.Application.Domain, upstreamHost, port, caddygo.DomainOptions{}) | |
| if err != nil { | |
| fmt.Println("Failed to add domain: ", err) | |
| taskCtx.LogAndUpdateStatus("Failed to add domain: "+err.Error(), shared_types.Failed) | |
| return err | |
| } | |
| if err := client.Reload(); err != nil { | |
| taskCtx.LogAndUpdateStatus("Failed to reload Caddy: "+err.Error(), shared_types.Failed) | |
| return err | |
| } | |
| return nil |
🤖 Prompt for AI Agents
In api/internal/features/deploy/tasks/create.go around lines 85-88, add a guard
so upstreamHost is not empty and does not equal the app domain: if
config.AppConfig.SSH.Host == "" || config.AppConfig.SSH.Host ==
TaskPayload.Application.Domain then set upstreamHost to a safe fallback like
"127.0.0.1" (so the Caddy upstream is valid and avoids proxy loops) before
calling AddDomainWithAutoTLS; also ensure you check and handle the error
returned by client.Reload() (return or log the error) instead of ignoring it so
reload failures are surfaced.
Summary by CodeRabbit
New Features
Style
Bug Fixes
Documentation
Chores