Skip to content

Commit f206cf7

Browse files
ryan953ryan953
authored
ref(feedback): Refactor feedback layout so banners sit on top of the grid (#92282)
I saw a situation once where a banner appeared, and i dismissed it. Then afterwards the search bar was stacked on top of the page filters. It can be reproduced by going to a project where `showWhatsNewBanner` is true, then dismissing the banner... which means we're going to set `data-banner=true` and mess with the layout. Then since the banner is dismissed we put `null` into that position and the layout was broken. So this adds a flex div to wrap the grid layout. If there's a banner the flex will hold it without messing with the grid below. I didn't take screenshots, but the scrollable areas inside the feedback list and details both work as before when the content is too short, or too tall and needs scrolling. A good way to test that is with this project: https://demo.dev.getsentry.net:7999/issues/feedback?statsPeriod=30d.. try changing the inbox to get a scrolling list or not. And then select a message so that area scrolls too. **Before** <img width="1460" alt="SCR-20250526-nkym" src="https://github.com/user-attachments/assets/a52e1789-e2a6-4b9c-9075-ae86060663f3" /> **After** | Width | Img | | --- | --- | | Wide | <img width="1589" alt="SCR-20250526-ngyg" src="https://github.com/user-attachments/assets/d3b4fd59-86fa-4bfb-b9f9-b4e1fe2663ab" /> | With Sidebar | <img width="1437" alt="SCR-20250526-ngwq" src="https://github.com/user-attachments/assets/6e17b563-57ed-4457-ab84-59d799955bc0" /> | Narrow | <img width="729" alt="SCR-20250526-ngxo" src="https://github.com/user-attachments/assets/d50976ba-9282-40dd-89dd-52b79c606a8c" /> | SetupContainer | <img width="1153" alt="SCR-20250526-nohe" src="https://github.com/user-attachments/assets/9448be5f-1bbb-4708-852a-7cb7d7330e98" />
1 parent fa155e9 commit f206cf7

File tree

3 files changed

+47
-56
lines changed

3 files changed

+47
-56
lines changed

‎static/app/components/feedback/feedbackOnboarding/feedbackWidgetBanner.tsx

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
import type {CSSProperties} from 'react';
2-
31
import replaysDeadRageBackground from 'sentry-images/spot/replay-dead-rage-changelog.svg';
42

53
import PageBanner from 'sentry/components/alerts/pageBanner';
@@ -9,7 +7,7 @@ import {t} from 'sentry/locale';
97
import useDismissAlert from 'sentry/utils/useDismissAlert';
108
import useOrganization from 'sentry/utils/useOrganization';
119

12-
export default function FeedbackWidgetBanner({style}: {style?: CSSProperties}) {
10+
export default function FeedbackWidgetBanner() {
1311
const {activateSidebar} = useFeedbackOnboardingSidebarPanel();
1412
const organization = useOrganization();
1513

@@ -23,7 +21,6 @@ export default function FeedbackWidgetBanner({style}: {style?: CSSProperties}) {
2321

2422
return (
2523
<PageBanner
26-
style={style}
2724
button={
2825
<Button
2926
priority="primary"

‎static/app/components/feedback/feedbackWhatsNewBanner.tsx

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import type {CSSProperties} from 'react';
21
import {useEffect} from 'react';
32

43
import replaysDeadRageBackground from 'sentry-images/spot/replay-dead-rage-changelog.svg';
@@ -11,12 +10,7 @@ import {trackAnalytics} from 'sentry/utils/analytics';
1110
import useDismissAlert from 'sentry/utils/useDismissAlert';
1211
import useOrganization from 'sentry/utils/useOrganization';
1312

14-
interface Props {
15-
className?: string;
16-
style?: CSSProperties;
17-
}
18-
19-
export default function FeedbackWhatsNewBanner({className, style}: Props) {
13+
export default function FeedbackWhatsNewBanner() {
2014
const organization = useOrganization();
2115

2216
const {dismiss, isDismissed} = useDismissAlert({
@@ -37,8 +31,6 @@ export default function FeedbackWhatsNewBanner({className, style}: Props) {
3731

3832
return (
3933
<PageBanner
40-
className={className}
41-
style={style}
4234
button={
4335
<LinkButton
4436
external

‎static/app/views/feedback/feedbackListPage.tsx

Lines changed: 45 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -74,31 +74,33 @@ export default function FeedbackListPage() {
7474
</Layout.Header>
7575
<PageFiltersContainer>
7676
<ErrorBoundary>
77-
<LayoutGrid data-banner={showWhatsNewBanner}>
77+
<Background>
7878
{showWidgetBanner ? (
79-
<FeedbackWidgetBanner style={{gridArea: 'banner'}} />
79+
<FeedbackWidgetBanner />
8080
) : showWhatsNewBanner ? (
81-
<FeedbackWhatsNewBanner style={{gridArea: 'banner'}} />
81+
<FeedbackWhatsNewBanner />
8282
) : null}
83-
<FeedbackFilters style={{gridArea: 'filters'}} />
84-
{hasSetupOneFeedback || hasSlug ? (
85-
<Fragment>
86-
<Container style={{gridArea: 'list'}}>
87-
<FeedbackList />
88-
</Container>
89-
<SearchContainer>
90-
<FeedbackSearch />
91-
</SearchContainer>
92-
<Container style={{gridArea: 'details'}}>
93-
<FeedbackItemLoader />
94-
</Container>
95-
</Fragment>
96-
) : (
97-
<SetupContainer>
98-
<FeedbackSetupPanel />
99-
</SetupContainer>
100-
)}
101-
</LayoutGrid>
83+
<LayoutGrid>
84+
<FeedbackFilters style={{gridArea: 'filters'}} />
85+
{hasSetupOneFeedback || hasSlug ? (
86+
<Fragment>
87+
<Container style={{gridArea: 'list'}}>
88+
<FeedbackList />
89+
</Container>
90+
<SearchContainer>
91+
<FeedbackSearch />
92+
</SearchContainer>
93+
<Container style={{gridArea: 'details'}}>
94+
<FeedbackItemLoader />
95+
</Container>
96+
</Fragment>
97+
) : (
98+
<SetupContainer>
99+
<FeedbackSetupPanel />
100+
</SetupContainer>
101+
)}
102+
</LayoutGrid>
103+
</Background>
102104
</ErrorBoundary>
103105
</PageFiltersContainer>
104106
</FeedbackQueryKeys>
@@ -107,10 +109,29 @@ export default function FeedbackListPage() {
107109
);
108110
}
109111

110-
const LayoutGrid = styled('div')`
112+
const Background = styled('div')`
111113
background: ${p => p.theme.background};
112114
overflow: hidden;
115+
display: flex;
116+
flex-direction: column;
117+
align-items: stretch;
118+
gap: ${space(2)};
119+
120+
@media (max-width: ${p => p.theme.breakpoints.medium}) {
121+
padding: ${space(2)};
122+
}
123+
124+
@media (min-width: ${p => p.theme.breakpoints.medium}) {
125+
padding: ${space(2)};
126+
}
127+
128+
@media (min-width: ${p => p.theme.breakpoints.large}) {
129+
padding: ${space(2)} ${space(4)};
130+
}
131+
`;
113132

133+
const LayoutGrid = styled('div')`
134+
overflow: hidden;
114135
flex-grow: 1;
115136
116137
display: grid;
@@ -122,42 +143,23 @@ const LayoutGrid = styled('div')`
122143
'filters search'
123144
'list details';
124145
125-
&[data-banner='true'] {
126-
grid-template-rows: max-content max-content 1fr;
127-
grid-template-areas:
128-
'banner banner'
129-
'filters search'
130-
'list details';
131-
}
132-
133146
@media (max-width: ${p => p.theme.breakpoints.medium}) {
134-
padding: ${space(2)};
135147
grid-template-columns: 1fr;
136148
grid-template-areas:
137149
'filters'
138150
'search'
139151
'list'
140152
'details';
141-
142-
&[data-banner='true'] {
143-
grid-template-areas:
144-
'banner'
145-
'filters'
146-
'search'
147-
'list'
148-
'details';
149-
}
150153
}
151154
152155
@media (min-width: ${p => p.theme.breakpoints.medium}) {
153-
padding: ${space(2)};
154156
grid-template-columns: minmax(1fr, 195px) 1fr;
155157
}
156158
157159
@media (min-width: ${p => p.theme.breakpoints.large}) {
158-
padding: ${space(2)} ${space(4)} ${space(2)} ${space(4)};
159160
grid-template-columns: 390px 1fr;
160161
}
162+
161163
@media (min-width: ${p => p.theme.breakpoints.large}) {
162164
grid-template-columns: minmax(390px, 1fr) 2fr;
163165
}

0 commit comments

Comments
 (0)