From 72eab6bcc4b22cbaba52812923eef5cd778c040d Mon Sep 17 00:00:00 2001 From: Linus Pahl Date: Mon, 12 Apr 2021 11:03:09 +0200 Subject: [PATCH] Simplify identification of each aggregation element in aggregation builder. (#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. --- .../aggregationwizard/AggregationWizard.tsx | 10 +-- .../ElementsConfiguration.tsx | 20 +++--- .../ElementsConfigurationActions.tsx | 6 +- .../AggregationElementType.ts | 1 + .../aggregationElements/GroupByElement.ts | 1 + .../ElementConfigurationContainer.test.tsx | 30 ++++----- .../ElementConfigurationContainer.tsx | 65 ++++++++++++++----- .../ElementConfigurationSection.tsx | 6 +- .../GroupByConfiguration.tsx | 48 +++++--------- .../MetricsConfiguration.tsx | 39 +++++------ .../SortConfiguration.tsx | 8 +-- .../VisualizationConfiguration.tsx | 6 +- 12 files changed, 125 insertions(+), 115 deletions(-) diff --git a/graylog2-web-interface/src/views/components/aggregationwizard/AggregationWizard.tsx b/graylog2-web-interface/src/views/components/aggregationwizard/AggregationWizard.tsx index d91f3904216e..57a84da6e6ad 100644 --- a/graylog2-web-interface/src/views/components/aggregationwizard/AggregationWizard.tsx +++ b/graylog2-web-interface/src/views/components/aggregationwizard/AggregationWizard.tsx @@ -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; `; @@ -60,7 +61,7 @@ const Section = styled.div` } `; -const _onElementCreate = ( +const onAddEmptyElement = ( elementKey: string, values: WidgetConfigFormValues, setValues: (formValues: WidgetConfigFormValues) => void, @@ -116,13 +117,14 @@ const AggregationWizard = ({ onChange, config, children }: EditWidgetComponentPr {({ values, setValues }) => ( <>
- _onElementCreate(elementKey, values, setValues)} + onAddEmptyElement(elementKey, values, setValues)} aggregationElements={aggregationElements} formValues={values} />
diff --git a/graylog2-web-interface/src/views/components/aggregationwizard/ElementsConfiguration.tsx b/graylog2-web-interface/src/views/components/aggregationwizard/ElementsConfiguration.tsx index 5f0b38dc8e1f..a1904e314037 100644 --- a/graylog2-web-interface/src/views/components/aggregationwizard/ElementsConfiguration.tsx +++ b/graylog2-web-interface/src/views/components/aggregationwizard/ElementsConfiguration.tsx @@ -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(); - const _onDeleteElement = (aggregationElement) => { - if (typeof aggregationElement.onDeleteAll !== 'function') { - return; - } - - setValues(aggregationElement.onDeleteAll(values)); - }; - return (
@@ -73,9 +70,10 @@ const ElementsConfiguration = ({ aggregationElementsByKey, config, onConfigChang const AggregationElementComponent = aggregationElement.component; return ( - _onDeleteElement(aggregationElement)} + titleSingular={aggregationElement.titleSingular} + onAddEmptyElement={() => onAddEmptyElement(aggregationElement.key, values, setValues)} key={aggregationElement.key}> diff --git a/graylog2-web-interface/src/views/components/aggregationwizard/ElementsConfigurationActions.tsx b/graylog2-web-interface/src/views/components/aggregationwizard/ElementsConfigurationActions.tsx index 148fb49dce80..306639dca317 100644 --- a/graylog2-web-interface/src/views/components/aggregationwizard/ElementsConfigurationActions.tsx +++ b/graylog2-web-interface/src/views/components/aggregationwizard/ElementsConfigurationActions.tsx @@ -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'}; @@ -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; `; diff --git a/graylog2-web-interface/src/views/components/aggregationwizard/aggregationElements/AggregationElementType.ts b/graylog2-web-interface/src/views/components/aggregationwizard/aggregationElements/AggregationElementType.ts index c697d289d677..56b2f9433cb5 100644 --- a/graylog2-web-interface/src/views/components/aggregationwizard/aggregationElements/AggregationElementType.ts +++ b/graylog2-web-interface/src/views/components/aggregationwizard/aggregationElements/AggregationElementType.ts @@ -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, diff --git a/graylog2-web-interface/src/views/components/aggregationwizard/aggregationElements/GroupByElement.ts b/graylog2-web-interface/src/views/components/aggregationwizard/aggregationElements/GroupByElement.ts index a4d216017f4c..c48602d9398e 100644 --- a/graylog2-web-interface/src/views/components/aggregationwizard/aggregationElements/GroupByElement.ts +++ b/graylog2-web-interface/src/views/components/aggregationwizard/aggregationElements/GroupByElement.ts @@ -177,6 +177,7 @@ export const emptyGrouping: ValuesGrouping = { const GroupByElement: AggregationElement = { title: 'Group By', + titleSingular: 'Grouping', key: 'groupBy', order: 1, allowCreate: () => true, diff --git a/graylog2-web-interface/src/views/components/aggregationwizard/elementConfiguration/ElementConfigurationContainer.test.tsx b/graylog2-web-interface/src/views/components/aggregationwizard/elementConfiguration/ElementConfigurationContainer.test.tsx index 99418a0c02e3..56e78e123587 100644 --- a/graylog2-web-interface/src/views/components/aggregationwizard/elementConfiguration/ElementConfigurationContainer.test.tsx +++ b/graylog2-web-interface/src/views/components/aggregationwizard/elementConfiguration/ElementConfigurationContainer.test.tsx @@ -22,8 +22,8 @@ import ElementConfigurationContainer from './ElementConfigurationContainer'; describe('ElementConfigurationContainer', () => { it('should render elements passed as children', () => { render( - {}} + {}} title="Aggregation Element Title"> Children of Dune , @@ -34,8 +34,8 @@ describe('ElementConfigurationContainer', () => { it('should render title', () => { render( - {}} + {}} title="Aggregation Element Title"> Children of Dune , @@ -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( - Children of Dune , ); - 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( - {}} + {}} title="Aggregation Element Title"> Children of Dune , ); - expect(screen.queryByTitle('Remove Aggregation Element Title')).not.toBeInTheDocument(); + expect(screen.queryByTitle('Add a Aggregation Element Title')).not.toBeInTheDocument(); }); }); diff --git a/graylog2-web-interface/src/views/components/aggregationwizard/elementConfiguration/ElementConfigurationContainer.tsx b/graylog2-web-interface/src/views/components/aggregationwizard/elementConfiguration/ElementConfigurationContainer.tsx index 1c04245c3443..37c6babe01e0 100644 --- a/graylog2-web-interface/src/views/components/aggregationwizard/elementConfiguration/ElementConfigurationContainer.tsx +++ b/graylog2-web-interface/src/views/components/aggregationwizard/elementConfiguration/ElementConfigurationContainer.tsx @@ -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; @@ -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 (
-
{title}
-
- {!isPermanentElement && ( - + + {title} + + + {allowAddEmptyElement && ( + )} -
+
{children} @@ -92,4 +121,8 @@ const ElementConfigurationContainer = ({ ); }; +ElementConfigurationContainer.defaultProps = { + titleSingular: undefined, +}; + export default ElementConfigurationContainer; diff --git a/graylog2-web-interface/src/views/components/aggregationwizard/elementConfiguration/ElementConfigurationSection.tsx b/graylog2-web-interface/src/views/components/aggregationwizard/elementConfiguration/ElementConfigurationSection.tsx index c0b159fd8dc0..918b64917040 100644 --- a/graylog2-web-interface/src/views/components/aggregationwizard/elementConfiguration/ElementConfigurationSection.tsx +++ b/graylog2-web-interface/src/views/components/aggregationwizard/elementConfiguration/ElementConfigurationSection.tsx @@ -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; } `); diff --git a/graylog2-web-interface/src/views/components/aggregationwizard/elementConfiguration/GroupByConfiguration.tsx b/graylog2-web-interface/src/views/components/aggregationwizard/elementConfiguration/GroupByConfiguration.tsx index 9a8166ce5013..9879a097e03a 100644 --- a/graylog2-web-interface/src/views/components/aggregationwizard/elementConfiguration/GroupByConfiguration.tsx +++ b/graylog2-web-interface/src/views/components/aggregationwizard/elementConfiguration/GroupByConfiguration.tsx @@ -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; @@ -53,8 +46,22 @@ const GroupByConfiguration = () => { return ( <> + + {({ field: { name, onChange, value } }) => ( + onChange({ target: { name, value: !groupBy.columnRollup } })} + checked={value} + disabled={disableColumnRollup}> + + Rollup Columns + + When rollup is enabled, an additional trace totalling individual subtraces will be included. + + + + )} + ( + render={() => ( <>
{groupBy.groupings.map((grouping, index) => { @@ -66,27 +73,6 @@ const GroupByConfiguration = () => { ); })}
- - - {({ field: { name, onChange, value } }) => ( - onChange({ target: { name, value: !groupBy.columnRollup } })} - checked={value} - disabled={disableColumnRollup}> - - Rollup Columns - - When rollup is enabled, an additional trace totalling individual subtraces will be included. - - - - )} - - - - - )} /> diff --git a/graylog2-web-interface/src/views/components/aggregationwizard/elementConfiguration/MetricsConfiguration.tsx b/graylog2-web-interface/src/views/components/aggregationwizard/elementConfiguration/MetricsConfiguration.tsx index f35da1ea0929..35d94abde419 100644 --- a/graylog2-web-interface/src/views/components/aggregationwizard/elementConfiguration/MetricsConfiguration.tsx +++ b/graylog2-web-interface/src/views/components/aggregationwizard/elementConfiguration/MetricsConfiguration.tsx @@ -18,8 +18,6 @@ import * as React from 'react'; import { useCallback } from 'react'; import { useFormikContext, FieldArray } from 'formik'; -import { Button, ButtonToolbar } from 'components/graylog'; - import ElementConfigurationSection from './ElementConfigurationSection'; import Metric from './Metric'; @@ -33,28 +31,21 @@ const MetricsConfiguration = () => { }, [setValues, values]); return ( - <> - ( - <> -
- {metrics.map((metric, index) => { - return ( - // eslint-disable-next-line react/no-array-index-key - removeMetric(index)}> - - - ); - })} -
- - - - - )} /> - + ( + <> +
+ {metrics.map((metric, index) => { + return ( + // eslint-disable-next-line react/no-array-index-key + removeMetric(index)}> + + + ); + })} +
+ + )} /> ); }; diff --git a/graylog2-web-interface/src/views/components/aggregationwizard/elementConfiguration/SortConfiguration.tsx b/graylog2-web-interface/src/views/components/aggregationwizard/elementConfiguration/SortConfiguration.tsx index 43f071f69add..e86b9c472972 100644 --- a/graylog2-web-interface/src/views/components/aggregationwizard/elementConfiguration/SortConfiguration.tsx +++ b/graylog2-web-interface/src/views/components/aggregationwizard/elementConfiguration/SortConfiguration.tsx @@ -17,7 +17,6 @@ import * as React from 'react'; import { FieldArray, useFormikContext } from 'formik'; -import { Button, ButtonToolbar } from 'components/graylog'; import { WidgetConfigFormValues } from 'views/components/aggregationwizard/WidgetConfigForm'; import ElementConfigurationSection from 'views/components/aggregationwizard/elementConfiguration/ElementConfigurationSection'; @@ -29,7 +28,7 @@ const SortConfiguration = () => { return ( ( + render={() => ( <>
{sort.map((s, index) => ( @@ -39,11 +38,6 @@ const SortConfiguration = () => { ))}
- - - )} /> ); diff --git a/graylog2-web-interface/src/views/components/aggregationwizard/elementConfiguration/VisualizationConfiguration.tsx b/graylog2-web-interface/src/views/components/aggregationwizard/elementConfiguration/VisualizationConfiguration.tsx index 48b77eed3144..f4e1143d627b 100644 --- a/graylog2-web-interface/src/views/components/aggregationwizard/elementConfiguration/VisualizationConfiguration.tsx +++ b/graylog2-web-interface/src/views/components/aggregationwizard/elementConfiguration/VisualizationConfiguration.tsx @@ -27,6 +27,8 @@ import VisualizationConfigurationOptions from 'views/components/aggregationwizar import { WidgetConfigFormValues } from 'views/components/aggregationwizard/WidgetConfigForm'; import { TIMESTAMP_FIELD } from 'views/Constants'; +import ElementConfigurationSection from './ElementConfigurationSection'; + const isTimeline = (values: WidgetConfigFormValues) => { if (!values.groupBy?.groupings || values.groupBy.groupings.length === 0) { return false; @@ -62,7 +64,7 @@ const VisualizationConfiguration = () => { const isTimelineChart = isTimeline(values); return ( -
+ {({ field: { name, value }, meta: { error } }) => ( { )} -
+ ); };