Repository navigation
feat(wework): add connection settings page - #1174
Merged
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR adds a connections settings page to the desktop workbench layout. Users can access it via a new settings button in the sidebar menu, which toggles a local UI state to display either the main workbench or the connections management interface. The new ConnectionsSettingsPage displays authorized devices with status, resource metrics, and per-device actions in a two-column layout. Localized UI strings and an integration test are included. ChangesConnections Settings Page Feature
🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 2
🧹 Nitpick comments (1)
wework/src/components/layout/DesktopWorkbenchLayout.test.tsx (1)
100-164: ⚡ Quick winSplit this test into focused cases to keep it maintainable.
Line 100 introduces a single 65-line test with many unrelated assertions; please split it into smaller test cases (or extract assertion helpers) so failures are easier to localize.
♻️ Proposed refactor
- test('opens the independent connection settings page from the settings menu', async () => { + async function openConnectionSettings() { + render(<DesktopWorkbenchLayout {...baseProps} />) + await userEvent.click(screen.getByTestId('settings-button')) + await userEvent.click(screen.getByTestId('settings-menu-button')) + } + + test('navigates to connection settings page', async () => { + await openConnectionSettings() + expect(screen.getByTestId('wework-settings-page')).toBeInTheDocument() + expect(screen.getByRole('heading', { name: '连接' })).toBeInTheDocument() + }) + + test('renders cloud device actions and metrics', async () => { + await openConnectionSettings() + expect(screen.getByText('yunpeng7-executor-372706c30fcd')).toBeInTheDocument() + expect(screen.getByText('CPU')).toBeInTheDocument() + expect(screen.getByText('MEM')).toBeInTheDocument() + expect(screen.getByText('磁盘')).toBeInTheDocument() + }) + + test('does not render deprecated/incorrect labels', async () => { + await openConnectionSettings() + expect(screen.queryByText('Start Task')).not.toBeInTheDocument() + expect(screen.queryByText('Online')).not.toBeInTheDocument() + }) - ... - })As per coding guidelines,
**/*.{py,ts,tsx}: Maximum 50 lines per function (preferred) in Python and TypeScript code.🤖 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/layout/DesktopWorkbenchLayout.test.tsx` around lines 100 - 164, The single large test "opens the independent connection settings page from the settings menu" is doing too many unrelated assertions; split it into focused tests (or extract helpers) to keep each test under ~50 lines. Create a small helper like renderAndOpenSettings (called from multiple tests) that renders <DesktopWorkbenchLayout {...baseProps} /> and clicks the 'settings-button' and 'settings-menu-button', then replace the giant test with multiple tests such as: a navigation/assertions test (check wework-settings-page, settings-back-button text, settings-nav-connections/projects presence and absence of settings-nav-general), a headings/content test (heading '连接', '连接设备', '项目', '可连接的设备', '云设备', '说明' and absence of English labels), a device-list/test-icons test (assert connection-device-icon-24a59054... classes, device name yunpeng7-executor..., version v1.712, status 在线, absence of Personal Devices/Linux-Device...), an actions/buttons test (presence of connection-terminal-button-24a59054..., connection-code-server-button..., connection-vnc-button..., and labels 终端/IDE/桌面), and a metrics/test-percentages test (CPU/MEM/磁盘 and 42%/68%/57% and connection-scale-wiki). Keep each test focused, reference the test ids used above when moving assertions so failures are localized.
🤖 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 `@wework/src/components/settings/ConnectionsSettingsPage.tsx`:
- Around line 159-163: In ConnectionsSettingsPage, several button elements
(e.g., the button using data-testid={testId} and other interactive elements in
the same component) use small height classes like h-8, h-9, h-10 and
min-h-[31px]; update each button's Tailwind classes to meet mobile accessibility
(at least 44×44) by replacing the height/size classes with h-11 and adding
min-w-[44px] (and remove or adjust conflicting min-h classes) for the buttons
referenced around the noted blocks (including the element with
data-testid={testId} and the buttons at the other ranges called out). Ensure
visual style (padding, gap) remains consistent after the change.
- Line 16: Buttons and nav items in ConnectionsSettingsPage (DeviceActionButton,
settings-back-button, settings-nav-*, connection-add-device-button and the
connections tab button) use heights like h-8/h-9/h-10 or min-h-[31px] which are
below the mobile touch target minimum (44×44); update their classes to ensure at
least h-11 and min-w-[44px] (or equivalent padding) so touch targets meet the
standard, and audit any custom components used by the page to propagate the size
change. Also fix the import: replace the incorrect import of useTranslation from
'react-i18next' with the project hook import '`@/hooks/useTranslation`' (or add a
wrapper hook at that path) so ConnectionsSettingsPage uses the local
useTranslation implementation used across the wework package. Ensure you update
all occurrences in this file that reference the old import and run style/type
checks.
---
Nitpick comments:
In `@wework/src/components/layout/DesktopWorkbenchLayout.test.tsx`:
- Around line 100-164: The single large test "opens the independent connection
settings page from the settings menu" is doing too many unrelated assertions;
split it into focused tests (or extract helpers) to keep each test under ~50
lines. Create a small helper like renderAndOpenSettings (called from multiple
tests) that renders <DesktopWorkbenchLayout {...baseProps} /> and clicks the
'settings-button' and 'settings-menu-button', then replace the giant test with
multiple tests such as: a navigation/assertions test (check
wework-settings-page, settings-back-button text,
settings-nav-connections/projects presence and absence of settings-nav-general),
a headings/content test (heading '连接', '连接设备', '项目', '可连接的设备', '云设备', '说明' and
absence of English labels), a device-list/test-icons test (assert
connection-device-icon-24a59054... classes, device name yunpeng7-executor...,
version v1.712, status 在线, absence of Personal Devices/Linux-Device...), an
actions/buttons test (presence of connection-terminal-button-24a59054...,
connection-code-server-button..., connection-vnc-button..., and labels
终端/IDE/桌面), and a metrics/test-percentages test (CPU/MEM/磁盘 and 42%/68%/57% and
connection-scale-wiki). Keep each test focused, reference the test ids used
above when moving assertions so failures are localized.
🪄 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: d057d52b-5b13-4bfa-92d0-b47f10e80338
📒 Files selected for processing (7)
wework/src/components/layout/DesktopSettingsMenu.tsxwework/src/components/layout/DesktopSidebar.tsxwework/src/components/layout/DesktopWorkbenchLayout.test.tsxwework/src/components/layout/DesktopWorkbenchLayout.tsxwework/src/components/settings/ConnectionsSettingsPage.tsxwework/src/i18n/locales/en/common.jsonwework/src/i18n/locales/zh-CN/common.json
| Terminal, | ||
| } from 'lucide-react' | ||
| import type { ComponentType } from 'react' | ||
| import { useTranslation } from 'react-i18next' |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify canonical translation hook usage patterns and current violation.
rg -n --type=ts --type=tsx "from '`@/hooks/useTranslation`'" wework/src
rg -n --type=ts --type=tsx "from 'react-i18next'" wework/src/components/settings/ConnectionsSettingsPage.tsx
fd -i "useTranslation*.ts" wework/src/hooksRepository: wecode-ai/Wegent
Length of output: 116
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="wework/src/components/settings/ConnectionsSettingsPage.tsx"
test -f "$FILE" && echo "Found: $FILE" || { echo "Missing: $FILE"; exit 1; }
echo "---- useTranslation import check ----"
rg -n "from 'react-i18next'|from \"react-i18next\"|useTranslation" "$FILE" || true
echo "---- repo-wide guideline check (examples) ----"
rg -n "from 'react-i18next'" wework/src --glob='*.{ts,tsx,js,jsx}' || true
rg -n "from '`@/hooks/useTranslation`'" wework/src --glob='*.{ts,tsx,js,jsx}' || true
echo "---- quick scan for small button heights in this file ----"
# capture common Tailwind height classes used for buttons/interactive UI
rg -n "h-(8|9|10)\b|min-h-\[(3[0-9]|[12][0-9]|[0-9])|min-h-\[3[0-9]\]|min-h-\[2[0-9]\]" "$FILE" || true
echo "---- show relevant sections around earlier line numbers ----"
# Use sed to show around the previously mentioned areas without dumping whole file
sed -n '1,120p' "$FILE" | nl -ba | sed -n '1,120p' || true
sed -n '140,220p' "$FILE" | nl -ba | sed -n '1,120p' || true
sed -n '220,320p' "$FILE" | nl -ba | sed -n '1,120p' || true
sed -n '320,380p' "$FILE" | nl -ba | sed -n '1,80p' || trueRepository: wecode-ai/Wegent
Length of output: 3589
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="wework/src/components/settings/ConnectionsSettingsPage.tsx"
echo "---- file header (imports) ----"
sed -n '1,60p' "$FILE" | cat -n
echo "---- check translation hook usage pattern (local) ----"
rg -n "useTranslation\(" "$FILE"
rg -n "useUnifiedMessages|useUnifiedMessages\(" "$FILE" || true
echo "---- confirm whether '`@/hooks/useTranslation`' exists and is used repo-wide ----"
fd -i "useTranslation*.ts*" wework/src/hooks || true
rg -n "from '`@/hooks/useTranslation`'" wework/src --glob='*.{ts,tsx,js,jsx}' || true
echo "---- show relevant sections around earlier reported lines ----"
for start in 140 230 250 270 295 300; do
end=$((start+35))
echo "### lines $start-$end"
sed -n "${start},${end}p" "$FILE" | cat -n
done
echo "---- enumerate interactive elements near those small heights ----"
# Print lines mentioning height/min-height plus nearby tags indicating interactivity.
rg -n "h-(8|9|10)\b|min-h-\[[0-9]+\]|min-h-\[(3[0-9]|[12][0-9]|[0-9])\]" "$FILE" || trueRepository: wecode-ai/Wegent
Length of output: 11014
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- locate '`@/hooks/useTranslation`' implementation ----"
rg -n "hooks/useTranslation" wework/src --glob='*.{ts,tsx,js,jsx}' || true
fd -i "useTranslation" wework/src/hooks || true
echo "---- confirm useUnifiedMessages usage in this file ----"
rg -n "useUnifiedMessages" wework/src/components/settings/ConnectionsSettingsPage.tsx || trueRepository: wecode-ai/Wegent
Length of output: 173
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- find any file that looks like a useTranslation hook ----"
fd -i "useTranslation" . || true
fd -i "*useTranslation*" . || true
echo "---- search for '`@/hooks/useTranslation`' usage across repo ----"
rg -n "from '`@/hooks/useTranslation`'" . --glob='*.{ts,tsx,js,jsx}' || true
echo "---- search for 'useUnifiedMessages' ----"
rg -n "useUnifiedMessages" . --glob='*.{ts,tsx,js,jsx}' || true
echo "---- search for 'hooks/useTranslation' path ----"
rg -n "hooks/useTranslation" . --glob='*.{ts,tsx,js,jsx}' || true
echo "---- check if any '`@/hooks/`' translation file exists ----"
fd -i "translation" wework/src/hooks . || trueRepository: wecode-ai/Wegent
Length of output: 50687
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "from '`@/hooks/useTranslation`'" wework/src --glob='*.{ts,tsx,js,jsx}' || true
rg -n "from '`@/hooks/useTranslation`'" . --glob='wework/src/**/*.{ts,tsx,js,jsx}' || true
ls -la wework/src/hooks || true
fd -i "useTranslation*.ts" wework/src || trueRepository: wecode-ai/Wegent
Length of output: 300
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- repo top-level dirs ----"
ls -ლა
echo "---- find wework tsconfig(s) ----"
fd -i "tsconfig*.json" wework -a 2>/dev/null || true
fd -i "tsconfig*.json" . -a 2>/dev/null | rg "wework" || true
echo "---- show wework tsconfig paths for '@' alias ----"
for f in $(fd -i "tsconfig*.json" wework --maxdepth 4 2>/dev/null); do
echo "### $f"
rg -n "\"paths\"|\"baseUrl\"|\"`@/hooks/useTranslation`\"|\"@\"" "$f" || true
done
echo "---- find any usage of '`@/hooks/useTranslation`' inside wework ----"
rg -n "from '`@/hooks/useTranslation`'" wework --glob='*.{ts,tsx,js,jsx}' || true
echo "---- find any local translation hook file under wework ----"
fd -i "useTranslation*.ts*" wework --type f 2>/dev/null || trueRepository: wecode-ai/Wegent
Length of output: 172
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- repo top-level dirs ----"
ls -la
echo "---- find wework tsconfig(s) ----"
fd -i "tsconfig*.json" wework 2>/dev/null || true
echo "---- show wework tsconfig paths for '@' alias ----"
for f in $(fd -i "tsconfig*.json" wework 2>/dev/null); do
echo "### $f"
rg -n "\"paths\"|\"baseUrl\"|\"`@/hooks/useTranslation`\"|\"@\"" "$f" || true
done
echo "---- find any usage of '`@/hooks/useTranslation`' inside wework ----"
rg -n "from '`@/hooks/useTranslation`'" wework --glob='*.{ts,tsx,js,jsx}' || true
echo "---- find any local translation hook file under wework ----"
fd -i "useTranslation*.ts*" wework 2>/dev/null || true
echo "---- find any useTranslation implementation under wework ----"
rg -n "function useTranslation|useTranslation\\s*\\(" wework --glob='*.{ts,tsx,js,jsx}' || trueRepository: wecode-ai/Wegent
Length of output: 5468
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cat -n wework/tsconfig.json | sed -n '1,120p'Repository: wecode-ai/Wegent
Length of output: 364
Fix mobile touch targets for buttons (min 44×44).
wework/src/components/settings/ConnectionsSettingsPage.tsx has interactive buttons styled with h-8/h-9/h-10 and a nav item with min-h-[31px] (e.g., DeviceActionButton, settings-back-button, settings-nav-*, the connections tab button, and connection-add-device-button), which falls below the h-11 min-w-[44px] requirement.
Also, this file imports useTranslation from react-i18next; in the wework package, @/* maps to wework/src/* and there is no wework/src/hooks/useTranslation implementation (only useIsDesktop.ts / useIsMobile.ts), so switching to @/hooks/useTranslation requires adding/using that hook in wework.
🤖 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` at line 16,
Buttons and nav items in ConnectionsSettingsPage (DeviceActionButton,
settings-back-button, settings-nav-*, connection-add-device-button and the
connections tab button) use heights like h-8/h-9/h-10 or min-h-[31px] which are
below the mobile touch target minimum (44×44); update their classes to ensure at
least h-11 and min-w-[44px] (or equivalent padding) so touch targets meet the
standard, and audit any custom components used by the page to propagate the size
change. Also fix the import: replace the incorrect import of useTranslation from
'react-i18next' with the project hook import '`@/hooks/useTranslation`' (or add a
wrapper hook at that path) so ConnectionsSettingsPage uses the local
useTranslation implementation used across the wework package. Ensure you update
all occurrences in this file that reference the old import and run style/type
checks.
Comment on lines
+159
to
+163
| <button | ||
| type="button" | ||
| data-testid={testId} | ||
| 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]" | ||
| > |
Contributor
There was a problem hiding this comment.
Increase button touch targets to 44×44 for mobile accessibility.
Several interactive buttons are below the required size (h-8, h-9, h-10, and min-h-[31px]). Update these to meet the minimum mobile target size.
Suggested class updates
- 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`]"
+ className="inline-flex h-11 min-w-[44px] items-center gap-1.5 rounded-md border border-[`#dedede`] bg-white px-2.5 text-xs font-medium text-[`#3c4043`] hover:bg-[`#f7f7f8`]"
- className="mb-4 flex h-9 items-center gap-2 rounded-md px-2 text-sm text-[`#777`] hover:bg-black/5"
+ className="mb-4 flex h-11 min-w-[44px] items-center gap-2 rounded-md px-2 text-sm text-[`#777`] hover:bg-black/5"
- className="flex min-h-[31px] w-full items-center gap-2 rounded-lg px-2.5 text-left text-sm font-medium"
+ className="flex h-11 min-w-[44px] w-full items-center gap-2 rounded-lg px-2.5 text-left text-sm font-medium"
- className="h-10 px-6 text-sm font-medium"
+ className="h-11 min-w-[44px] px-6 text-sm font-medium"
- className="inline-flex h-8 items-center gap-1.5 rounded-full bg-[`#f5f5f5`] px-3 text-sm text-[`#2d2d2d`] hover:bg-[`#ececec`]"
+ className="inline-flex h-11 min-w-[44px] items-center gap-1.5 rounded-full bg-[`#f5f5f5`] px-3 text-sm text-[`#2d2d2d`] hover:bg-[`#ececec`]"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: 242-247, 254-261, 282-291, 304-307
🤖 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 159
- 163, In ConnectionsSettingsPage, several button elements (e.g., the button
using data-testid={testId} and other interactive elements in the same component)
use small height classes like h-8, h-9, h-10 and min-h-[31px]; update each
button's Tailwind classes to meet mobile accessibility (at least 44×44) by
replacing the height/size classes with h-11 and adding min-w-[44px] (and remove
or adjust conflicting min-h classes) for the buttons referenced around the noted
blocks (including the element with data-testid={testId} and the buttons at the
other ranges called out). Ensure visual style (padding, gap) remains consistent
after the change.
This was referenced May 28, 2026
This was referenced Jun 4, 2026
Merged
This was referenced Jun 16, 2026
This was referenced Jul 1, 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
Release Notes
New Features
Tests
Localization