Skip to content

Commit

Permalink
Simplify identification of each aggregation element in aggregation bu…
Browse files Browse the repository at this point in the history 
…ilder. (#10387)

* Display background for each element section instead of each element.

* Display add section button as icon next to element headline

* Cleaning up previous changes

* Improving naming of add element section button title.

* Removing no longer needed "Add a sort" button.

* Removing no longer needed import.

* Renaming `addEmptySection` to `addEmptyElement`.

* Renaming `allowAddSection` to `allowAddEmptyElement`.

* Renaming `onAddElementSection` to `onAddEmptyElement`.

* Implementing proper theme colors for recent aggregation builder layout changes.

* Add gap between aggregation builder controls and visualization.

* Add border for element configuration form containers.

* Fixing style linter warnings.

* Fixing remaining style linter warnings.
  • Loading branch information
@linuspahl
linuspahl committed Apr 12, 2021
1 parent ce3579c commit 72eab6b
Show file tree
Hide file tree
Showing 12 changed files with 125 additions and 115 deletions.
10 changes: 6 additions & 4 deletions graylog2-web-interface/src/views/components/aggregationwizard/AggregationWizard.tsx
View file
Expand Up @@ -41,9 +41,10 @@ const Wrapper = styled.div`

const Controls = styled.div`
height: 100%;
min-width: 300px;
min-width: 315px;
max-width: 500px;
flex: 1;
flex: 1.2;
padding-right: 15px;
overflow-y: auto;
`;

Expand All @@ -60,7 +61,7 @@ const Section = styled.div`
}
`;

const _onElementCreate = (
const onAddEmptyElement = (
elementKey: string,
values: WidgetConfigFormValues,
setValues: (formValues: WidgetConfigFormValues) => void,
Expand Down Expand Up @@ -116,13 +117,14 @@ const AggregationWizard = ({ onChange, config, children }: EditWidgetComponentPr
{({ values, setValues }) => (
<>
<Section data-testid="add-element-section">
<AggregationElementSelect onElementCreate={(elementKey) => _onElementCreate(elementKey, values, setValues)}
<AggregationElementSelect onElementCreate={(elementKey) => onAddEmptyElement(elementKey, values, setValues)}
aggregationElements={aggregationElements}
formValues={values} />
</Section>
<Section data-testid="configure-elements-section">
<ElementsConfiguration aggregationElementsByKey={aggregationElementsByKey}
config={config}
onAddEmptyElement={onAddEmptyElement}
onConfigChange={onChange} />
</Section>
</>
Expand Down
20 changes: 9 additions & 11 deletions graylog2-web-interface/src/views/components/aggregationwizard/ElementsConfiguration.tsx
View file
Expand Up @@ -43,19 +43,16 @@ type Props = {
aggregationElementsByKey: { [elementKey: string]: AggregationElement }
config: AggregationWidgetConfig,
onConfigChange: (config: AggregationWidgetConfig) => void,
onAddEmptyElement: (
elementKey: string,
values: WidgetConfigFormValues,
setValues: (formValues: WidgetConfigFormValues) => void,
) => void,
}

const ElementsConfiguration = ({ aggregationElementsByKey, config, onConfigChange }: Props) => {
const ElementsConfiguration = ({ aggregationElementsByKey, config, onConfigChange, onAddEmptyElement }: Props) => {
const { values, setValues, dirty } = useFormikContext<WidgetConfigFormValues>();

const _onDeleteElement = (aggregationElement) => {
if (typeof aggregationElement.onDeleteAll !== 'function') {
return;
}

setValues(aggregationElement.onDeleteAll(values));
};

return (
<Container>
<div>
Expand All @@ -73,9 +70,10 @@ const ElementsConfiguration = ({ aggregationElementsByKey, config, onConfigChang
const AggregationElementComponent = aggregationElement.component;

return (
<ElementConfigurationContainer isPermanentElement={aggregationElement.onDeleteAll === undefined}
<ElementConfigurationContainer allowAddEmptyElement={aggregationElement.allowCreate(values)}
title={aggregationElement.title}
onDeleteAll={() => _onDeleteElement(aggregationElement)}
titleSingular={aggregationElement.titleSingular}
onAddEmptyElement={() => onAddEmptyElement(aggregationElement.key, values, setValues)}
key={aggregationElement.key}>
<AggregationElementComponent config={config} onConfigChange={onConfigChange} />
</ElementConfigurationContainer>
Expand Down
6 changes: 3 additions & 3 deletions ...og2-web-interface/src/views/components/aggregationwizard/ElementsConfigurationActions.tsx
View file
Expand Up @@ -27,12 +27,12 @@ import type { WidgetConfigFormValues } from './WidgetConfigForm';
const ConfigActions = styled.div<{ scrolledToBottom: boolean }>(({ theme, scrolledToBottom }) => css`
position: sticky;
width: 100%;
bottom: 0px;
bottom: 0;
padding-top: 5px;
background: ${theme.colors.global.contentBackground};
z-index: 1;
:before {
::before {
box-shadow: 1px -2px 3px rgb(0 0 0 / 25%);
content: ' ';
display: ${scrolledToBottom ? 'block' : 'none'};
Expand All @@ -47,7 +47,7 @@ const ConfigActions = styled.div<{ scrolledToBottom: boolean }>(({ theme, scroll
const ScrolledToBottomIndicator = styled.div`
width: 100%;
position: absolute;
bottom: 0px;
bottom: 0;
height: 5px;
z-index: 0;
`;
Expand Down
1 change: 1 addition & 0 deletions ...face/src/views/components/aggregationwizard/aggregationElements/AggregationElementType.ts
View file
Expand Up @@ -20,6 +20,7 @@ import type { WidgetConfigFormValues, WidgetConfigValidationErrors } from '../Wi

export type AggregationElement = {
title: string,
titleSingular?: string,
key: string,
allowCreate: (formValues: WidgetConfigFormValues) => boolean,
order: number,
Expand Down
1 change: 1 addition & 0 deletions ...eb-interface/src/views/components/aggregationwizard/aggregationElements/GroupByElement.ts
View file
Expand Up @@ -177,6 +177,7 @@ export const emptyGrouping: ValuesGrouping = {

const GroupByElement: AggregationElement = {
title: 'Group By',
titleSingular: 'Grouping',
key: 'groupBy',
order: 1,
allowCreate: () => true,
Expand Down
30 changes: 15 additions & 15 deletions .../components/aggregationwizard/elementConfiguration/ElementConfigurationContainer.test.tsx
View file
Expand Up @@ -22,8 +22,8 @@ import ElementConfigurationContainer from './ElementConfigurationContainer';
describe('ElementConfigurationContainer', () => {
it('should render elements passed as children', () => {
render(
<ElementConfigurationContainer isPermanentElement={false}
onDeleteAll={() => {}}
<ElementConfigurationContainer allowAddEmptyElement
onAddEmptyElement={() => {}}
title="Aggregation Element Title">
Children of Dune
</ElementConfigurationContainer>,
Expand All @@ -34,8 +34,8 @@ describe('ElementConfigurationContainer', () => {

it('should render title', () => {
render(
<ElementConfigurationContainer isPermanentElement={false}
onDeleteAll={() => {}}
<ElementConfigurationContainer allowAddEmptyElement
onAddEmptyElement={() => {}}
title="Aggregation Element Title">
Children of Dune
</ElementConfigurationContainer>,
Expand All @@ -44,33 +44,33 @@ describe('ElementConfigurationContainer', () => {
expect(screen.getByText('Aggregation Element Title')).toBeInTheDocument();
});

it('should call on delete when clicking delete icon', async () => {
const onDeleteAllMock = jest.fn();
it('should call on onAddEmptyElement when adding a section', async () => {
const onAddEmptyElementMock = jest.fn();

render(
<ElementConfigurationContainer isPermanentElement={false}
onDeleteAll={onDeleteAllMock}
<ElementConfigurationContainer allowAddEmptyElement
onAddEmptyElement={onAddEmptyElementMock}
title="Aggregation Element Title">
Children of Dune
</ElementConfigurationContainer>,
);

const deleteButton = screen.getByTitle('Remove Aggregation Element Title');
const addButton = screen.getByTitle('Add a Aggregation Element Title');

fireEvent.click(deleteButton);
fireEvent.click(addButton);

expect(onDeleteAllMock).toHaveBeenCalledTimes(1);
expect(onAddEmptyElementMock).toHaveBeenCalledTimes(1);
});

it('should not display delete icon if element is permanent', async () => {
it('should not display add section icon if adding element section is not allowed', async () => {
render(
<ElementConfigurationContainer isPermanentElement
onDeleteAll={() => {}}
<ElementConfigurationContainer allowAddEmptyElement={false}
onAddEmptyElement={() => {}}
title="Aggregation Element Title">
Children of Dune
</ElementConfigurationContainer>,
);

expect(screen.queryByTitle('Remove Aggregation Element Title')).not.toBeInTheDocument();
expect(screen.queryByTitle('Add a Aggregation Element Title')).not.toBeInTheDocument();
});
});
65 changes: 49 additions & 16 deletions ...views/components/aggregationwizard/elementConfiguration/ElementConfigurationContainer.tsx
View file
Expand Up @@ -20,11 +20,8 @@ import styled, { css } from 'styled-components';
import IconButton from 'components/common/IconButton';

const Wrapper = styled.div(({ theme }) => css`
background-color: ${theme.colors.variant.lightest.default};
border: 1px solid ${theme.colors.variant.lighter.default};
padding: 6px 6px 3px 6px;
border-radius: 6px;
margin-bottom: 6px;
border-radius: 6px;
:last-child {
margin-bottom: 0;
Expand Down Expand Up @@ -56,34 +53,66 @@ const Wrapper = styled.div(({ theme }) => css`
}
`);

const Header = styled.div`
const Header = styled.div(({ theme }) => css`
display: flex;
justify-content: space-between;
margin-bottom: 5px;
`;
align-items: center;
margin-bottom: 1px;
min-height: 26px;
font-weight: bold;
position: relative;
::before {
content: ' ';
top: 50%;
width: 100%;
border-bottom: 1px solid ${theme.utils.contrastingColor(theme.colors.global.contentBackground, 'AA')};
position: absolute;
}
`);

const ElementTitle = styled.div(({ theme }) => css`
background-color: ${theme.colors.global.contentBackground};
z-index: 1;
padding-right: 8px;
`);

const ElementActions = styled.div(({ theme }) => css`
background-color: ${theme.colors.global.contentBackground};
z-index: 1;
padding-left: 5px;
`);

const StyledIconButton = styled(IconButton)(({ theme }) => `
color: ${theme.colors.global.textDefault};
`);

type Props = {
allowAddEmptyElement: boolean,
children: React.ReactNode,
isPermanentElement: boolean,
onDeleteAll: () => void
onAddEmptyElement: () => void,
title: string,
titleSingular?: string,
}

const ElementConfigurationContainer = ({
children,
isPermanentElement,
onDeleteAll,
allowAddEmptyElement,
onAddEmptyElement,
title,
titleSingular,
}: Props) => {
return (
<Wrapper>
<Header>
<div>{title}</div>
<div>
{!isPermanentElement && (
<IconButton title={`Remove ${title}`} name="trash" onClick={onDeleteAll} />
<ElementTitle>
{title}
</ElementTitle>
<ElementActions>
{allowAddEmptyElement && (
<StyledIconButton title={`Add a ${titleSingular ?? title}`} name="plus" onClick={onAddEmptyElement} />
)}
</div>
</ElementActions>
</Header>
<div>
{children}
Expand All @@ -92,4 +121,8 @@ const ElementConfigurationContainer = ({
);
};

ElementConfigurationContainer.defaultProps = {
titleSingular: undefined,
};

export default ElementConfigurationContainer;
6 changes: 4 additions & 2 deletions ...c/views/components/aggregationwizard/elementConfiguration/ElementConfigurationSection.tsx
View file
Expand Up @@ -20,11 +20,13 @@ import styled, { css } from 'styled-components';
import { IconButton } from 'components/common';

const SectionContainer = styled.div(({ theme }) => css`
border-bottom: 1px solid ${theme.colors.variant.lighter.default};
background-color: ${theme.colors.variant.lightest.default};
margin-bottom: 5px;
padding: 6px 6px 3px 6px;
border-radius: 3px;
border: 1px solid ${theme.colors.variant.lighter.default};
:last-of-type {
border-bottom: 0;
margin-bottom: 0;
}
`);
Expand Down
48 changes: 17 additions & 31 deletions ...face/src/views/components/aggregationwizard/elementConfiguration/GroupByConfiguration.tsx
View file
Expand Up @@ -20,21 +20,14 @@ import { useFormikContext, FieldArray, Field } from 'formik';
import styled from 'styled-components';

import { HoverForHelp } from 'components/common';
import { Button, ButtonToolbar, Checkbox } from 'components/graylog';
import { Checkbox } from 'components/graylog';

import ElementConfigurationSection from './ElementConfigurationSection';
import GroupBy from './GroupBy';

import GroupByElement, { emptyGrouping } from '../aggregationElements/GroupByElement';
import GroupByElement from '../aggregationElements/GroupByElement';
import { WidgetConfigFormValues } from '../WidgetConfigForm';

const ActionsBar = styled.div`
display: flex;
justify-content: space-between;
align-items: center;
margin-bottom: 3px;
`;

const RollupColumnsLabel = styled.div`
display: flex;
align-items: center;
Expand All @@ -53,8 +46,22 @@ const GroupByConfiguration = () => {

return (
<>
<Field name="groupBy.columnRollup">
{({ field: { name, onChange, value } }) => (
<Checkbox onChange={() => onChange({ target: { name, value: !groupBy.columnRollup } })}
checked={value}
disabled={disableColumnRollup}>
<RollupColumnsLabel>
Rollup Columns
<RollupHoverForHelp title="Rollup Columns">
When rollup is enabled, an additional trace totalling individual subtraces will be included.
</RollupHoverForHelp>
</RollupColumnsLabel>
</Checkbox>
)}
</Field>
<FieldArray name="groupBy.groupings"
render={(arrayHelpers) => (
render={() => (
<>
<div>
{groupBy.groupings.map((grouping, index) => {
Expand All @@ -66,27 +73,6 @@ const GroupByConfiguration = () => {
);
})}
</div>
<ActionsBar>
<Field name="groupBy.columnRollup">
{({ field: { name, onChange, value } }) => (
<Checkbox onChange={() => onChange({ target: { name, value: !groupBy.columnRollup } })}
checked={value}
disabled={disableColumnRollup}>
<RollupColumnsLabel>
Rollup Columns
<RollupHoverForHelp title="Rollup Columns">
When rollup is enabled, an additional trace totalling individual subtraces will be included.
</RollupHoverForHelp>
</RollupColumnsLabel>
</Checkbox>
)}
</Field>
<ButtonToolbar>
<Button className="pull-right" bsSize="small" type="button" onClick={() => arrayHelpers.push(emptyGrouping)}>
Add Grouping
</Button>
</ButtonToolbar>
</ActionsBar>
</>
)} />
</>
Expand Down

0 comments on commit 72eab6b

Please sign in to comment.