Skip to content

Commit b556326

Browse files
Ahmed-LabsAhmed-Labs
authored
chore(dashboards): Address global filter UI feedback (#103902)
- Adds the dataset name to the menu title when selecting filter tags. - Fixes spacing issues in filterSelector and addFilter menu titles. - Truncates filter key and value in the global filter selector (using middle ellipsis truncation). Note: An easy way to test truncation is to add a spans global filter for `span.description` or `sentry.normalized_description`
1 parent d61d118 commit b556326

File tree

6 files changed

+82
-41
lines changed

6 files changed

+82
-41
lines changed

‎static/app/views/dashboards/globalFilter/addFilter.spec.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ describe('AddFilter', () => {
6262
await userEvent.click(screen.getByText('Errors'));
6363

6464
// Should see filter key options for the dataset
65-
expect(screen.getByText('Select Filter Tag')).toBeInTheDocument();
65+
expect(screen.getByText('Select Errors Tag')).toBeInTheDocument();
6666
expect(screen.getByText(mockFilterKeys['browser.name']!.key)).toBeInTheDocument();
6767
expect(screen.getByText(mockFilterKeys.environment!.key)).toBeInTheDocument();
6868

‎static/app/views/dashboards/globalFilter/addFilter.tsx‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import {ValueType} from 'sentry/components/searchQueryBuilder/tokens/filterKeyLi
1010
import {getInitialFilterText} from 'sentry/components/searchQueryBuilder/tokens/utils';
1111
import {IconAdd, IconArrow} from 'sentry/icons';
1212
import {t} from 'sentry/locale';
13-
import {space} from 'sentry/styles/space';
1413
import type {Tag} from 'sentry/types/group';
1514
import {
1615
FieldKind,
@@ -19,6 +18,7 @@ import {
1918
type FieldDefinition,
2019
} from 'sentry/utils/fields';
2120
import type {SearchBarData} from 'sentry/views/dashboards/datasetConfig/base';
21+
import {MenuTitleWrapper} from 'sentry/views/dashboards/globalFilter/filterSelector';
2222
import {getFieldDefinitionForDataset} from 'sentry/views/dashboards/globalFilter/utils';
2323
import {WidgetType, type GlobalFilter} from 'sentry/views/dashboards/types';
2424
import {shouldExcludeTracingKeys} from 'sentry/views/performance/utils';
@@ -178,7 +178,14 @@ function AddFilter({globalFilters, getSearchBarData, onAddFilter}: AddFilterProp
178178
size="md"
179179
menuWidth="300px"
180180
menuTitle={
181-
isSelectingFilterKey ? t('Select Filter Tag') : t('Select Filter Dataset')
181+
<MenuTitleWrapper>
182+
{isSelectingFilterKey
183+
? t(
184+
'Select %s Tag',
185+
selectedDataset ? getDatasetLabel(selectedDataset) : 'Filter'
186+
)
187+
: t('Select Filter Dataset')}
188+
</MenuTitleWrapper>
182189
}
183190
menuFooter={isSelectingFilterKey && filterOptionsMenuFooter}
184191
trigger={triggerProps => (
@@ -197,10 +204,10 @@ export default AddFilter;
197204
const FooterWrap = styled('div')`
198205
display: grid;
199206
grid-auto-flow: column;
200-
gap: ${space(2)};
207+
gap:${p => p.theme.space.xl}
201208
202209
/* If there's FooterMessage above */
203210
&:not(:first-child) {
204-
margin-top: ${space(1)};
211+
margin-top: ${p => p.theme.space.md};
205212
}
206213
`;

‎static/app/views/dashboards/globalFilter/filterSelector.spec.tsx‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ describe('FilterSelector', () => {
3535
/>
3636
);
3737

38-
const button = screen.getByRole('button', {name: mockGlobalFilter.tag.key + ':'});
38+
const button = screen.getByRole('button', {name: mockGlobalFilter.tag.key + ' :'});
3939
await userEvent.click(button);
4040

4141
expect(screen.getByText('chrome')).toBeInTheDocument();
@@ -53,7 +53,7 @@ describe('FilterSelector', () => {
5353
/>
5454
);
5555

56-
const button = screen.getByRole('button', {name: mockGlobalFilter.tag.key + ':'});
56+
const button = screen.getByRole('button', {name: mockGlobalFilter.tag.key + ' :'});
5757
await userEvent.click(button);
5858

5959
await userEvent.click(screen.getByRole('checkbox', {name: 'Select firefox'}));
@@ -84,7 +84,7 @@ describe('FilterSelector', () => {
8484
/>
8585
);
8686

87-
const button = screen.getByRole('button', {name: mockGlobalFilter.tag.key + ':'});
87+
const button = screen.getByRole('button', {name: mockGlobalFilter.tag.key + ' :'});
8888
await userEvent.click(button);
8989

9090
expect(screen.getByRole('checkbox', {name: 'Select firefox'})).toBeChecked();
@@ -101,7 +101,7 @@ describe('FilterSelector', () => {
101101
/>
102102
);
103103

104-
const button = screen.getByRole('button', {name: mockGlobalFilter.tag.key + ':'});
104+
const button = screen.getByRole('button', {name: mockGlobalFilter.tag.key + ' :'});
105105
await userEvent.click(button);
106106
await userEvent.click(screen.getByRole('button', {name: 'Remove Filter'}));
107107

‎static/app/views/dashboards/globalFilter/filterSelector.tsx‎

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {MutableSearch} from 'sentry/components/searchSyntax/mutableSearch';
1515
import {t} from 'sentry/locale';
1616
import {space} from 'sentry/styles/space';
1717
import {keepPreviousData, useQuery} from 'sentry/utils/queryClient';
18+
import {middleEllipsis} from 'sentry/utils/string/middleEllipsis';
1819
import {useDebouncedValue} from 'sentry/utils/useDebouncedValue';
1920
import usePageFilters from 'sentry/utils/usePageFilters';
2021
import {type SearchBarData} from 'sentry/views/dashboards/datasetConfig/base';
@@ -124,7 +125,10 @@ function FilterSelector({
124125
const optionMap = new Map<string, SelectOption<string>>();
125126
const fixedOptionMap = new Map<string, SelectOption<string>>();
126127
const addOption = (value: string, map: Map<string, SelectOption<string>>) =>
127-
map.set(value, {label: value, value});
128+
map.set(value, {
129+
label: middleEllipsis(value, 70, /[\s-_:]/),
130+
value,
131+
});
128132

129133
// Filter values in the global filter
130134
activeFilterValues.forEach(value => addOption(value, optionMap));
@@ -180,7 +184,7 @@ function FilterSelector({
180184
};
181185

182186
const renderMenuHeaderTrailingItems = ({closeOverlay}: any) => (
183-
<Flex gap="md">
187+
<Flex gap="lg">
184188
{activeFilterValues.length > 0 && (
185189
<StyledButton
186190
aria-label={t('Clear Selections')}
@@ -228,7 +232,9 @@ function FilterSelector({
228232
onClose={() => {
229233
setStagedFilterValues([]);
230234
}}
231-
menuTitle={t('%s Filter', getDatasetLabel(dataset))}
235+
menuTitle={
236+
<MenuTitleWrapper>{t('%s Filter', getDatasetLabel(dataset))}</MenuTitleWrapper>
237+
}
232238
menuHeaderTrailingItems={renderMenuHeaderTrailingItems}
233239
triggerProps={{
234240
children: renderFilterSelectorTrigger(),
@@ -252,7 +258,6 @@ function FilterSelector({
252258
setStagedFilterValues(value);
253259
}}
254260
sizeLimit={30}
255-
maxMenuWidth={500}
256261
onClose={() => {
257262
setSearchQuery('');
258263
setStagedFilterValues([]);
@@ -261,7 +266,9 @@ function FilterSelector({
261266
emptyMessage={
262267
isFetching ? t('Loading filter values...') : t('No filter values found')
263268
}
264-
menuTitle={t('%s Filter', getDatasetLabel(dataset))}
269+
menuTitle={
270+
<MenuTitleWrapper>{t('%s Filter', getDatasetLabel(dataset))}</MenuTitleWrapper>
271+
}
265272
menuHeaderTrailingItems={renderMenuHeaderTrailingItems}
266273
triggerProps={{
267274
children: renderFilterSelectorTrigger(),
@@ -279,6 +286,12 @@ const StyledButton = styled(Button)`
279286
padding: 0 ${space(0.5)};
280287
margin: ${p =>
281288
p.theme.isChonk
282-
? `-${space(0.5)} -${space(0.5)}`
283-
: `-${space(0.25)} -${space(0.25)}`};
289+
? `-${p.theme.space.xs} -${p.theme.space.xs}`
290+
: `-${p.theme.space['2xs']} -${p.theme.space['2xs']}`};
291+
`;
292+
293+
export const MenuTitleWrapper = styled('span')`
294+
display: inline-block;
295+
padding-top: ${p => p.theme.space.xs};
296+
padding-bottom: ${p => p.theme.space.xs};
284297
`;

‎static/app/views/dashboards/globalFilter/filterSelectorTrigger.tsx‎

Lines changed: 28 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
import styled from '@emotion/styled';
22

3+
import {Flex} from '@sentry/scraps/layout';
4+
35
import {Badge} from 'sentry/components/core/badge';
46
import type {SelectOption} from 'sentry/components/core/compactSelect/types';
57
import LoadingIndicator from 'sentry/components/loadingIndicator';
6-
import TextOverflow from 'sentry/components/textOverflow';
78
import {t} from 'sentry/locale';
89
import {space} from 'sentry/styles/space';
910
import {prettifyTagKey} from 'sentry/utils/fields';
@@ -33,16 +34,23 @@ function FilterSelectorTrigger({
3334
const isAllSelected =
3435
activeFilterValues.length === 0 || activeFilterValues.length === options.length;
3536

37+
const tagKey = prettifyTagKey(tag.key);
38+
const filterValue = activeFilterValues[0] ?? '';
39+
const separator = <FilterValueSeparator>{':'}</FilterValueSeparator>;
40+
3641
return (
3742
<ButtonLabelWrapper>
38-
<TextOverflow>
39-
{prettifyTagKey(tag.key)}:{' '}
40-
{!isFetching && (
41-
<span style={{fontWeight: 'normal'}}>
42-
{isAllSelected ? t('All') : activeFilterValues[0]}
43-
</span>
44-
)}
45-
</TextOverflow>
43+
<FilterValueTruncated>{tagKey}</FilterValueTruncated>
44+
{separator}
45+
{!isFetching && (
46+
<span style={{fontWeight: 'normal'}}>
47+
{isAllSelected ? (
48+
t('All')
49+
) : (
50+
<FilterValueTruncated>{filterValue}</FilterValueTruncated>
51+
)}
52+
</span>
53+
)}
4654
{isFetching && <StyledLoadingIndicator size={14} />}
4755
{shouldShowBadge && (
4856
<StyledBadge type="default">{`+${activeFilterValues.length - 1}`}</StyledBadge>
@@ -70,11 +78,16 @@ const StyledBadge = styled(Badge)`
7078
padding: 0 ${space(0.5)};
7179
`;
7280

73-
const ButtonLabelWrapper = styled('span')`
74-
width: 100%;
75-
text-align: left;
81+
const ButtonLabelWrapper = styled(Flex)`
7682
align-items: center;
77-
display: inline-grid;
78-
grid-template-columns: 1fr auto;
79-
line-height: 1;
83+
`;
84+
85+
const FilterValueSeparator = styled('span')`
86+
margin-right: ${space(0.5)};
87+
`;
88+
89+
const FilterValueTruncated = styled('div')`
90+
${p => p.theme.overflowEllipsis};
91+
max-width: 300px;
92+
width: min-content;
8093
`;

‎static/app/views/dashboards/globalFilter/numericFilterSelector.tsx‎

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@ import {
1616
type TokenResult,
1717
} from 'sentry/components/searchSyntax/parser';
1818
import {t} from 'sentry/locale';
19-
import {space} from 'sentry/styles/space';
2019
import {getDatasetLabel} from 'sentry/views/dashboards/globalFilter/addFilter';
20+
import {MenuTitleWrapper} from 'sentry/views/dashboards/globalFilter/filterSelector';
2121
import type {GenericFilterSelectorProps} from 'sentry/views/dashboards/globalFilter/genericFilterSelector';
2222
import {
2323
BetweenFilterSelectorTrigger,
@@ -94,7 +94,7 @@ function useNativeOperatorFilter(
9494

9595
const renderInputField = () => {
9696
return (
97-
<Input
97+
<StyledInput
9898
aria-label="Filter value"
9999
value={stagedFilterValue}
100100
onChange={e => {
@@ -287,7 +287,11 @@ function NumericFilterSelector({
287287
filter.resetValues();
288288
setStagedIsNativeOperator(isNativeOperator);
289289
}}
290-
menuTitle={t('%s Filter', getDatasetLabel(globalFilter.dataset))}
290+
menuTitle={
291+
<MenuTitleWrapper>
292+
{t('%s Filter', getDatasetLabel(globalFilter.dataset))}
293+
</MenuTitleWrapper>
294+
}
291295
menuHeaderTrailingItems={() => (
292296
<StyledButton
293297
aria-label={t('Remove Filter')}
@@ -350,24 +354,24 @@ function NumericFilterSelector({
350354
export default NumericFilterSelector;
351355

352356
const MenuBodyWrap = styled('div')`
353-
margin: 4px;
357+
padding: ${p => p.theme.space.md};
354358
`;
355359

356360
const FooterWrap = styled('div')`
357361
display: grid;
358362
grid-auto-flow: column;
359-
gap: ${space(2)};
363+
gap: ${p => p.theme.space.xl};
360364
361365
/* If there's FooterMessage above */
362366
&:not(:first-child) {
363-
margin-top: ${space(1)};
367+
margin-top: ${p => p.theme.space.md};
364368
}
365369
`;
366370
const FooterInnerWrap = styled('div')`
367371
grid-row: -1;
368372
display: grid;
369373
grid-auto-flow: column;
370-
gap: ${space(1)};
374+
gap: ${p => p.theme.space.md};
371375
justify-self: end;
372376
justify-items: end;
373377
@@ -385,9 +389,13 @@ const StyledButton = styled(Button)`
385389
font-size: inherit;
386390
font-weight: ${p => p.theme.fontWeight.normal};
387391
color: ${p => p.theme.subText};
388-
padding: 0 ${space(0.5)};
392+
padding: 0 ${p => p.theme.space.xs};
389393
margin: ${p =>
390394
p.theme.isChonk
391-
? `-${space(0.5)} -${space(0.5)}`
392-
: `-${space(0.25)} -${space(0.25)}`};
395+
? `-${p.theme.space.xs} -${p.theme.space.xs}`
396+
: `-${p.theme.space['2xs']} -${p.theme.space['2xs']}`};
397+
`;
398+
399+
const StyledInput = styled(Input)`
400+
text-align: center;
393401
`;

0 commit comments

Comments
 (0)