Repository navigation
feat(devices): add cloud device connection controls - #1187
Merged
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR adds device session and cloud device management capabilities: backend endpoints for terminal/code-server sessions, a reworked settings UI for managing cloud devices with session launching and device operations, a client API layer wrapping these endpoints, and comprehensive documentation and tests. ChangesDevice Session and Cloud Device Management
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
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 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (1)
wework/src/types/devices.ts (1)
34-40: ⚡ Quick winTighten
CloudDeviceResponseunions to match device contract.Line 38 and Line 39 use
string, which loses type safety vsDeviceInfo’s constrained unions. Reusing the same union domain helps catch API drift earlier.♻️ Proposed refactor
export interface CloudDeviceResponse { id: number device_id: string name: string - status: string - device_type: string + status: DeviceInfo['status'] + device_type: DeviceInfo['device_type'] message: string }🤖 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 `@wework/src/types/devices.ts` around lines 34 - 40, CloudDeviceResponse currently types device_type and status as plain strings which loses the constrained unions defined on DeviceInfo; update CloudDeviceResponse to reuse DeviceInfo's union types for those fields (e.g. import or reference the DeviceInfo device_type and status unions) so the API contract matches DeviceInfo and catches drift early—adjust the interface declaration for CloudDeviceResponse to replace device_type: string and status: string with the corresponding DeviceInfo union types.
🤖 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 `@backend/app/api/endpoints/devices.py`:
- Around line 485-488: When mapping a caught DeviceSessionError to an
HTTPException in devices.py (the blocks that currently do raise
HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=str(exc))),
preserve exception causality by using exception chaining: raise the
HTTPException with the same status_code and detail but append "from exc" (e.g.,
raise HTTPException(..., detail=str(exc)) from exc). Apply this change to both
locations handling DeviceSessionError (the block around the existing raise at
lines ~485-488 and the similar block at ~526-529) so the original
DeviceSessionError appears in tracebacks.
- Around line 458-463: Add explicit return type annotations and the tracing
decorator to the new async endpoint handlers: annotate start_device_terminal
with -> DeviceSessionResponse and prepend the function with `@trace_async`(), and
do the same for the other async handler defined around lines 499-504 (apply
`@trace_async`() and add its correct response model return type). Ensure you
import trace_async if needed and place the decorator directly above each async
def (start_device_terminal and the other handler name) so tracing wraps the
entire coroutine.
- Line 479: Replace the magic literal project_id=0 in the two session-start
calls with a named module-level constant (e.g. SYSTEM_PROJECT_ID or
DEFAULT_PROJECT_ID); declare this constant at the top of the module and use it
in place of the literal in both locations (the two occurrences of project_id=0
in devices.py) so callers like the session-start invocations reference the
constant (e.g., DEFAULT_PROJECT_ID) instead of the hard-coded 0.
In `@wework/src/components/settings/AddCloudDeviceDialog.tsx`:
- Around line 66-74: The close button in AddCloudDeviceDialog
(data-testid="add-cloud-device-close") and other dialog buttons in the same
component are below the 44×44 mobile touch minimum; update their classNames to
include the mobile sizing utility classes (e.g., h-11 min-w-[44px]) and adjust
padding/margin as needed so the visual layout remains correct while ensuring the
touch target is at least 44×44; apply the same change to the other buttons in
this component (the ones referenced around the dialog action area) so all
interactive elements meet the h-11 min-w-[44px] guideline.
- Around line 35-36: The dialog currently uses hardcoded Chinese strings (e.g.,
the error message set via setError and several dialog labels/buttons) — import
useTranslation from '`@/hooks/useTranslation`' and the feature namespace, add i18n
keys for each copy used in AddCloudDeviceDialog (replace occurrences around
setError, the dialog title/body at the referenced ranges, and button/placeholder
text), and replace hardcoded strings with t('yourNamespace.key') calls (provide
sensible keys and use the translation fallback where needed); ensure
onCreatingChange?.(false) remains but the error text passed to setError is
t('yourNamespace.createFailed') instead of the literal Chinese string.
In `@wework/src/components/settings/ConnectionsSettingsPage.tsx`:
- Around line 76-93: Replace hardcoded UI copy in StatusPill with the
translation hook: import useTranslation from '`@/hooks/useTranslation`', call
const { t } = useTranslation('connections') (use a single namespace for this
feature, e.g. 'connections'), and replace the literals '在线' and '离线' with
t('status.online') and t('status.offline'); also localize the status dot aria
label if present. Do the same pattern for the other hardcoded labels mentioned
(ranges 168-173, 219-230, 487-534, 685-692, 775-788, 835-836): import the same
useTranslation namespace, add appropriate keys (e.g., status.online,
status.offline, connection.title, connection.description, etc.), and replace
literal strings with t(...) calls; ensure you import from
'`@/hooks/useTranslation`' (not 'react-i18next') and keep keys consistent within
the single namespace.
- Around line 110-119: The shared action button renders at h-8 (and another at
h-7 w-7) which violates mobile touch-target rules; update the reusable button
implementations (the JSX button that uses props testId, onClick, disabled, Icon,
label) to use className values that enforce at least 44×44 touch targets
(replace h-8/h-7 with h-11 and add min-w-[44px], keeping existing padding/visual
styles) and apply the same change to the other instance referenced around the
second button render so all consumers inherit the compliant size.
- Around line 461-464: The heading <h3> using onClick (the element with
className "cursor-pointer truncate text-sm font-semibold text-[`#2d2d2d`]" and
onClick={handleStartEdit}) is mouse-only; replace it with a semantic interactive
element (e.g., a <button>) or add full keyboard semantics. Move the
onClick={handleStartEdit}, title, and className to the new button, preserve
visual styles via the same classes (or convert any heading-specific styles), and
ensure the button has an accessible label (use its text content or aria-label)
so keyboard users can activate the rename action; if you choose to keep a
non-button element, add role="button", tabIndex={0}, and keydown handlers that
call handleStartEdit on Enter/Space.
- Around line 267-272: Clamp the metric percentage before using it in the bar
width and display: ensure the `value` used in the inline style (`style={{ width:
`${value ?? 0}%` }}`) and the text span (`{value === null ? '--%' :
`${value}%`}`) is clamped to the 0–100 range (e.g., compute a local `clamped =
Math.max(0, Math.min(100, value ?? 0))` or a small helper `clampPercent(value)`)
and use `clamped` for both the width and the displayed `${clamped}%` (while
still rendering '--%' when original `value` is null) so out-of-range values
cannot overflow the UI; update references in ConnectionsSettingsPage component
accordingly.
---
Nitpick comments:
In `@wework/src/types/devices.ts`:
- Around line 34-40: CloudDeviceResponse currently types device_type and status
as plain strings which loses the constrained unions defined on DeviceInfo;
update CloudDeviceResponse to reuse DeviceInfo's union types for those fields
(e.g. import or reference the DeviceInfo device_type and status unions) so the
API contract matches DeviceInfo and catches drift early—adjust the interface
declaration for CloudDeviceResponse to replace device_type: string and status:
string with the corresponding DeviceInfo union types.
🪄 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: 63c43c15-6fce-4e9b-88b8-ec9348ea6df6
📒 Files selected for processing (16)
backend/app/api/endpoints/devices.pydocs/en/user-guide/ai-devices/local-device-support.mddocs/zh/user-guide/ai-devices/local-device-support.mdwework/src/api/devices.test.tswework/src/api/devices.tswework/src/components/layout/DesktopWorkbenchLayout.test.tsxwework/src/components/settings/AddCloudDeviceDialog.tsxwework/src/components/settings/ConnectionsSettingsPage.test.tsxwework/src/components/settings/ConnectionsSettingsPage.tsxwework/src/pages/PluginManagementPage.tsxwework/src/pages/PluginsPage.tsxwework/src/types/devices.tswework/tailwind.config.jswework/tsconfig.app.jsonwework/tsconfig.jsonwework/vite.config.ts
Comment on lines
+458
to
+463
| @router.post("/{device_id}/terminal", response_model=DeviceSessionResponse) | ||
| async def start_device_terminal( | ||
| device_id: str, | ||
| db: Session = Depends(get_db), | ||
| current_user: User = Depends(security.get_current_user), | ||
| ): |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add return type hints and async tracing decorators on the new endpoints.
Both new async handlers are missing explicit return annotations and endpoint-level tracing decorators.
As per coding guidelines, use type hints in Python code and apply @trace_async() to entire async functions.
Also applies to: 499-504
🧰 Tools
🪛 Ruff (0.15.14)
[warning] 461-461: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
[warning] 462-462: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
🤖 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 `@backend/app/api/endpoints/devices.py` around lines 458 - 463, Add explicit
return type annotations and the tracing decorator to the new async endpoint
handlers: annotate start_device_terminal with -> DeviceSessionResponse and
prepend the function with `@trace_async`(), and do the same for the other async
handler defined around lines 499-504 (apply `@trace_async`() and add its correct
response model return type). Ensure you import trace_async if needed and place
the decorator directly above each async def (start_device_terminal and the other
handler name) so tracing wraps the entire coroutine.
| db=db, | ||
| user_id=current_user.id, | ||
| device_id=device_id, | ||
| project_id=0, |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Extract project_id=0 into a named constant.
The literal 0 is repeated in both session-start calls; define a module constant for clarity and contract intent.
♻️ Proposed refactor
DEFAULT_DEVICE_SESSION_PATH = "/home/ubuntu/.wegent-executor/workspace"
+DEFAULT_DEVICE_PROJECT_ID = 0
...
- project_id=0,
+ project_id=DEFAULT_DEVICE_PROJECT_ID,
...
- project_id=0,
+ project_id=DEFAULT_DEVICE_PROJECT_ID,As per coding guidelines, extract magic numbers to named constants in Python code.
Also applies to: 520-520
🤖 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 `@backend/app/api/endpoints/devices.py` at line 479, Replace the magic literal
project_id=0 in the two session-start calls with a named module-level constant
(e.g. SYSTEM_PROJECT_ID or DEFAULT_PROJECT_ID); declare this constant at the top
of the module and use it in place of the literal in both locations (the two
occurrences of project_id=0 in devices.py) so callers like the session-start
invocations reference the constant (e.g., DEFAULT_PROJECT_ID) instead of the
hard-coded 0.
Comment on lines
+485
to
+488
| raise HTTPException( | ||
| status_code=status.HTTP_400_BAD_REQUEST, | ||
| detail=str(exc), | ||
| ) |
Contributor
There was a problem hiding this comment.
Preserve exception causality when mapping session errors to HTTP errors.
Use exception chaining so traceback clearly shows the original DeviceSessionError.
🛠️ Proposed fix
except DeviceSessionError as exc:
- raise HTTPException(
+ raise HTTPException(
status_code=status.HTTP_400_BAD_REQUEST,
detail=str(exc),
- )
+ ) from exc
...
except DeviceSessionError as exc:
- raise HTTPException(
+ raise HTTPException(
status_code=status.HTTP_400_BAD_REQUEST,
detail=str(exc),
- )
+ ) from excAlso applies to: 526-529
🧰 Tools
🪛 Ruff (0.15.14)
[warning] 485-488: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
🤖 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 `@backend/app/api/endpoints/devices.py` around lines 485 - 488, When mapping a
caught DeviceSessionError to an HTTPException in devices.py (the blocks that
currently do raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST,
detail=str(exc))), preserve exception causality by using exception chaining:
raise the HTTPException with the same status_code and detail but append "from
exc" (e.g., raise HTTPException(..., detail=str(exc)) from exc). Apply this
change to both locations handling DeviceSessionError (the block around the
existing raise at lines ~485-488 and the similar block at ~526-529) so the
original DeviceSessionError appears in tracebacks.
Comment on lines
+35
to
+36
| setError(e instanceof Error ? e.message : '创建失败，请重试') | ||
| onCreatingChange?.(false) |
Contributor
There was a problem hiding this comment.
Move dialog copy and fallback error text into i18n keys.
This new dialog ships hardcoded Chinese strings instead of useTranslation, so non-Chinese locales won’t render localized UI consistently.
As per coding guidelines: Always import translations from '@/hooks/useTranslation', not from 'react-i18next', and use single namespace matching your feature.
Also applies to: 61-64, 91-92, 100-101
🤖 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 `@wework/src/components/settings/AddCloudDeviceDialog.tsx` around lines 35 -
36, The dialog currently uses hardcoded Chinese strings (e.g., the error message
set via setError and several dialog labels/buttons) — import useTranslation from
'`@/hooks/useTranslation`' and the feature namespace, add i18n keys for each copy
used in AddCloudDeviceDialog (replace occurrences around setError, the dialog
title/body at the referenced ranges, and button/placeholder text), and replace
hardcoded strings with t('yourNamespace.key') calls (provide sensible keys and
use the translation fallback where needed); ensure onCreatingChange?.(false)
remains but the error text passed to setError is t('yourNamespace.createFailed')
instead of the literal Chinese string.
Comment on lines
+66
to
+74
| <button | ||
| type="button" | ||
| data-testid="add-cloud-device-close" | ||
| onClick={onClose} | ||
| disabled={loading} | ||
| className="-mr-1 -mt-1 shrink-0 rounded-md p-1 text-[#9aa0a6] hover:bg-[#f5f5f5] hover:text-[#3c4043] disabled:cursor-not-allowed disabled:opacity-50" | ||
| > | ||
| <X className="h-4 w-4" /> | ||
| </button> |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Increase dialog button touch targets to mobile minimum size.
These controls are below 44×44, which will be hard to use on touch devices.
As per coding guidelines: All interactive elements on mobile must be at least 44px × 44px - use h-11 min-w-[44px] for buttons.
Also applies to: 84-99
🤖 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 `@wework/src/components/settings/AddCloudDeviceDialog.tsx` around lines 66 -
74, The close button in AddCloudDeviceDialog
(data-testid="add-cloud-device-close") and other dialog buttons in the same
component are below the 44×44 mobile touch minimum; update their classNames to
include the mobile sizing utility classes (e.g., h-11 min-w-[44px]) and adjust
padding/margin as needed so the visual layout remains correct while ensuring the
touch target is at least 44×44; apply the same change to the other buttons in
this component (the ones referenced around the dialog action area) so all
interactive elements meet the h-11 min-w-[44px] guideline.
Comment on lines
+76
to
93
| function StatusPill({ status }: { status: DeviceInfo['status'] }) { | ||
| const isOnline = status === 'online' | ||
|
|
||
| return ( | ||
| <span className="inline-flex items-center gap-2 text-xs text-[#6b6f76]"> | ||
| <span | ||
| className={`inline-flex items-center gap-1 rounded-full px-1.5 py-0.5 text-[11px] ${ | ||
| isOnline | ||
| ? 'bg-[#eff6ff] text-[#2563eb]' | ||
| : 'bg-[#f3f4f6] text-[#9ca3af]' | ||
| }`} | ||
| > | ||
| <span | ||
| className={`h-1.5 w-1.5 rounded-full ${isOnline ? 'bg-[#35c26b]' : 'bg-[#aab1bc]'}`} | ||
| className={`h-1.5 w-1.5 rounded-full ${isOnline ? 'bg-[#409eff]' : 'bg-[#d1d5db]'}`} | ||
| aria-hidden="true" | ||
| /> | ||
| {isOnline | ||
| ? t('workbench.connection_status_online', '在线') | ||
| : t('workbench.connection_status_offline', '离线')} | ||
| {isOnline ? '在线' : '离线'} | ||
| </span> | ||
| ) |
Contributor
There was a problem hiding this comment.
Localize newly added UI copy via useTranslation instead of hardcoded literals.
This file mixes many hardcoded Chinese/English labels/messages, which breaks locale consistency for the settings experience.
As per coding guidelines: Always import translations from '@/hooks/useTranslation', not from 'react-i18next', and use single namespace matching your feature.
Also applies to: 168-173, 219-230, 487-534, 685-692, 775-788, 835-836
🤖 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 `@wework/src/components/settings/ConnectionsSettingsPage.tsx` around lines 76 -
93, Replace hardcoded UI copy in StatusPill with the translation hook: import
useTranslation from '`@/hooks/useTranslation`', call const { t } =
useTranslation('connections') (use a single namespace for this feature, e.g.
'connections'), and replace the literals '在线' and '离线' with t('status.online')
and t('status.offline'); also localize the status dot aria label if present. Do
the same pattern for the other hardcoded labels mentioned (ranges 168-173,
219-230, 487-534, 685-692, 775-788, 835-836): import the same useTranslation
namespace, add appropriate keys (e.g., status.online, status.offline,
connection.title, connection.description, etc.), and replace literal strings
with t(...) calls; ensure you import from '`@/hooks/useTranslation`' (not
'react-i18next') and keep keys consistent within the single namespace.
Comment on lines
+110
to
+119
| <button | ||
| type="button" | ||
| data-testid={testId} | ||
| onClick={onClick} | ||
| disabled={disabled} | ||
| className="inline-flex h-8 items-center gap-1.5 rounded-md border border-[#dedede] bg-white px-2.5 text-xs font-medium text-[#3c4043] hover:bg-[#f7f7f8] disabled:cursor-not-allowed disabled:opacity-50" | ||
| > | ||
| <div className="flex items-start justify-between gap-4"> | ||
| <div className="flex min-w-0 items-start gap-3"> | ||
| <div | ||
| data-testid={`connection-device-icon-${device.id}`} | ||
| className="mt-0.5 flex h-7 w-7 shrink-0 items-center justify-center text-[#3c4043]" | ||
| > | ||
| <Cloud className="h-4 w-4" /> | ||
| </div> | ||
| <div className="min-w-0"> | ||
| <div className="flex min-w-0 items-center gap-2"> | ||
| <h3 className="truncate text-sm font-semibold text-[#2d2d2d]">{device.name}</h3> | ||
| <span className="shrink-0 rounded-full bg-[#f7f7f8] px-2 py-0.5 text-xs text-[#6b6f76]"> | ||
| {device.version || '-'} | ||
| </span> | ||
| </div> | ||
| <div className="mt-1"> | ||
| <StatusPill status={device.status} /> | ||
| </div> | ||
| </div> | ||
| </div> | ||
|
|
||
| <div className="flex shrink-0 gap-2"> | ||
| <DeviceActionButton | ||
| testId={`connection-terminal-button-${device.id}`} | ||
| icon={Terminal} | ||
| label="终端" | ||
| /> | ||
| <DeviceActionButton | ||
| testId={`connection-code-server-button-${device.id}`} | ||
| icon={Code2} | ||
| label="IDE" | ||
| /> | ||
| <DeviceActionButton | ||
| testId={`connection-vnc-button-${device.id}`} | ||
| icon={Monitor} | ||
| label="桌面" | ||
| /> | ||
| </div> | ||
| </div> | ||
|
|
||
| <div className="mt-3 flex flex-wrap items-center gap-x-5 gap-y-2 rounded-md bg-[#fafafa] px-3 py-2"> | ||
| <ResourceMetric | ||
| icon={Cpu} | ||
| label={t('workbench.connection_resource_cpu', 'CPU')} | ||
| value={device.cpuUsage} | ||
| /> | ||
| <ResourceMetric | ||
| icon={MemoryStick} | ||
| label={t('workbench.connection_resource_memory', 'MEM')} | ||
| value={device.memoryUsage} | ||
| /> | ||
| <ResourceMetric | ||
| icon={HardDrive} | ||
| label={t('workbench.connection_resource_disk', '磁盘')} | ||
| value={device.diskUsage} | ||
| /> | ||
| </div> | ||
| </div> | ||
| <Icon className="h-3.5 w-3.5" /> | ||
| <span>{label}</span> | ||
| </button> |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Resize shared action buttons to meet mobile touch-target requirements.
Both reusable button components render below 44×44 (h-8, h-7 w-7), so every consumer inherits non-compliant touch targets.
As per coding guidelines: All interactive elements on mobile must be at least 44px × 44px - use h-11 min-w-[44px] for buttons.
Also applies to: 137-145
🤖 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 `@wework/src/components/settings/ConnectionsSettingsPage.tsx` around lines 110
- 119, The shared action button renders at h-8 (and another at h-7 w-7) which
violates mobile touch-target rules; update the reusable button implementations
(the JSX button that uses props testId, onClick, disabled, Icon, label) to use
className values that enforce at least 44×44 touch targets (replace h-8/h-7 with
h-11 and add min-w-[44px], keeping existing padding/visual styles) and apply the
same change to the other instance referenced around the second button render so
all consumers inherit the compliant size.
Comment on lines
+267
to
+272
| style={{ width: `${value ?? 0}%`, backgroundColor: color }} | ||
| /> | ||
| </div> | ||
| <span className="w-8 shrink-0 text-right font-medium text-[#3c4043]">{value}%</span> | ||
| <span className="w-8 shrink-0 text-right font-medium text-[#3c4043]"> | ||
| {value === null ? '--%' : `${value}%`} | ||
| </span> |
Contributor
There was a problem hiding this comment.
Clamp metric percentage before rendering width/text.
Raw values are used directly for %; values outside 0–100 can overflow or render invalid UI.
📏 Suggested guard
- <div
- className="h-full rounded-full"
- style={{ width: `${value ?? 0}%`, backgroundColor: color }}
- />
+ const safeValue = value === null ? 0 : Math.max(0, Math.min(100, value))
+ <div
+ className="h-full rounded-full"
+ style={{ width: `${safeValue}%`, backgroundColor: color }}
+ />
...
- {value === null ? '--%' : `${value}%`}
+ {value === null ? '--%' : `${safeValue}%`}📝 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
| style={{ width: `${value ?? 0}%`, backgroundColor: color }} | |
| /> | |
| </div> | |
| <span className="w-8 shrink-0 text-right font-medium text-[#3c4043]">{value}%</span> | |
| <span className="w-8 shrink-0 text-right font-medium text-[#3c4043]"> | |
| {value === null ? '--%' : `${value}%`} | |
| </span> | |
| const safeValue = value === null ? 0 : Math.max(0, Math.min(100, value)) | |
| <div | |
| className="h-full rounded-full" | |
| style={{ width: `${safeValue}%`, backgroundColor: color }} | |
| /> | |
| </div> | |
| <span className="w-8 shrink-0 text-right font-medium text-[`#3c4043`]"> | |
| {value === null ? '--%' : `${safeValue}%`} | |
| </span> |
🤖 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 `@wework/src/components/settings/ConnectionsSettingsPage.tsx` around lines 267
- 272, Clamp the metric percentage before using it in the bar width and display:
ensure the `value` used in the inline style (`style={{ width: `${value ?? 0}%`
}}`) and the text span (`{value === null ? '--%' : `${value}%`}`) is clamped to
the 0–100 range (e.g., compute a local `clamped = Math.max(0, Math.min(100,
value ?? 0))` or a small helper `clampPercent(value)`) and use `clamped` for
both the width and the displayed `${clamped}%` (while still rendering '--%' when
original `value` is null) so out-of-range values cannot overflow the UI; update
references in ConnectionsSettingsPage component accordingly.
Comment on lines
+461
to
+464
| <h3 | ||
| className="cursor-pointer truncate text-sm font-semibold text-[#2d2d2d]" | ||
| onClick={handleStartEdit} | ||
| title="点击修改名称" |
Contributor
There was a problem hiding this comment.
Replace clickable heading with a keyboard-accessible control.
Using <h3 onClick> makes rename action mouse-only for many users; use a <button> (or add proper keyboard semantics) for accessibility.
♿ Minimal accessible pattern
- <h3
- className="cursor-pointer truncate text-sm font-semibold text-[`#2d2d2d`]"
- onClick={handleStartEdit}
- title="点击修改名称"
- >
- {device.name}
- </h3>
+ <button
+ type="button"
+ onClick={handleStartEdit}
+ className="truncate text-left text-sm font-semibold text-[`#2d2d2d`]"
+ title="点击修改名称"
+ >
+ {device.name}
+ </button>🤖 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 `@wework/src/components/settings/ConnectionsSettingsPage.tsx` around lines 461
- 464, The heading <h3> using onClick (the element with className
"cursor-pointer truncate text-sm font-semibold text-[`#2d2d2d`]" and
onClick={handleStartEdit}) is mouse-only; replace it with a semantic interactive
element (e.g., a <button>) or add full keyboard semantics. Move the
onClick={handleStartEdit}, title, and className to the new button, preserve
visual styles via the same classes (or convert any heading-specific styles), and
ensure the button has an accessible label (use its text content or aria-label)
so keyboard users can activate the rename action; if you choose to keep a
non-button element, add role="button", tabIndex={0}, and keydown handlers that
call handleStartEdit on Enter/Space.
This was referenced May 29, 2026
This was referenced Jun 17, 2026
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
2 participants
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 by CodeRabbit
New Features
Documentation
Tests