Skip to content

ref(nav) use topbar slot for feedback button #112443

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
JonasBa merged 11 commits into from
Apr 9, 2026
Merged

ref(nav) use topbar slot for feedback button #112443

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
ef655ae
docs: Add FeedbackButton TopBar.Slot migration task list
JonasBa Apr 7, 2026
b6140f2
ref(nav): Wrap FeedbackButton in TopBar.Slot for page frame
JonasBa Apr 7, 2026
3446474
ref(nav): Remove size prop from FeedbackButton inside TopBar.Slot and…
JonasBa Apr 8, 2026
7d577ab
ref(nav) remove plan
JonasBa Apr 8, 2026
617ff8f
:hammer_and_wrench: apply pre-commit fixes
getsantry[bot] Apr 8, 2026
c53a5c8
ref(nav): Hoist duplicated FeedbackButton feedbackOptions to module-l…
JonasBa Apr 8, 2026
d601b5a
:hammer_and_wrench: apply pre-commit fixes
getsantry[bot] Apr 8, 2026
fdf5f78
fix(trace-summary): return null when hasPageFrameFeature is true to a…
cursoragent Apr 8, 2026
7d54af5
refactor(issue-details): extract feedback options to const to avoid d…
cursoragent Apr 9, 2026
59185a6
Merge branch 'master' into jb/nav/settingspageheader
cursoragent Apr 9, 2026
8461e0e
refactor(feedback): extract feedback options to const in automation a…
cursoragent Apr 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion static/app/components/profiling/continuousProfileHeader.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import {trackAnalytics} from 'sentry/utils/analytics';
import {generateLinkToEventInTraceView} from 'sentry/utils/discover/urls';
import {useLocation} from 'sentry/utils/useLocation';
import {useOrganization} from 'sentry/utils/useOrganization';
import {TopBar} from 'sentry/views/navigation/topBar';
import {useHasPageFrameFeature} from 'sentry/views/navigation/useHasPageFrameFeature';

interface ContinuousProfileHeader {
transaction: Event | null;
Expand All @@ -21,6 +23,7 @@ interface ContinuousProfileHeader {
export function ContinuousProfileHeader({transaction}: ContinuousProfileHeader) {
const location = useLocation();
const organization = useOrganization();
const hasPageFrameFeature = useHasPageFrameFeature();

// @TODO add breadcrumbs when other views are implemented
const breadCrumbs = useMemo((): ProfilingBreadcrumbsProps['trails'] => {
Expand Down Expand Up @@ -51,7 +54,13 @@ export function ContinuousProfileHeader({transaction}: ContinuousProfileHeader)
</SmallerProfilingBreadcrumbsWrapper>
</SmallerHeaderContent>
<StyledHeaderActions>
<FeedbackButton />
{hasPageFrameFeature ? (
Copy link
Copy Markdown
Contributor

@sentry sentry bot Apr 8, 2026

Choose a reason for hiding this comment

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

Bug: When hasPageFrameFeature is true, the FeedbackButton is rendered as an icon-only button without an aria-label, making it inaccessible to screen reader users.
Severity: MEDIUM

Suggested Fix

In all 10 modified files, add an aria-label to the FeedbackButton component when it is rendered as an icon-only button. For example: <FeedbackButton aria-label={t('Give Feedback')}>.... This will provide an accessible name for screen readers, matching the pattern already used for the fallback button in topBar.tsx.

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/components/profiling/continuousProfileHeader.tsx#L57

Potential issue: When the `page-frame` feature flag is enabled, `FeedbackButton`
components are rendered inside `TopBar.Slot` with `{null}` as children. This creates an
icon-only button that lacks an accessible name, as no `aria-label` or visually-hidden
text is provided. This violates WCAG accessibility standards because users with screen
readers will hear the element announced as just a "button" without any context about its
purpose. This accessibility regression is present in all 10 files modified by the pull
request.

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

<TopBar.Slot name="feedback">
<FeedbackButton>{null}</FeedbackButton>
Copy link
Copy Markdown
Contributor

@nsdeschenes nsdeschenes Apr 8, 2026

Choose a reason for hiding this comment

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

Is this null child needed, or can it just be a self-closing tag?

Copy link
Copy Markdown
Member Author

@JonasBa JonasBa Apr 9, 2026

Choose a reason for hiding this comment

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

No, we need it. If you check <FeedbackButton, it checks for props.children === undefined and initializes a default label otherwise 😢

</TopBar.Slot>
) : (
<FeedbackButton />
)}
{transactionTarget && (
<LinkButton size="sm" onClick={handleGoToTransaction} to={transactionTarget}>
{t('Go to Trace')}
Expand Down
11 changes: 10 additions & 1 deletion static/app/components/profiling/profileHeader.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import {generateLinkToEventInTraceView} from 'sentry/utils/discover/urls';
import {isSchema, isSentrySampledProfile} from 'sentry/utils/profiling/guards/profile';
import {useLocation} from 'sentry/utils/useLocation';
import {useOrganization} from 'sentry/utils/useOrganization';
import {TopBar} from 'sentry/views/navigation/topBar';
import {useHasPageFrameFeature} from 'sentry/views/navigation/useHasPageFrameFeature';
import {useProfiles} from 'sentry/views/profiling/profilesProvider';

function getTransactionName(input: Profiling.ProfileInput): string {
Expand All @@ -36,6 +38,7 @@ function ProfileHeader({transaction, projectId, eventId}: ProfileHeaderProps) {
const location = useLocation();
const organization = useOrganization();
const profiles = useProfiles();
const hasPageFrameFeature = useHasPageFrameFeature();

const transactionName =
profiles.type === 'resolved' ? getTransactionName(profiles.data) : '';
Expand Down Expand Up @@ -89,7 +92,13 @@ function ProfileHeader({transaction, projectId, eventId}: ProfileHeaderProps) {
</SmallerProfilingBreadcrumbsWrapper>
</SmallerHeaderContent>
<StyledHeaderActions>
<FeedbackButton />
{hasPageFrameFeature ? (
<TopBar.Slot name="feedback">
<FeedbackButton>{null}</FeedbackButton>
</TopBar.Slot>
) : (
<FeedbackButton />
Comment on lines +95 to +100
Copy link
Copy Markdown
Contributor

@sentry sentry bot Apr 8, 2026

Choose a reason for hiding this comment

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

Bug: The FeedbackButton is rendered as an icon-only button without an aria-label, making it inaccessible to screen readers.
Severity: MEDIUM

Suggested Fix

Add aria-label={t('Give Feedback')} to all instances of FeedbackButton that are rendered inside TopBar.Slot. This will provide an accessible name for screen readers, consistent with the pattern used for the fallback button in topBar.tsx.

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/components/profiling/profileHeader.tsx#L95-L100

Potential issue: The `FeedbackButton` component is rendered inside `TopBar.Slot` with
`children={null}`. This configuration creates an icon-only button. However, no
`aria-label` is provided as a prop. The underlying button component uses `aria-label` or
string `children` to create an accessible name. Since both are missing, the rendered
button has no accessible name, making it unusable for screen reader users. This
accessibility issue is present in all 10 files where the `FeedbackButton` was migrated
to use the `TopBar.Slot` pattern.

)}
{transactionTarget && (
<LinkButton size="sm" onClick={handleGoToTransaction} to={transactionTarget}>
{t('Go to Trace')}
Expand Down
11 changes: 10 additions & 1 deletion static/app/views/alerts/list/header.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import {useLocation} from 'sentry/utils/useLocation';
import {useNavigate} from 'sentry/utils/useNavigate';
import {useOrganization} from 'sentry/utils/useOrganization';
import {makeAlertsPathname} from 'sentry/views/alerts/pathnames';
import {TopBar} from 'sentry/views/navigation/topBar';
import {useHasPageFrameFeature} from 'sentry/views/navigation/useHasPageFrameFeature';

type Props = {
activeTab: 'stream' | 'rules';
Expand All @@ -25,6 +27,7 @@ export function AlertHeader({activeTab}: Props) {
const location = useLocation();
const organization = useOrganization();
const {selection} = usePageFilters();
const hasPageFrameFeature = useHasPageFrameFeature();
/**
* Incidents list is currently at the organization level, but the link needs to
* go down to a specific project scope.
Expand Down Expand Up @@ -79,7 +82,13 @@ export function AlertHeader({activeTab}: Props) {
>
{t('Create Alert')}
</CreateAlertButton>
<FeedbackButton />
{hasPageFrameFeature ? (
<TopBar.Slot name="feedback">
<FeedbackButton>{null}</FeedbackButton>
</TopBar.Slot>
) : (
<FeedbackButton />
)}
<LinkButton
size="sm"
onClick={handleNavigateToSettings}
Expand Down
33 changes: 23 additions & 10 deletions static/app/views/automations/components/automationFeedbackButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,31 @@
import {FeedbackButton} from 'sentry/components/feedbackButton/feedbackButton';
import {t} from 'sentry/locale';
import {TopBar} from 'sentry/views/navigation/topBar';
import {useHasPageFrameFeature} from 'sentry/views/navigation/useHasPageFrameFeature';

const automationFeedbackOptions = {
messagePlaceholder: t('How can we improve the alerts experience?'),
tags: {
['feedback.source']: 'automations',
['feedback.owner']: 'aci',
},
};

export function AutomationFeedbackButton() {
const hasPageFrameFeature = useHasPageFrameFeature();

if (hasPageFrameFeature) {
return (
<TopBar.Slot name="feedback">
<FeedbackButton feedbackOptions={automationFeedbackOptions}>
{null}
</FeedbackButton>
</TopBar.Slot>
);
}

return (
<FeedbackButton
size="sm"
feedbackOptions={{
messagePlaceholder: t('How can we improve the alerts experience?'),
tags: {
['feedback.source']: 'automations',
['feedback.owner']: 'aci',
},
}}
>
<FeedbackButton size="sm" feedbackOptions={automationFeedbackOptions}>
{t('Feedback')}
</FeedbackButton>
);
Expand Down
11 changes: 10 additions & 1 deletion static/app/views/dashboards/manage/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ import type {DashboardsLayout} from 'sentry/views/dashboards/manage/types';
import {DashboardFilter, PREBUILT_DASHBOARD_LABEL} from 'sentry/views/dashboards/types';
import type {DashboardDetails, DashboardListItem} from 'sentry/views/dashboards/types';
import {PREBUILT_DASHBOARDS} from 'sentry/views/dashboards/utils/prebuiltConfigs';
import {TopBar} from 'sentry/views/navigation/topBar';
import {useHasPageFrameFeature} from 'sentry/views/navigation/useHasPageFrameFeature';
import RouteError from 'sentry/views/routeError';

import DashboardGrid from './dashboardGrid';
Expand Down Expand Up @@ -162,6 +164,7 @@ function ManageDashboards() {
const navigate = useNavigate();
const location = useLocation();
const api = useApi();
const hasPageFrameFeature = useHasPageFrameFeature();
const dashboardGridRef = useRef<HTMLDivElement>(null);
const hasPrebuiltDashboards = organization.features.includes(
'dashboards-prebuilt-insights-dashboards'
Expand Down Expand Up @@ -645,7 +648,13 @@ function ManageDashboards() {
</TemplateSwitch>
)}

<FeedbackButton />
{hasPageFrameFeature ? (
<TopBar.Slot name="feedback">
<FeedbackButton>{null}</FeedbackButton>
</TopBar.Slot>
) : (
<FeedbackButton />
)}
<Feature features={['dashboards-ai-generate']}>
{({hasFeature: hasAiGenerate}) =>
hasAiGenerate && areAiFeaturesAllowed ? (
Expand Down
31 changes: 21 additions & 10 deletions static/app/views/detectors/components/monitorFeedbackButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,29 @@
import {FeedbackButton} from 'sentry/components/feedbackButton/feedbackButton';
import {t} from 'sentry/locale';
import {TopBar} from 'sentry/views/navigation/topBar';
import {useHasPageFrameFeature} from 'sentry/views/navigation/useHasPageFrameFeature';

const monitorFeedbackOptions = {
messagePlaceholder: t('How can we improve the monitor experience?'),
tags: {
['feedback.source']: 'monitors',
['feedback.owner']: 'aci',
},
};

export function MonitorFeedbackButton() {
const hasPageFrameFeature = useHasPageFrameFeature();

if (hasPageFrameFeature) {
return (
<TopBar.Slot name="feedback">
<FeedbackButton feedbackOptions={monitorFeedbackOptions}>{null}</FeedbackButton>
</TopBar.Slot>
);
}

return (
<FeedbackButton
size="sm"
feedbackOptions={{
messagePlaceholder: t('How can we improve the monitor experience?'),
tags: {
['feedback.source']: 'monitors',
['feedback.owner']: 'aci',
},
}}
>
<FeedbackButton size="sm" feedbackOptions={monitorFeedbackOptions}>
{t('Feedback')}
</FeedbackButton>
);
Expand Down
11 changes: 10 additions & 1 deletion static/app/views/explore/errors/content.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ import {
} from 'sentry/views/explore/errors/body';
import {ErrorsFilterSection} from 'sentry/views/explore/errors/filterContent';
import {useControlSectionExpanded} from 'sentry/views/explore/hooks/useControlSectionExpanded';
import {TopBar} from 'sentry/views/navigation/topBar';
import {useHasPageFrameFeature} from 'sentry/views/navigation/useHasPageFrameFeature';

export default function ErrorsContent() {
const organization = useOrganization();
Expand All @@ -37,6 +39,7 @@ export default function ErrorsContent() {
}

function ErrorsHeader() {
const hasPageFrameFeature = useHasPageFrameFeature();
return (
<Layout.Header unified>
<Layout.HeaderContent unified>
Expand All @@ -45,7 +48,13 @@ function ErrorsHeader() {
</Layout.Title>
</Layout.HeaderContent>
<Layout.HeaderActions>
<FeedbackButton />
{hasPageFrameFeature ? (
<TopBar.Slot name="feedback">
<FeedbackButton>{null}</FeedbackButton>
</TopBar.Slot>
) : (
<FeedbackButton />
)}
</Layout.HeaderActions>
</Layout.Header>
);
Expand Down
29 changes: 20 additions & 9 deletions static/app/views/explore/logs/content.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ import {
} from 'sentry/views/explore/queryParams/context';
import {TraceItemDataset} from 'sentry/views/explore/types';
import {useOnboardingProject} from 'sentry/views/insights/common/queries/useOnboardingProject';
import {TopBar} from 'sentry/views/navigation/topBar';
import {useHasPageFrameFeature} from 'sentry/views/navigation/useHasPageFrameFeature';

export default function LogsContent() {
const organization = useOrganization();
Expand Down Expand Up @@ -93,12 +95,21 @@ export default function LogsContent() {
);
}

const logsFeedbackOptions = {
messagePlaceholder: t('How can we make logs work better for you?'),
tags: {
['feedback.source']: 'logs-listing',
['feedback.owner']: 'performance',
},
};

function LogsHeader() {
const pageId = useQueryParamsId();
const title = useQueryParamsTitle();
const organization = useOrganization();
const {data: savedQuery} = useGetSavedQuery(pageId);
const onboardingProject = useOnboardingProject({property: 'hasLogs'});
const hasPageFrameFeature = useHasPageFrameFeature();

const hasSavedQueryTitle =
defined(pageId) && defined(savedQuery) && savedQuery.name.length > 0;
Expand All @@ -120,15 +131,15 @@ function LogsHeader() {
</Layout.HeaderContent>
<Layout.HeaderActions>
<Grid flow="column" align="center" gap="md">
<FeedbackButton
feedbackOptions={{
messagePlaceholder: t('How can we make logs work better for you?'),
tags: {
['feedback.source']: 'logs-listing',
['feedback.owner']: 'performance',
},
}}
/>
{hasPageFrameFeature ? (
<TopBar.Slot name="feedback">
<FeedbackButton feedbackOptions={logsFeedbackOptions}>
{null}
</FeedbackButton>
</TopBar.Slot>
) : (
<FeedbackButton feedbackOptions={logsFeedbackOptions} />
)}
{defined(onboardingProject) && <SetupLogsButton />}
</Grid>
</Layout.HeaderActions>
Expand Down
29 changes: 20 additions & 9 deletions static/app/views/explore/metrics/content.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ import {
} from 'sentry/views/explore/queryParams/savedQuery';
import {TraceItemDataset} from 'sentry/views/explore/types';
import {useOnboardingProject} from 'sentry/views/insights/common/queries/useOnboardingProject';
import {TopBar} from 'sentry/views/navigation/topBar';
import {useHasPageFrameFeature} from 'sentry/views/navigation/useHasPageFrameFeature';

export default function MetricsContent() {
const organization = useOrganization();
Expand Down Expand Up @@ -69,12 +71,21 @@ export default function MetricsContent() {
);
}

const metricsFeedbackOptions = {
messagePlaceholder: t('How can we make metrics work better for you?'),
tags: {
['feedback.source']: 'metrics-listing',
['feedback.owner']: 'performance',
},
};

function MetricsHeader() {
const location = useLocation();
const pageId = getIdFromLocation(location, ID_KEY);
const title = getTitleFromLocation(location, TITLE_KEY);
const organization = useOrganization();
const {data: savedQuery} = useGetSavedQuery(pageId);
const hasPageFrameFeature = useHasPageFrameFeature();

const hasSavedQueryTitle =
defined(pageId) && defined(savedQuery) && savedQuery.name.length > 0;
Expand All @@ -97,15 +108,15 @@ function MetricsHeader() {
</Layout.Title>
</Layout.HeaderContent>
<Layout.HeaderActions>
<FeedbackButton
feedbackOptions={{
messagePlaceholder: t('How can we make metrics work better for you?'),
tags: {
['feedback.source']: 'metrics-listing',
['feedback.owner']: 'performance',
},
}}
/>
{hasPageFrameFeature ? (
<TopBar.Slot name="feedback">
<FeedbackButton feedbackOptions={metricsFeedbackOptions}>
{null}
</FeedbackButton>
</TopBar.Slot>
) : (
<FeedbackButton feedbackOptions={metricsFeedbackOptions} />
)}
</Layout.HeaderActions>
</Layout.Header>
);
Expand Down
11 changes: 10 additions & 1 deletion static/app/views/explore/multiQueryMode/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ import {useGetSavedQuery} from 'sentry/views/explore/hooks/useGetSavedQueries';
import {MultiQueryModeContent} from 'sentry/views/explore/multiQueryMode/content';
import {SavedQueryEditMenu} from 'sentry/views/explore/savedQueryEditMenu';
import {StarSavedQueryButton} from 'sentry/views/explore/starSavedQueryButton';
import {TopBar} from 'sentry/views/navigation/topBar';
import {useHasPageFrameFeature} from 'sentry/views/navigation/useHasPageFrameFeature';
import {makeTracesPathname} from 'sentry/views/traces/pathnames';

export default function MultiQueryMode() {
Expand All @@ -25,6 +27,7 @@ export default function MultiQueryMode() {

const id = getIdFromLocation(location);
const {data: savedQuery} = useGetSavedQuery(id);
const hasPageFrameFeature = useHasPageFrameFeature();

return (
<Feature
Expand Down Expand Up @@ -58,7 +61,13 @@ export default function MultiQueryMode() {
<Grid flow="column" align="center" gap="md">
<StarSavedQueryButton />
{defined(id) && savedQuery?.isPrebuilt === false && <SavedQueryEditMenu />}
<FeedbackButton />
{hasPageFrameFeature ? (
<TopBar.Slot name="feedback">
<FeedbackButton>{null}</FeedbackButton>
</TopBar.Slot>
) : (
<FeedbackButton />
)}
</Grid>
</Layout.HeaderActions>
</Layout.Header>
Expand Down
Loading
Loading