Skip to content

ref(settings): update action prop and remove hasPageFrame#115815

Merged
natemoo-re merged 24 commits into
masterfrom
nm/page-frame/settings-actions
May 21, 2026
Merged

ref(settings): update action prop and remove hasPageFrame#115815
natemoo-re merged 24 commits into
masterfrom
nm/page-frame/settings-actions

Conversation

@natemoo-re
Copy link
Copy Markdown
Member

@natemoo-re natemoo-re commented May 19, 2026

Apologies for the big cleanup PR, but it was a bunch of interconnected changes that were hard to break up.

Settings pages projected page-level action buttons into the TopBar via SettingsPageHeader.actionTopBar.Slot name="actions". The TopBar should only contain search, Ask Seer, and feedback. This PR removes that projection, cleans up the fully-rolled-out hasPageFrame feature flag, and aligns billing/usage layouts.

TopBar & SettingsSearch

  • New search slot on TopBar; SettingsSearch moves from actionssearch

SettingsPageHeader

  • Remove legacy non-page-frame rendering path, noTitleStyles, className props
  • Repurpose action prop: renders inline next to subtitle instead of projecting into TopBar
  • Subtitle gets a default max-width: 72ch for readability
  • Consumers pass subtitle + action as props, no Flex boilerplate

hasPageFrame cleanup

  • Remove useHasPageFrameFeature() from settings infrastructure and ~40 page components
  • Delete orphaned organizationRegionAction.tsx and unused settingsHeader.tsx
  • Last breadcrumb segment renders as <span> (not a link), matching page-frame design

Inline actions

  • Pages with standalone action buttons (Dynamic Sampling, Console SDK Invites, Seer Autofix/Code Review, Repositories, API Tokens, Org Auth Tokens, Client Keys, Relay, User Feedback) now use the action prop directly
  • Pages with custom feedback buttons use <TopBar.Slot name="feedback"> instead

Billing/Usage layouts

  • Align SubscriptionSettingsLayout with SettingsLayout: remove dead hasPageFrame ternary, switch to search TopBar slot, add background="primary"
  • Clean up SubscriptionPageContainer to hardcode page-frame path
  • Remove background="secondary" prop from SubscriptionPageContainer callers

Spacing

  • Settings layout top padding normalized: md (8px) → xl (16px)
  • Remove redundant padding-top on Subtitle component

@github-actions github-actions Bot added the Scope: Frontend Automatically applied to PRs that change frontend components label May 19, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 19, 2026

📊 Type Coverage Diff

✅ No new type safety issues introduced. Coverage: 93.60%

Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 15dc8b1. Configure here.

Comment thread static/app/views/settings/components/settingsPageHeader.tsx
@natemoo-re natemoo-re changed the title ref(page-frame): remove action prop and hasPageFrame from settings pages ref(settings): update action prop and hasPageFrame May 20, 2026
@natemoo-re natemoo-re changed the title ref(settings): update action prop and hasPageFrame ref(settings): update action prop and remove hasPageFrame May 20, 2026
Comment on lines +22 to +26
<Fragment>
{typeof title === 'string' ? (
<BreadcrumbTitle routes={routes} title={title} />
) : (
title && <Layout.Title>{title}</Layout.Title>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: SettingsPageHeader now uses BreadcrumbTitle which renders null outside a BreadcrumbProvider, causing the title on the 'Accept organization invite' page to disappear.
Severity: HIGH

Suggested Fix

In SettingsPageHeader, add a check for the BreadcrumbContext. If the context is not available, fall back to rendering the title using a component that does not depend on it, such as the previous <Layout.Title>{title}</Layout.Title>. This will restore the title on pages like acceptOrganizationInvite that are rendered outside the main settings layout.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: static/app/views/settings/components/settingsPageHeader.tsx#L22-L26

Potential issue: The `SettingsPageHeader` component was changed to unconditionally use
`BreadcrumbTitle` for string titles. However, `BreadcrumbTitle` is a no-op and renders
`null` when not used within a `BreadcrumbProvider` context. The
`acceptOrganizationInvite` page is rendered outside any settings layout that provides
this context, as it's part of an unauthenticated flow with no organization context. As a
result, the page title, `t('Accept organization invite')`, is no longer displayed to
users visiting the organization invite acceptance page, leaving the page without a
heading.

Also affects:

  • static/app/views/acceptOrganizationInvite/index.tsx

Did we get this right? 👍 / 👎 to inform future reviews.

Copy link
Copy Markdown
Member

@JonasBa JonasBa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes look good - I've went and reviewed the preview deployment and checked the pages, and everything looks good. Great cleanup @natemoo-re 🫶

} from './constants';

const Slot = slot(['title', 'actions', 'feedback'] as const);
const Slot = slot(['title', 'search', 'actions', 'feedback'] as const);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great idea to extend the topbar with a search slot 👍🏼

…sPageFrame from settings infrastructure

Add a dedicated 'search' slot to TopBar for SettingsSearch, replacing its
use of the 'actions' slot. Remove the action prop from SettingsPageHeader
entirely — it was projecting page-level actions into the TopBar, which
should only contain search, Ask Seer, and feedback.

Strip all hasPageFrame feature-flag checks from settings infrastructure
components (SettingsPageHeader, SettingsLayout, SettingsHeader,
SettingsWrapper, SettingsBreadcrumb) since the flag is now always true.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
natemoo-re and others added 17 commits May 21, 2026 11:00
More tests that relied on SettingsPageHeader rendering visible heading
text. Also fixes breadcrumb test (last crumb is now a span, not a
link) and moves key-details test-id to a wrapper div so acceptance
tests can find it.
Move key-details data-test-id to wrapper div with proper indentation.
Fix projectKeys details test to use test-id instead of heading text.
Fix audit log test to use visible page content.
Bump desktop top padding from md (8px) to xl (16px) so
descriptions and alerts below the TopBar have more breathing room.
Remove hasPageFrame dead code from SubscriptionSettingsLayout and
SubscriptionPageContainer. Use search TopBar slot instead of actions.
Normalize default padding to xl to match other settings pages.
… rendering

SettingsPageHeader now renders action inline next to subtitle in a
Flex layout, replacing the boilerplate each consumer repeated. This
simplifies 10 files that previously wrapped subtitle+action in Flex.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ernal Flex wrappers

Move actions back into the `action` prop where the remove/restore
roundtrip left them rendered outside the component. Fix subtitle+action
Flex alignment from `center` to `start` for multiline subtitles, and
drop the no-op `padding: 0` on Subtitle.
…r callers

Callers were passing background="secondary" which overrode the
hardcoded background="primary" in SubscriptionPageContainer via
the ...rest spread, preventing the white page background from
rendering.

Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
Constrain subtitle text to 72ch for better readability, regardless of
whether page-level actions are present.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@natemoo-re natemoo-re merged commit e773d88 into master May 21, 2026
70 checks passed
@natemoo-re natemoo-re deleted the nm/page-frame/settings-actions branch May 21, 2026 15:32
natemoo-re added a commit that referenced this pull request May 21, 2026
…es (#116013)

Follow-up to #115815. That PR removed the `hasPageFrame` branch from
`SettingsPageHeader`, making it unconditionally render `BreadcrumbTitle`
for string titles. `BreadcrumbTitle` returns `null` and only updates
breadcrumb context as a side effect — but the accept-invite and
accept-transfer pages are routed outside `SettingsWrapper`, so there's
no `BreadcrumbProvider` ancestor. The page headings silently
disappeared.

These pages aren't settings pages, so they shouldn't use
`SettingsPageHeader` at all. Replaced with `Layout.Title` directly.
JonasBa pushed a commit that referenced this pull request May 21, 2026
Apologies for the big cleanup PR, but it was a bunch of interconnected
changes that were hard to break up.

Settings pages projected page-level action buttons into the TopBar via
`SettingsPageHeader.action` → `TopBar.Slot name="actions"`. The TopBar
should only contain search, Ask Seer, and feedback. This PR removes that
projection, cleans up the fully-rolled-out `hasPageFrame` feature flag,
and aligns billing/usage layouts.

**TopBar & SettingsSearch**
- New `search` slot on TopBar; `SettingsSearch` moves from `actions` →
`search`

**SettingsPageHeader**
- Remove legacy non-page-frame rendering path, `noTitleStyles`,
`className` props
- Repurpose `action` prop: renders inline next to subtitle instead of
projecting into TopBar
- Subtitle gets a default `max-width: 72ch` for readability
- Consumers pass `subtitle` + `action` as props, no Flex boilerplate

**`hasPageFrame` cleanup**
- Remove `useHasPageFrameFeature()` from settings infrastructure and ~40
page components
- Delete orphaned `organizationRegionAction.tsx` and unused
`settingsHeader.tsx`
- Last breadcrumb segment renders as `<span>` (not a link), matching
page-frame design

**Inline actions**
- Pages with standalone action buttons (Dynamic Sampling, Console SDK
Invites, Seer Autofix/Code Review, Repositories, API Tokens, Org Auth
Tokens, Client Keys, Relay, User Feedback) now use the `action` prop
directly
- Pages with custom feedback buttons use `<TopBar.Slot name="feedback">`
instead

**Billing/Usage layouts**
- Align `SubscriptionSettingsLayout` with `SettingsLayout`: remove dead
`hasPageFrame` ternary, switch to `search` TopBar slot, add
`background="primary"`
- Clean up `SubscriptionPageContainer` to hardcode page-frame path
- Remove `background="secondary"` prop from `SubscriptionPageContainer`
callers

**Spacing**
- Settings layout top padding normalized: `md` (8px) → `xl` (16px)
- Remove redundant `padding-top` on `Subtitle` component

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
JonasBa pushed a commit that referenced this pull request May 21, 2026
…es (#116013)

Follow-up to #115815. That PR removed the `hasPageFrame` branch from
`SettingsPageHeader`, making it unconditionally render `BreadcrumbTitle`
for string titles. `BreadcrumbTitle` returns `null` and only updates
breadcrumb context as a side effect — but the accept-invite and
accept-transfer pages are routed outside `SettingsWrapper`, so there's
no `BreadcrumbProvider` ancestor. The page headings silently
disappeared.

These pages aren't settings pages, so they shouldn't use
`SettingsPageHeader` at all. Replaced with `Layout.Title` directly.
natemoo-re added a commit that referenced this pull request May 21, 2026
Apologies for the big cleanup PR, but it was a bunch of interconnected
changes that were hard to break up.

Settings pages projected page-level action buttons into the TopBar via
`SettingsPageHeader.action` → `TopBar.Slot name="actions"`. The TopBar
should only contain search, Ask Seer, and feedback. This PR removes that
projection, cleans up the fully-rolled-out `hasPageFrame` feature flag,
and aligns billing/usage layouts.

**TopBar & SettingsSearch**
- New `search` slot on TopBar; `SettingsSearch` moves from `actions` →
`search`

**SettingsPageHeader**
- Remove legacy non-page-frame rendering path, `noTitleStyles`,
`className` props
- Repurpose `action` prop: renders inline next to subtitle instead of
projecting into TopBar
- Subtitle gets a default `max-width: 72ch` for readability
- Consumers pass `subtitle` + `action` as props, no Flex boilerplate

**`hasPageFrame` cleanup**
- Remove `useHasPageFrameFeature()` from settings infrastructure and ~40
page components
- Delete orphaned `organizationRegionAction.tsx` and unused
`settingsHeader.tsx`
- Last breadcrumb segment renders as `<span>` (not a link), matching
page-frame design

**Inline actions**
- Pages with standalone action buttons (Dynamic Sampling, Console SDK
Invites, Seer Autofix/Code Review, Repositories, API Tokens, Org Auth
Tokens, Client Keys, Relay, User Feedback) now use the `action` prop
directly
- Pages with custom feedback buttons use `<TopBar.Slot name="feedback">`
instead

**Billing/Usage layouts**
- Align `SubscriptionSettingsLayout` with `SettingsLayout`: remove dead
`hasPageFrame` ternary, switch to `search` TopBar slot, add
`background="primary"`
- Clean up `SubscriptionPageContainer` to hardcode page-frame path
- Remove `background="secondary"` prop from `SubscriptionPageContainer`
callers

**Spacing**
- Settings layout top padding normalized: `md` (8px) → `xl` (16px)
- Remove redundant `padding-top` on `Subtitle` component

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
natemoo-re added a commit that referenced this pull request May 21, 2026
…es (#116013)

Follow-up to #115815. That PR removed the `hasPageFrame` branch from
`SettingsPageHeader`, making it unconditionally render `BreadcrumbTitle`
for string titles. `BreadcrumbTitle` returns `null` and only updates
breadcrumb context as a side effect — but the accept-invite and
accept-transfer pages are routed outside `SettingsWrapper`, so there's
no `BreadcrumbProvider` ancestor. The page headings silently
disappeared.

These pages aren't settings pages, so they shouldn't use
`SettingsPageHeader` at all. Replaced with `Layout.Title` directly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants