refactor: make status indicator presentation reusable

[a-effort]

Moves the presentational part of ServerStatusIndicator into ui/status-indicator, so other surfaces can reuse the icon, label and popover without the server availability model. ServerStatusIndicator keeps the classification and the authorize branch, and composes the new component.

Behaviour is unchanged. ServerStatusIndicator.test.tsx is the same as on main and still passes.

PopoverContent also gets an aria-label. Radix gives it role="dialog" with no name, so it announced as an unnamed dialog. The trigger string will not work here, since it ends in "Show details" and would tell you to open something already open.

Sequence note: merge after #150, which rewrites the same return statement. Then, rebase this onto #150, then #153 onto this.

[Copilot]

Copilot review overview

🟡 Changes recommended

The reusable component can still render an unnamed dialog when its optional content label is omitted.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1

Open (1)

• Provide fallback accessible label when contentAriaLabel is omitted · New

What changed in this PR

Extracts a reusable status-indicator presentation component while retaining server-specific availability and authorization logic.

Changes:

• Adds the reusable StatusIndicator component and tests.
• Refactors ServerStatusIndicator to compose it.
• Adds localized popover accessible names.

File	Description
src/​components/​ui/​status-indicator.tsx	Adds the reusable status UI shell.
src/​components/​ui/​status-indicator.test.tsx	Tests rendering, popovers, and accessibility.
src/​components/​servers/​ServerStatusIndicator.tsx	Uses the extracted component.
src/​i18n/​locales/​en-US/​mcpServer.json	Adds the English dialog label.
src/​i18n/​locales/​es-ES/​mcpServer.json	Adds the Spanish dialog label.
src/​i18n/​locales/​pt-BR/​mcpServer.json	Adds the Portuguese dialog label.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[marekdano]

Findings

🟠 Medium

1. isAbbreviated truthy check drops falsy fullLabel

File: src/components/ui/status-indicator.tsx:58

isAbbreviated uses Boolean(fullLabel && fullLabel !== label) — a truthy check instead of an "is defined" check. If fullLabel is ever "" while label is non-empty (e.g. a blank/missing translation), it's treated as "not abbreviated": the sr-only full-label span is skipped, and the aria-label fallback chain (contentAriaLabel ?? fullLabel ?? label) skips the falsy empty string too — silently losing the intended accessible name.

2. Status icon JSX duplicated across two files

File: src/components/servers/ServerStatusIndicator.tsx:66 (vs. status-indicator.tsx:63-67)

The status icon JSX is now duplicated (once in the authorize-button branch, once in StatusIndicator's content builder), where before it was one shared icon const. Any future icon/a11y tweak has to be made in both places, or the authorize button's icon silently desyncs from every other state. The exported STATUS_INDICATOR_TRIGGER_CLASS constant looks like the escape-hatch symptom of the abstraction boundary being drawn in the wrong place.

3. triggerAriaLabel optional despite being "required"

File: src/components/ui/status-indicator.tsx:16

triggerAriaLabel is typed optional even though its doc comment says it's "Required wherever children are given," with no runtime enforcement. A future caller with children but no triggerAriaLabel silently loses the guaranteed descriptive name.

🟡 Low

4. Dead size prop reinvents existing pattern

File: src/components/ui/status-indicator.tsx:19, 27-30

The new size?: "xs" | "sm" prop and its hand-rolled SIZE_CLASS map are dead code — no caller anywhere passes size — and reinvent a pattern the codebase already has via class-variance-authority (used for size variants in button.tsx and badge.tsx).

5. Docstring claims consumers that don't exist yet

File: src/components/ui/status-indicator.tsx:43

The docstring claims the catalog card and source picker are consumers sharing this shape, but this PR only migrates ServerStatusIndicator. CatalogResults.tsx and SourceSelection.tsx still hand-roll their own icon+label markup — the docstring is currently misleading until those two are migrated.

6. Unconditional formatMessage calls when non-interactive

File: src/components/servers/ServerStatusIndicator.tsx:110

triggerAriaLabel/contentAriaLabel are computed via intl.formatMessage unconditionally, even when interactive={false} (where StatusIndicator discards both and renders a plain span). The old code early-returned before this formatting.

7. Ambiguous aria-label fallback for popover content

File: src/components/ui/status-indicator.tsx:90

When a caller omits contentAriaLabel, the popover's aria-label falls back to the bare status word (e.g. "Available") with no entity identifier — multiple indicators for different entities would get identical accessible names. Better than pre-PR (no aria-label at all), but ambiguous.

[vishu-bh]

implementation looks clean and focused. A minor edge case to check

[vishu-bh]

children is typed as ReactNode, so 0 is valid renderable content. This truthiness check treats 0 as absent and silently disables the popover. Could we use children == null instead and add a children={0} regression test?

[a-effort]

Thanks for reviewing! Changes added in 82ac6a2 & f025d8c. Details:

@marekdano's review:

From LLM-assisted analysis of the feedback, the recommendation is to move forward with these changes:

Feedback item 2: implemented.

The status icon is now a StatusIndicatorIcon export used by both the component and the authorize branch, so the copies cannot drift. The authorize branch needs an onClick, a disabled state and the stacked two-label width trick, none of which StatusIndicator can render, so STATUS_INDICATOR_TRIGGER_CLASS stays as the shared piece.

Feedback item 4: implemented.

Removed the size prop and SIZE_CLASS. Nothing passes it, as you say. It was headroom for the catalog footer size mismatch queued behind #6972, but that can be added when needed as a follow up.

Feedback item 5: implemented.

Reworded the docstring to say what the component is for rather than naming consumers. The catalog card only becomes one in #153, and the source picker is not one on either branch.

Feedback item 1: partly implemented.

The aria-label chain used ??, so an empty contentAriaLabel won over the label and left the dialog unnamed, which is what the fallback was added to prevent. It is || now, with a test for the blank case. Left isAbbreviated as a truthy check: an empty fullLabel should not aria-hide the visible label and render an empty sr-only span, so truthiness is the right test there.

From LLM-assisted analysis of the feedback, the recommendation is to not move forward with these changes:

Feedback item 3: not implemented.

The type matches the behaviour, the comment did not. A button is named by its content, so the trigger has a name without triggerAriaLabel, which the "names the trigger from the visible label" test pins. The prop is an override for where the label alone does not say what the status is about, and the comment now says that.

Feedback item 7: not implemented.

The fallback is less specific than a caller-supplied name, agreed, but passing one is the caller's job and every caller does. The fallback is there so a caller who forgets gets an ambiguous dialog name rather than none.

Feedback item 6: not implemented.

Nothing is discarded today: ServersTable is the only call site and it does not pass interactive, so both labels are used. But that makes interactive a prop with no caller, like size, so it is removed in f025d8c.

@vishu-bh's review:
Thanks for checking this out! From LLM-assisted analysis of the feedback, the recommendation is to not move forward for the following reason:

!children does treat 0 as absent, but children == null isn't the right fix: {cond && <X/>} yields false, which must disable the popover rather than open an empty one, and == null would not catch that. A 0 as popover body doesn't arise in practice; a falsy conditional child does.

Lmk if I'm missing something, though pls! 🙌

[marekdano]

All findings have been addressed!
The PR looks good now.

LGTM 🚀

[vishu-bh]

PR looks god to merge but needs conflicts resolution