Skip to content

Commit

Permalink
feat(explore): initial styling revisions (#78135)
Browse files Browse the repository at this point in the history
Gradually moving it towards the standard query UI (as agreed upon by
multiple teams)

**BEFORE:**
![Screenshot 2024-09-26 at 4 32
51 PM](https://github.com/user-attachments/assets/6772b39d-8b70-4143-88c2-371cb3c73b84)

**AFTER:**
![Screenshot 2024-09-26 at 4 32
59 PM](https://github.com/user-attachments/assets/3a9782fd-88eb-44cd-9747-ec251403775a)
  • Loading branch information
doralchan authored Sep 27, 2024
1 parent 876bc7c commit 5319e0d
Show file tree
Hide file tree
Showing 10 changed files with 98 additions and 112 deletions.
4 changes: 2 additions & 2 deletions static/app/views/explore/charts/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ export function ExploreCharts({query}: ExploreChartsProps) {
<ChartSettingsContainer>
<CompactSelect
size="xs"
triggerProps={{prefix: t('Display')}}
triggerProps={{prefix: t('Type')}}
value={chartType}
options={exploreChartTypeOptions}
onChange={option => handleChartTypeChange(option.value, index)}
Expand Down Expand Up @@ -173,7 +173,7 @@ const ChartContainer = styled('div')`
display: grid;
gap: 0;
grid-template-columns: 1fr;
margin-bottom: ${space(3)};
margin-bottom: ${space(2)};
`;

const ChartHeader = styled('div')`
Expand Down
60 changes: 21 additions & 39 deletions static/app/views/explore/content.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -65,35 +65,31 @@ function ExploreContentImpl({}: ExploreContentProps) {
<Layout.Page>
<Layout.Header>
<Layout.HeaderContent>
<Title>{t('Explore')}</Title>
<Layout.Title>{t('Explore')}</Layout.Title>
</Layout.HeaderContent>
<Layout.HeaderActions>
<ButtonBar gap={1}>
<Button onClick={switchToOldTraceExplorer}>
{t('Switch to old Trace Explore')}
{t('Switch to Old Trace Explore')}
</Button>
<FeedbackWidgetButton />
</ButtonBar>
</Layout.HeaderActions>
</Layout.Header>
<Body>
<FilterActions>
<PageFilterBar condensed>
<ProjectPageFilter />
<EnvironmentPageFilter />
<DatePageFilter />
</PageFilterBar>
<SpanSearchQueryBuilder
supportedAggregates={supportedAggregates}
projects={selection.projects}
initialQuery={userQuery}
onSearch={setUserQuery}
searchSource="explore"
/>
</FilterActions>
<Side>
<ExploreToolbar extras={toolbarExtras} />
</Side>
<PageFilterBar condensed>
<ProjectPageFilter />
<EnvironmentPageFilter />
<DatePageFilter />
</PageFilterBar>
<StyledSpanSearchQueryBuilder
supportedAggregates={supportedAggregates}
projects={selection.projects}
initialQuery={userQuery}
onSearch={setUserQuery}
searchSource="explore"
/>
<ExploreToolbar extras={toolbarExtras} />
<Main fullWidth>
<ExploreCharts query={userQuery} />
<ExploreTables />
Expand All @@ -113,34 +109,20 @@ export function ExploreContent(props: ExploreContentProps) {
);
}

const Title = styled(Layout.Title)`
margin-bottom: ${space(2)};
`;

const FilterActions = styled('div')`
grid-column: 1 / -1;
display: grid;
const Body = styled(Layout.Body)`
display: flex;
flex-direction: column;
gap: ${space(2)};
grid-template-columns: auto;
@media (min-width: ${p => p.theme.breakpoints.large}) {
grid-template-columns: auto 1fr;
grid-template-columns: 300px minmax(100px, auto);
}
`;

const Body = styled(Layout.Body)`
@media (min-width: ${p => p.theme.breakpoints.large}) {
display: grid;
grid-template-columns: 275px minmax(100px, auto);
align-content: start;
gap: ${p => (!p.noRowGap ? `${space(3)}` : `0 ${space(3)}`)};
}
const StyledSpanSearchQueryBuilder = styled(SpanSearchQueryBuilder)`
grid-column: 2/3;
`;

const Main = styled(Layout.Main)`
grid-column: 2/3;
`;

const Side = styled(Layout.Side)`
grid-column: 1/2;
`;
5 changes: 2 additions & 3 deletions static/app/views/explore/tables/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -68,12 +68,11 @@ function ExploreSamplesTable() {
</TabList>
</Tabs>
<Button
size="sm"
disabled={tab !== Tab.SPAN}
onClick={openColumnEditor}
icon={<IconTable />}
>
{t('Columns')}
{t('Edit Table')}
</Button>
</SamplesTableHeader>
{tab === Tab.SPAN && <SpansTable />}
Expand All @@ -86,5 +85,5 @@ const SamplesTableHeader = styled('div')`
display: flex;
flex-direction: row;
justify-content: space-between;
margin-bottom: ${space(1)};
margin-bottom: ${space(2)};
`;
26 changes: 13 additions & 13 deletions static/app/views/explore/toolbar/index.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ describe('ExploreToolbar', function () {

const section = screen.getByTestId('section-result-mode');
const samples = within(section).getByRole('radio', {name: 'Samples'});
const aggregates = within(section).getByRole('radio', {name: 'Aggregate'});
const aggregates = within(section).getByRole('radio', {name: 'Aggregates'});

expect(samples).toBeChecked();
expect(aggregates).not.toBeChecked();
Expand All @@ -89,7 +89,7 @@ describe('ExploreToolbar', function () {
await userEvent.click(within(groupBy).getByRole('button', {name: 'None'}));
await userEvent.click(within(groupBy).getByRole('option', {name: 'release'}));
expect(groupBys).toEqual(['release']);
await userEvent.click(within(groupBy).getByRole('button', {name: '+Add Group By'}));
await userEvent.click(within(groupBy).getByRole('button', {name: 'Add Group'}));
expect(groupBys).toEqual(['release', '']);

await userEvent.click(samples);
Expand Down Expand Up @@ -139,7 +139,7 @@ describe('ExploreToolbar', function () {
]);

// try adding an overlay
await userEvent.click(within(section).getByRole('button', {name: '+Add Overlay'}));
await userEvent.click(within(section).getByRole('button', {name: 'Add Overlay'}));
await userEvent.click(within(section).getByRole('button', {name: 'span.duration'}));
await userEvent.click(within(section).getByRole('option', {name: 'span.self_time'}));
expect(visualizes).toEqual([
Expand All @@ -150,7 +150,7 @@ describe('ExploreToolbar', function () {
]);

// try adding a new chart
await userEvent.click(within(section).getByRole('button', {name: '+Add Chart'}));
await userEvent.click(within(section).getByRole('button', {name: 'Add Chart'}));
expect(visualizes).toEqual([
{
yAxes: ['avg(span.self_time)', 'count(span.self_time)'],
Expand All @@ -160,20 +160,20 @@ describe('ExploreToolbar', function () {
]);

// delete first overlay
await userEvent.click(within(section).getAllByLabelText('Remove')[0]);
await userEvent.click(within(section).getAllByLabelText('Remove Overlay')[0]);
expect(visualizes).toEqual([
{yAxes: ['count(span.self_time)'], chartType: ChartType.LINE},
{yAxes: ['count(span.duration)'], chartType: ChartType.LINE},
]);

// delete second chart
await userEvent.click(within(section).getAllByLabelText('Remove')[1]);
await userEvent.click(within(section).getAllByLabelText('Remove Overlay')[1]);
expect(visualizes).toEqual([
{yAxes: ['count(span.self_time)'], chartType: ChartType.LINE},
]);

// only one left so cant be deleted
expect(within(section).getByLabelText('Remove')).toBeDisabled();
expect(within(section).getByLabelText('Remove Overlay')).toBeDisabled();
});

it('allows changing sort by', async function () {
Expand Down Expand Up @@ -249,7 +249,7 @@ describe('ExploreToolbar', function () {
// click the aggregates mode to enable
await userEvent.click(
within(screen.getByTestId('section-result-mode')).getByRole('radio', {
name: 'Aggregate',
name: 'Aggregates',
})
);

Expand All @@ -261,7 +261,7 @@ describe('ExploreToolbar', function () {
expect(within(section).getByRole('button', {name: 'span.op'})).toBeInTheDocument();
expect(groupBys).toEqual(['span.op']);

await userEvent.click(within(section).getByRole('button', {name: '+Add Group By'}));
await userEvent.click(within(section).getByRole('button', {name: 'Add Group'}));
expect(groupBys).toEqual(['span.op', '']);

await userEvent.click(within(section).getByRole('button', {name: 'None'}));
Expand All @@ -276,16 +276,16 @@ describe('ExploreToolbar', function () {
).toBeInTheDocument();
expect(groupBys).toEqual(['span.op', 'span.description']);

await userEvent.click(within(section).getAllByLabelText('Remove')[0]);
await userEvent.click(within(section).getAllByLabelText('Remove Group')[0]);
expect(groupBys).toEqual(['span.description']);

// only 1 left but it's not empty
expect(within(section).getByLabelText('Remove')).toBeEnabled();
expect(within(section).getByLabelText('Remove Group')).toBeEnabled();

await userEvent.click(within(section).getByLabelText('Remove'));
await userEvent.click(within(section).getByLabelText('Remove Group'));
expect(groupBys).toEqual(['']);

// last one and it's empty
expect(within(section).getByLabelText('Remove')).toBeDisabled();
expect(within(section).getByLabelText('Remove Group')).toBeDisabled();
});
});
30 changes: 19 additions & 11 deletions static/app/views/explore/toolbar/styles.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,37 +12,45 @@ export const ToolbarHeader = styled('div')`
flex-direction: row;
justify-content: space-between;
align-items: baseline;
margin-bottom: ${space(0.5)};
`;

export const ToolbarHeading = styled('h6')<{disabled?: boolean}>`
color: ${p => (p.disabled ? p.theme.gray300 : p.theme.purple300)};
export const ToolbarLabel = styled('h6')<{disabled?: boolean}>`
color: ${p => (p.disabled ? p.theme.gray300 : p.theme.gray500)};
height: ${p => p.theme.form.md.height};
min-height: ${p => p.theme.form.md.minHeight};
font-size: ${p => p.theme.form.md.fontSize};
line-height: ${p => p.theme.form.md.lineHeight};
text-decoration: underline dotted
${p => (p.disabled ? p.theme.gray300 : p.theme.purple300)};
margin: 0 0 ${space(1)} 0;
${p => (p.disabled ? p.theme.gray300 : p.theme.gray300)};
margin: 0;
`;

export const ToolbarHeaderButton = styled(Button)<{disabled?: boolean}>`
color: ${p => (p.disabled ? p.theme.gray300 : p.theme.purple300)};
color: ${p => (p.disabled ? p.theme.gray300 : p.theme.gray500)};
`;

export const ToolbarFooterButton = styled(Button)<{disabled?: boolean}>`
color: ${p => p.theme.gray300};
color: ${p => (p.disabled ? p.theme.gray300 : p.theme.linkColor)};
`;

export const ToolbarFooter = styled('div')<{disabled?: boolean}>`
margin-top: ${space(0.5)};
margin: ${space(0.5)} 0;
`;

export const ToolbarRow = styled('div')`
display: flex;
justify-content: space-between;
flex-direction: row;
gap: ${space(0.5)};
margin-bottom: ${space(0.5)};
div,
label,
span {
flex-grow: 1;
:not(:first-child) {
padding-top: ${space(1)};
> button {
width: 100%;
font-weight: normal;
}
}
`;
11 changes: 3 additions & 8 deletions static/app/views/explore/toolbar/toolbarDataset.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import {SegmentedControl} from 'sentry/components/segmentedControl';
import {t} from 'sentry/locale';
import {DiscoverDatasets} from 'sentry/utils/discover/types';

import {ToolbarHeader, ToolbarHeading, ToolbarSection} from './styles';
import {ToolbarHeader, ToolbarLabel, ToolbarSection} from './styles';

interface ToolbarDatasetProps {
dataset: DiscoverDatasets;
Expand All @@ -13,14 +13,9 @@ export function ToolbarDataset({dataset, setDataset}: ToolbarDatasetProps) {
return (
<ToolbarSection data-test-id="section-dataset">
<ToolbarHeader>
<ToolbarHeading>{t('Dataset')}</ToolbarHeading>
<ToolbarLabel>{t('Dataset')}</ToolbarLabel>
</ToolbarHeader>
<SegmentedControl
size="sm"
aria-label={t('Dataset')}
value={dataset}
onChange={setDataset}
>
<SegmentedControl aria-label={t('Dataset')} value={dataset} onChange={setDataset}>
<SegmentedControl.Item key={DiscoverDatasets.SPANS_EAP}>
{t('EAP Spans')}
</SegmentedControl.Item>
Expand Down
16 changes: 8 additions & 8 deletions static/app/views/explore/toolbar/toolbarGroupBy.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {useCallback, useMemo} from 'react';
import {Button} from 'sentry/components/button';
import type {SelectOption} from 'sentry/components/compactSelect';
import {CompactSelect} from 'sentry/components/compactSelect';
import {IconAdd} from 'sentry/icons/iconAdd';
import {IconDelete} from 'sentry/icons/iconDelete';
import {t} from 'sentry/locale';
import {useGroupBys} from 'sentry/views/explore/hooks/useGroupBys';
Expand All @@ -13,7 +14,7 @@ import {useSpanTags} from '../contexts/spanTagsContext';
import {
ToolbarHeader,
ToolbarHeaderButton,
ToolbarHeading,
ToolbarLabel,
ToolbarRow,
ToolbarSection,
} from './styles';
Expand Down Expand Up @@ -64,23 +65,22 @@ export function ToolbarGroupBy({disabled}: ToolbarGroupByProps) {
return (
<ToolbarSection data-test-id="section-group-by">
<ToolbarHeader>
<ToolbarHeading disabled={disabled}>{t('Group By')}</ToolbarHeading>
<ToolbarLabel disabled={disabled}>{t('Group By')}</ToolbarLabel>
<ToolbarHeaderButton
disabled={disabled}
size="xs"
size="zero"
onClick={addGroupBy}
borderless
>
{t('+Add Group By')}
</ToolbarHeaderButton>
aria-label={t('Add Group')}
icon={<IconAdd />}
/>
</ToolbarHeader>
<div>
{groupBys.map((groupBy, index) => (
<ToolbarRow key={index}>
<CompactSelect
searchable
disabled={disabled}
size="sm"
options={options}
value={groupBy}
onChange={newGroupBy => setGroupBy(index, newGroupBy)}
Expand All @@ -91,7 +91,7 @@ export function ToolbarGroupBy({disabled}: ToolbarGroupByProps) {
size="zero"
disabled={disabled || (groupBys.length <= 1 && groupBy === '')}
onClick={() => deleteGroupBy(index)}
aria-label={t('Remove')}
aria-label={t('Remove Group')}
/>
</ToolbarRow>
))}
Expand Down
Loading

0 comments on commit 5319e0d

Please sign in to comment.