Repository navigation
fix(social): UI polish round 3 — fewer duplicate entries, tighter copy, Escape closes panels (#173) - #176
Conversation
Sidebar and + menu consistency, sharing settings layout, English Chief greeting default, tab title sync, and friend-chat empty hint. Regenerate ds-bot patches from core. Co-authored-by: Danyang Chen <birfychen@gmail.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 14006ba7b7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const sync = () => { | ||
| if (typeof document === 'undefined') return | ||
| const row = Object.values(sessions.getSnapshot().byId ?? {}).find(session => (session?.retainedBy?.mainView ?? 0) > 0) | ||
| const label = sessionTabLabel(roster.getSnapshot(), row?.id) |
There was a problem hiding this comment.
Declare the roster before registering the tab-title effect
Under the synchronous ctx.effect contract used by the client plugin, this effect immediately calls sync(), which reads roster even though that const is not initialized until line 66. Normal FlyBot client startup therefore throws a TDZ ReferenceError here and aborts the remainder of apply(); create the roster before registering this effect.
Useful? React with 👍 / 👎.
| const grants = view?.grants ?? [] | ||
| return h('div', { className: 'fs fs-card', 'data-flybot': 'manage-access' }, | ||
| h('div', { className: 'fs-name' }, 'Manage access'), | ||
| h('div', { className: 'fs-name' }, 'People with access'), |
There was a problem hiding this comment.
Update the owned subsystem documentation
Changing this label (and removing the Accept invite action) leaves docs/subsystems/social.md describing the old Manage access label and the removed action, while this commit also changes owned core-client/core-host paths without updating their subsystem docs or carrying a Docs-not-needed: trailer. ARCH_DOCS_BASE=84b8ccd^ node core/scripts/check-arch-docs.mjs consequently fails for all three subsystems, so update the affected docs or add a justified trailer before landing.
AGENTS.md reference: AGENTS.md:L94-L94
Useful? React with 👍 / 👎.
| if (document.title !== next) document.title = next | ||
| } | ||
| sync() | ||
| return sessions.subscribe(sync) |
There was a problem hiding this comment.
Subscribe the tab title to roster changes
The tab label is derived from both the session list and roster, but this cleanup subscribes only to sessions. When pull() refreshes an active remote Bot's name/owner or shared-room display metadata without a navigation/session-list event, the document title keeps the old label until some later session change; subscribe sync to both sources and dispose both subscriptions.
Useful? React with 👍 / 👎.
| const onKey = (event) => { | ||
| if (event.key === 'Escape') { event.preventDefault(); onClose() } |
There was a problem hiding this comment.
Close only the topmost Escape layer
When both social panels are open (FriendsBots can render adding and faceOpen simultaneously), or a core overlay is above this panel, this document-level handler runs alongside the other panel/global Escape handler. Because it only prevents the default and does not coordinate with the existing topmost-layer stack, one Escape invokes multiple close callbacks and dismisses multiple layers; make both social panels participate in a single topmost Escape mechanism.
Useful? React with 👍 / 👎.
Refs #173
Summary
Past roomsheader in sentence case.Add friendentry (removed duplicateAccept invite, same panel);Add Botinstead ofAdd Agent.000000styled as placeholder.Sharingsection,People with access, shorter invite/access copy, duplicate/delete card titledMore.Skipped (follow-up in #173): popover
New…menu, right-click menu trim, tab order / focus ring. Face/past-room items belong to #170–#172.Fork PR: Birfy#22
Test plan
ARCH_DOCS_BASE=upstream/main npm run verify(agent run, Node 22)