Skip to content
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.

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

feat(explore): initial styling revisions #78135

Merged
merged 2 commits into from
Sep 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
Loading