Skip to content

Commit d56c0a2

Browse files
committed
fix(settings): address review — remove aside footgun, sticky tabs, restore tooltips
- Remove the SettingsHeaderConfig 'aside' escape hatch (the last stale-prone, div-admitting path). Add SettingsAction.onPrefetch so the teammates Invite chip (hover-prefetch) is pure data — nothing renders header JSX now, so the signature-vs-ref staleness cursor flagged is gone. - useIsomorphicLayoutEffect for header registration: section switches flush the new header before paint (no stale/blank header frame). - group-detail: pin the config tabs (sticky top-0) so they stay visible while the detail body scrolls. - Restore the team-management Invite disabled-reason tooltip via the new SettingsAction.tooltip field.
1 parent 8bb7f54 commit d56c0a2

5 files changed

Lines changed: 41 additions & 31 deletions

File tree

apps/sim/app/workspace/[workspaceId]/settings/components/settings-header/settings-header.tsx

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,16 @@ import {
88
useCallback,
99
useContext,
1010
useEffect,
11+
useLayoutEffect,
1112
useMemo,
1213
useRef,
1314
useState,
1415
} from 'react'
1516
import { Chip, ChipInput, ChipLink, Search, Tooltip } from '@sim/emcn'
1617

18+
/** `useLayoutEffect` on the client (flush header changes before paint), `useEffect` during SSR. */
19+
const useIsomorphicLayoutEffect = typeof window === 'undefined' ? useEffect : useLayoutEffect
20+
1721
/** The strict contract for a settings header action — rendered as a {@link Chip}, data only. */
1822
export interface SettingsAction {
1923
text: string
@@ -24,6 +28,8 @@ export interface SettingsAction {
2428
disabled?: boolean
2529
/** Hover/focus tooltip (e.g. why the action is disabled) — the shell renders it; no per-page JSX. */
2630
tooltip?: string
31+
/** Warm a lazy resource on hover/focus (e.g. prefetch the upgrade flow). */
32+
onPrefetch?: () => void
2733
}
2834

2935
export interface SettingsHeaderSearch {
@@ -49,8 +55,6 @@ export interface SettingsHeaderConfig {
4955
search?: SettingsHeaderSearch
5056
/** Forwarded to the scroll region (e.g. for programmatic scroll-to-bottom). */
5157
scrollContainerRef?: Ref<HTMLDivElement>
52-
/** Escape hatch for a right-aligned widget that genuinely cannot be a chip. */
53-
aside?: ReactNode
5458
}
5559

5660
const EMPTY_CONFIG: SettingsHeaderConfig = {}
@@ -82,9 +86,9 @@ function computeSignature(c: SettingsHeaderConfig): string {
8286
x.disabled ?? false,
8387
x.icon ? 1 : 0,
8488
x.tooltip ?? '',
89+
x.onPrefetch ? 1 : 0,
8590
]),
8691
s: c.search ? [c.search.value, c.search.placeholder ?? '', c.search.disabled ?? false] : null,
87-
aside: c.aside ? 1 : 0,
8892
})
8993
}
9094

@@ -111,25 +115,25 @@ export function SettingsHeaderProvider({ children }: { children: ReactNode }) {
111115
export function useSettingsHeader(config: SettingsHeaderConfig) {
112116
const register = useContext(RegisterContext)
113117

114-
useEffect(() => {
118+
useIsomorphicLayoutEffect(() => {
115119
register?.(config)
116120
})
117121

118-
useEffect(() => {
122+
useIsomorphicLayoutEffect(() => {
119123
return () => register?.(EMPTY_CONFIG)
120124
}, [register])
121125
}
122126

123127
/**
124128
* The single owner of settings page chrome: the header bar (back chip, Docs link,
125-
* action chips, `aside`), the scroll region, and the centered column led by the
126-
* title + description, then search and `{children}`.
129+
* action chips), the scroll region, and the centered column led by the title +
130+
* description, then search and `{children}`.
127131
*/
128132
export function SettingsHeaderShell({ children }: { children: ReactNode }) {
129133
const read = useContext(ReadContext)
130134
const configRef = read?.configRef
131135
const config = configRef?.current ?? EMPTY_CONFIG
132-
const { title, description, docsLink, back, actions, search, aside, scrollContainerRef } = config
136+
const { title, description, docsLink, back, actions, search, scrollContainerRef } = config
133137

134138
return (
135139
<div className='flex h-full flex-col bg-[var(--bg)]'>
@@ -147,7 +151,6 @@ export function SettingsHeaderShell({ children }: { children: ReactNode }) {
147151
Docs
148152
</ChipLink>
149153
)}
150-
{aside}
151154
{actions?.map((action, index) => {
152155
const chip = (
153156
<Chip
@@ -156,6 +159,16 @@ export function SettingsHeaderShell({ children }: { children: ReactNode }) {
156159
active={action.active}
157160
leftIcon={action.icon}
158161
onClick={() => configRef?.current.actions?.[index]?.onSelect()}
162+
onMouseEnter={
163+
action.onPrefetch
164+
? () => configRef?.current.actions?.[index]?.onPrefetch?.()
165+
: undefined
166+
}
167+
onFocus={
168+
action.onPrefetch
169+
? () => configRef?.current.actions?.[index]?.onPrefetch?.()
170+
: undefined
171+
}
159172
disabled={action.disabled}
160173
>
161174
{action.text}

apps/sim/app/workspace/[workspaceId]/settings/components/settings-panel/settings-panel.tsx

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,6 @@ interface SettingsPanelProps {
5454
description?: string
5555
/** Overrides the nav-driven docs link (the "Docs" link rendered in the header bar). */
5656
docsLink?: string
57-
/** Escape hatch for a right-aligned widget that genuinely cannot be a chip. Rare. */
58-
aside?: ReactNode
5957
/** Forwarded to the scroll region (e.g. for programmatic scroll-to-bottom). */
6058
scrollContainerRef?: Ref<HTMLDivElement>
6159
}
@@ -76,7 +74,6 @@ export function SettingsPanel({
7674
title,
7775
description,
7876
docsLink,
79-
aside,
8077
scrollContainerRef,
8178
}: SettingsPanelProps) {
8279
const section = useSettingsSection()
@@ -89,7 +86,6 @@ export function SettingsPanel({
8986
back,
9087
actions,
9188
search,
92-
aside,
9389
scrollContainerRef,
9490
})
9591

apps/sim/app/workspace/[workspaceId]/settings/components/team-management/team-management.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -320,6 +320,7 @@ export function TeamManagement() {
320320
variant: 'primary',
321321
onSelect: () => setInviteModalOpen(true),
322322
disabled: isInvitationsDisabled,
323+
tooltip: isInvitationsDisabled ? 'Invitations are disabled' : undefined,
323324
},
324325
]}
325326
>

apps/sim/app/workspace/[workspaceId]/settings/components/teammates/teammates.tsx

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
'use client'
22

33
import { useCallback, useMemo, useState } from 'react'
4-
import { Chip, ChipDropdown, Plus, toast } from '@sim/emcn'
4+
import { ChipDropdown, Plus, toast } from '@sim/emcn'
55
import { getErrorMessage } from '@sim/utils/errors'
66
import { useQueryClient } from '@tanstack/react-query'
77
import { useParams, useRouter } from 'next/navigation'
@@ -170,18 +170,16 @@ export function Teammates() {
170170
onChange: setSearchTerm,
171171
placeholder: 'Search teammates...',
172172
}}
173-
aside={
174-
<Chip
175-
leftIcon={Plus}
176-
variant='primary'
177-
onClick={handleInvite}
178-
onMouseEnter={isInvitationsDisabled ? prefetchUpgrade : undefined}
179-
onFocus={isInvitationsDisabled ? prefetchUpgrade : undefined}
180-
title={inviteDisabledReason ?? undefined}
181-
>
182-
Invite
183-
</Chip>
184-
}
173+
actions={[
174+
{
175+
text: 'Invite',
176+
icon: Plus,
177+
variant: 'primary',
178+
onSelect: handleInvite,
179+
tooltip: inviteDisabledReason ?? undefined,
180+
onPrefetch: isInvitationsDisabled ? prefetchUpgrade : undefined,
181+
},
182+
]}
185183
>
186184
<MemberSection
187185
label={`Teammates (${teammates.length})`}

apps/sim/ee/access-control/components/group-detail.tsx

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1274,11 +1274,13 @@ export function GroupDetail({
12741274
},
12751275
]}
12761276
>
1277-
<ChipModalTabs
1278-
tabs={tabs}
1279-
value={configTab}
1280-
onChange={(value) => setConfigTab(value as ConfigTab)}
1281-
/>
1277+
<div className='sticky top-0 z-10 bg-[var(--bg)]'>
1278+
<ChipModalTabs
1279+
tabs={tabs}
1280+
value={configTab}
1281+
onChange={(value) => setConfigTab(value as ConfigTab)}
1282+
/>
1283+
</div>
12821284

12831285
{configTab === 'general' && (
12841286
<>

0 commit comments

Comments
 (0)