CORE-2708: Compose render-callback classNames in react-aria-components wrappers

[OpenStaxClaude]

Jira: CORE-2708

Problem

Several ui-components wrappers around react-aria-components (RAC) added their base CSS class with classNames('base', className). Any RAC props type extending RenderProps<T> types className as string | ((values) => string) — the callback form is how consumers style against component state (isOpen, isFocused, isSelected, …). classnames ignores function arguments, so the callback was silently discarded: the caller's classes never reached the DOM, and neither TypeScript nor the runtime reported anything.

> require('classnames')('base', (v) => 'dynamic-' + v)
'base'   // the callback is dropped, no warning

Changes

Use RAC's composeRenderProps to normalise both forms, following the pattern already applied to Mask, OverlayMask, and OverlayWrapper in #127.

Component	File
NavBarMenuItem	src/components/NavBarMenuButtons.tsx
NavBarPopover	src/components/NavBarMenuButtons.tsx
TreeCheckbox	src/components/Tree/TreeCheckbox.tsx
NavBarButton	src/components/NavBarButton.tsx

TreeCheckbox keeps building its class list into a local, but the local is now the composed callback, so the conditional checkbox-label--disabled class stays in the composed output.

NavBarButton decision

The ticket asked for a recorded decision here. Widened, rather than left narrowed.

NavBarButtonProps re-declared className?: string, which intersected with ButtonProps and narrowed the callback form away. Nothing was silently dropped (the type prevented it), but the wrapper removed a supported RAC API from the public surface for no stated reason. Removing the re-declaration lets the type inherit the callback form from ButtonProps, and the implementation now composes it. This only widens what is accepted — existing string className callers are unaffected. A comment on the type records why it is not re-declared, so the narrowing does not creep back in.

Testing

Each fixed component has a test that passes a callback className and asserts the composed result, plus a companion test asserting the string form still composes. Confirmed each new callback test fails before the fix (verified by reverting the four source files and re-running), so they are load-bearing rather than decorative.

• npm test — 240 passed, 116 snapshots passed. No snapshot churn: the emitted class list for the string form is unchanged.
• npm run lint — clean.
• npx tsc --noEmit — clean.

Out of scope, but confirmed real (flagged per the ticket)

style has the same callback-vs-value duality in StyleRenderProps, and these wrappers spread it into a plain object literal ({...style}). I checked whether that matters: it is the same bug, not a theoretical one.

• <NavBarPopover style={() => ({color: 'red'})} /> type-checks cleanly (npx tsc --noEmit) — spreading a function into an object literal is legal TS and contributes no properties.
• At runtime the caller's styles are silently dropped. Verified against TreeCheckbox: with a style callback, element.style.color is "".

So style is less protected than I assumed, not more — there is no type error to catch it. It needs the same composeRenderProps treatment, but it is a distinct change (the wrappers merge their CSS-variable objects into style, so composing means merging inside a callback rather than at the top of the component). Left out of this PR to keep it scoped to the ticket's acceptance criteria — happy to file the follow-up, or fold it in here if reviewers would rather it ship together.

🤖 Generated with Claude Code

---

CI: Jira check — resolved (correcting an earlier wrong diagnosis)

This check now passes. The cause was that .github/workflows/checks.yml referenced secrets.JiraEmail / secrets.JiraToken, but the org secrets are named JIRA_EMAIL / JIRA_TOKEN. The mismatched names resolved to empty strings, so the action sent an empty Basic auth header and Jira returned 401. @RoyEJohnson fixed the casing in a15d901.

I had concluded from the 401 that the org's Atlassian API token was expired or revoked and needed an admin to rotate it. That was wrong — nothing was wrong with the token, and no rotation is needed. I reasoned that because both inputs rendered as *** in the job log they must be set and non-empty; that inference doesn't hold, since GitHub masks the input display either way.

The v0.1.16 pin bump I pushed during review is no longer on this branch, and it isn't needed — v0.1.15 passes now that the secret names are right. It was still useful in the moment: v0.1.16 adds a GET /rest/api/3/myself pre-flight that surfaced the 401 in 4 seconds, where v0.1.15 has no response.ok check anywhere and funnels any auth failure into the misleading no matching issues found. That's worth fixing upstream on its own merits, but it's not this PR's business.

🤖 Generated with Claude Code

---

CI: jira-linked-action pin bumped (added in review)

Per review, .github/workflows/checks.yml now pins openstax/jira-linked-action@v0.1.16 (was v0.1.15).

This did not make the Jira check pass, but it diagnosed it. v0.1.16 adds a GET /rest/api/3/myself credential pre-flight, and the check now fails in 4s with:

Jira API error 401 Unauthorized: Client must be authenticated to access this resource.

So the check was never failing over linking, project keys, or dev-status indexing — the org-level JiraEmail/JiraToken secrets are being rejected. v0.1.15 had no response.ok check, so it passed the 401 body into the search logic and reported it as no matching issues found. Fixing it requires rotating the Atlassian API token behind the org secret, which needs org admin. See the comment below for the full write-up.

build (both workflows) and Deploy are green.

[OpenStaxClaude]

CI status: build green, Jira check red for a pre-existing reason

build passes (lint + full test suite) and the Deploy workflow succeeded. The only red check is Jira, failing with no matching issues found. I don't believe that's about this PR, and here's what I checked rather than assuming:

It is not a missing link. CORE-2708 does show this PR in its Jira development panel (CORE-2708: … (#136), source CORE-2708-compose-render-props-classnames → main). The branch name, PR title, and commit message all carry the key.

It is not the candidate query. This repo pins openstax/jira-linked-action@v0.1.15, which runs:

project = DISCO and resolution is empty and development[pullrequests].all > 0

then queries /rest/dev-status/1.0/issue/details?applicationType=github&dataType=pullrequest per issue and looks for an exact pr.url match. I ran that JQL directly: it returns 48 issues and CORE-2708 is in the list, so the DISCO-vs-CORE project key is resolving fine and the ticket is a candidate. The failure is downstream, in the per-issue dev-status comparison — which I can't inspect without the Jira API credentials the action uses.

It is not specific to this PR. #127 fails the identical check today, and it is open, non-draft, and correctly linked to CORE-2002. #134 and #135 were both merged with this check red. I re-ran the job three times over ~15 minutes (in case it was an indexing lag after PR creation) — same result each time.

For what it's worth, the action's README documents a hardcoded DISCO → CORE redirect and an issue-key fast path that were added in v0.1.16; this repo is pinned to v0.1.15, which has neither. Bumping the pin is the obvious thing to try, but I'd rather not change shared CI config as a drive-by on a component-library bugfix. Happy to open a separate ticket/PR for it if that's wanted.

Flagging rather than silently ignoring — if a reviewer knows this check to be meaningful here and I've misread it, please say so and I'll dig further.

[Copilot]

Pull request overview

Fixes react-aria-components wrapper components so caller-supplied render-callback className props are preserved (instead of being silently dropped by classnames), matching established RAC patterns in this codebase.

Changes:

• Use composeRenderProps to compose base wrapper classes with both string and render-callback className forms in NavBarMenuItem, NavBarPopover, NavBarButton, and TreeCheckbox.
• Widen NavBarButtonProps by removing the className?: string re-declaration so the RAC callback form is supported by the public API.
• Add targeted tests verifying both callback and string className composition; document the fix in the changelog.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
src/components/Tree/TreeCheckbox.tsx	Compose base + caller className via composeRenderProps so callback classNames reach the DOM.
src/components/Tree/TreeCheckbox.spec.tsx	Add DOM assertions covering callback and string className composition (including disabled modifier).
src/components/NavBarMenuButtons.tsx	Compose base + caller className for NavBarMenuItem and NavBarPopover using composeRenderProps.
src/components/NavBarMenuButtons.spec.tsx	Add tests verifying callback and string className composition for NavBarMenuItem and NavBarPopover.
src/components/NavBarButton.tsx	Stop narrowing className in props and compose base + caller className with composeRenderProps.
src/components/NavBarButton.spec.tsx	Add tests verifying callback and string className composition for NavBarButton.
CHANGELOG.md	Document the fixed render-callback className behavior and the NavBarButtonProps type widening.

---

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

[RoyEJohnson]

Let's try bumping the pin.

[OpenStaxClaude]

Pin bumped — and it found the actual problem: the Jira credentials are 401ing

Pushed 7dd3d9b bumping openstax/jira-linked-action from v0.1.15 to v0.1.16 (latest tag). build is green.

The Jira check is still red, but it now fails with something actionable instead of no matching issues found:

##[error]Jira API error 401 Unauthorized: Client must be authenticated to access this resource.

It fails in 4 seconds, before any issue search runs. v0.1.16 added a GET /rest/api/3/myself credential pre-flight, and that is what is rejecting. So this was never about linking, project keys, or dev-status indexing — the Jira API credentials the action is given are not valid.

That also explains the things I couldn't reconcile in my earlier comment: why #127 fails the same check while correctly linked, why #134/#135 merged red, and why the candidate JQL returns CORE-2708 fine when I run it under my own credentials. v0.1.15 had no response.ok check anywhere — it passed the 401 body straight into .json() and carried on, so every auth failure surfaced as no matching issues found. Bumping the pin was the right call; thanks for the nudge.

What needs to happen next — not something I can do

secrets.JiraEmail / secrets.JiraToken are not repo-level secrets here:

$ gh api repos/openstax/ui-components/actions/secrets
{"total_count":0,"secrets":[]}

So they resolve from org-level openstax secrets, which means this is broken for every repo running this action, not just this one. Both render as *** in the job log, so they are set and non-empty — they are being rejected, which is what an expired or revoked Atlassian API token looks like.

Someone with org admin needs to rotate the Atlassian API token behind the org JiraToken secret (and confirm JiraEmail matches the account that owns it). I don't have admin:org, so I can't read or set them, and I can't verify a fix from here.

What I'd suggest for this PR

The pin bump is worth keeping regardless of when the token gets rotated — it converts a silent, misleading failure into a one-line diagnosis, and it's the version this repo's own upstream workflow uses. But the check will stay red until the secret is fixed, and that is unrelated to the component changes under review.

Happy to go either way: merge with the check red (as #134/#135 were), or hold until the token is rotated. Your call — and if you'd rather the pin bump not ride along on a component-library bugfix now that we know the fix is elsewhere, say so and I'll pull it into its own PR.

Still open from before

The style follow-up. It is the same silent-drop bug as className, confirmed at runtime and not caught by the type checker, so it is real and currently untracked. Unless you'd rather I fold it into this PR, I'll file it as its own CORE ticket so it doesn't get lost when this merges.

[OpenStaxClaude]

Correcting my earlier write-up on this thread, and closing out the CI thread.

The Jira check passes now. The cause was the secret names, not the credentials: the workflow referenced secrets.JiraEmail / secrets.JiraToken while the org secrets are JIRA_EMAIL / JIRA_TOKEN. They resolved to empty strings, the action sent an empty Basic auth header, and Jira returned 401. Thanks @RoyEJohnson for spotting it in a15d901.

I was wrong to conclude the org's Atlassian token was expired or revoked and needed an admin to rotate it. Nothing was wrong with the token. My reasoning was that both inputs rendered as *** in the job log, so they must be set and non-empty — that inference doesn't hold, since GitHub masks the input display either way. Sorry for pointing at an infra fix that wasn't needed.

The v0.1.16 pin is not on this branch and isn't needed — v0.1.15 passes now. I've removed the stale section from the PR description. The bump did earn its keep as a diagnostic (its GET /rest/api/3/myself pre-flight surfaced the 401 in 4s, where v0.1.15 has no response.ok check and funnels every auth failure into no matching issues found), so hardening that upstream is worth doing on its own — but not from this PR.

Ready for review. Jira, build, and Deploy are all green on a15d901. The review comment on this PR was "let's try bumping the pin", which the casing fix has now resolved by other means, so there's nothing outstanding from it on my side.

One note on the local suite: ProfileMenu has an intermittent snapshot failure on data-focus-visible / data-focused that shows up only in full-suite runs, never in isolation, and never in CI. It predates this branch and is unrelated to these changes — flagging it as something worth its own look, not asking for anything here.