Skip to content

Commit

Permalink
Improve reusability of search sidebar components. (#21314)
Browse files Browse the repository at this point in the history
* Improve reusability os search sidebar component.

* Improve reusability of highlighting rules components.

* Update tests

* Cleanup
  • Loading branch information
linuspahl authored Jan 10, 2025
1 parent d625082 commit cd07b1e
Show file tree
Hide file tree
Showing 14 changed files with 255 additions and 168 deletions.
6 changes: 4 additions & 2 deletions graylog2-web-interface/src/views/components/Search.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ import { selectCurrentQueryResults } from 'views/logic/slices/viewSelectors';
import useAppSelector from 'stores/useAppSelector';
import useParameters from 'views/hooks/useParameters';
import useSearchConfiguration from 'hooks/useSearchConfiguration';
import useViewTitle from 'views/hooks/useViewTitle';

import ExternalValueActionsProvider from './ExternalValueActionsProvider';

Expand Down Expand Up @@ -84,10 +85,11 @@ const SearchArea = styled(PageContentLayout)(() => {
`;
});

const ConnectedSidebar = (props: Omit<React.ComponentProps<typeof Sidebar>, 'results'>) => {
const ConnectedSidebar = (props: Omit<React.ComponentProps<typeof Sidebar>, 'results' | 'title'>) => {
const results = useAppSelector(selectCurrentQueryResults);
const title = useViewTitle();

return <Sidebar results={results} {...props} />;
return <Sidebar results={results} title={title} {...props} />;
};

const ViewAdditionalContextProvider = ({ children }: { children: React.ReactNode }) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,15 @@ import styled, { css } from 'styled-components';

import type { SearchPreferencesLayout } from 'views/components/contexts/SearchPagePreferencesContext';
import { IconButton } from 'components/common';
import useViewTitle from 'views/hooks/useViewTitle';

type Props = {
children: React.ReactNode,
closeSidebar: () => void,
enableSidebarPinning: boolean,
forceSideBarPinned: boolean,
searchPageLayout: SearchPreferencesLayout | undefined | null,
sectionTitle: string,
forceSideBarPinned: boolean,
title: string,
};

export const Container = styled.div<{ $sidebarIsPinned: boolean }>(({ theme, $sidebarIsPinned }) => css`
Expand Down Expand Up @@ -75,7 +76,7 @@ const Header = styled.div`
grid-row: 1;
`;

const SearchTitle = styled.div`
const TitleSection = styled.div`
height: 35px;
display: grid;
grid-template-columns: 1fr auto;
Expand Down Expand Up @@ -127,7 +128,7 @@ const SectionContent = styled.div`
}
`;

const toggleSidebarPinning = (searchPageLayout) => {
const toggleSidebarPinning = (searchPageLayout: SearchPreferencesLayout) => {
if (!searchPageLayout) {
return;
}
Expand All @@ -137,28 +138,27 @@ const toggleSidebarPinning = (searchPageLayout) => {
togglePinning();
};

const ContentColumn = ({ children, sectionTitle, closeSidebar, searchPageLayout, forceSideBarPinned }: Props) => {
const ContentColumn = ({ children, title, sectionTitle, closeSidebar, searchPageLayout, forceSideBarPinned, enableSidebarPinning }: Props) => {
const sidebarIsPinned = searchPageLayout?.config.sidebar.isPinned || forceSideBarPinned;
const title = useViewTitle();

return (
<Container $sidebarIsPinned={sidebarIsPinned}>
<ContentGrid>
<Header>
<SearchTitle title={title}>
<TitleSection title={title}>
<CenterVertical>
<Title onClick={closeSidebar}>{title}</Title>
</CenterVertical>
{!forceSideBarPinned && (
<CenterVertical>
<OverlayToggle $sidebarIsPinned={sidebarIsPinned}>
<IconButton onClick={() => toggleSidebarPinning(searchPageLayout)}
title={`Display sidebar ${sidebarIsPinned ? 'as overlay' : 'inline'}`}
name="keep" />
</OverlayToggle>
</CenterVertical>
{!forceSideBarPinned && enableSidebarPinning && (
<CenterVertical>
<OverlayToggle $sidebarIsPinned={sidebarIsPinned}>
<IconButton onClick={() => toggleSidebarPinning(searchPageLayout)}
title={`Display sidebar ${sidebarIsPinned ? 'as overlay' : 'inline'}`}
name="keep" />
</OverlayToggle>
</CenterVertical>
)}
</SearchTitle>
</TitleSection>
<HorizontalRule />
<SectionTitle>{sectionTitle}</SectionTitle>
</Header>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ describe('<Sidebar />', () => {

const renderSidebar = () => render(
<TestStoreProvider>
<Sidebar results={queryResult}>
<Sidebar results={queryResult} title="Sidebar Title">
<TestComponent />
</Sidebar>
</TestStoreProvider>,
Expand Down Expand Up @@ -227,7 +227,7 @@ describe('<Sidebar />', () => {

await screen.findByText('Execution');

fireEvent.click(await screen.findByText('Query Title'));
fireEvent.click(await screen.findByText('Sidebar Title'));

expect(screen.queryByText('Execution')).toBeNull();
});
Expand Down
21 changes: 13 additions & 8 deletions graylog2-web-interface/src/views/components/sidebar/Sidebar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import styled, { css } from 'styled-components';
import type QueryResult from 'views/logic/QueryResult';
import type { SearchPreferencesLayout } from 'views/components/contexts/SearchPagePreferencesContext';
import SearchPagePreferencesContext from 'views/components/contexts/SearchPagePreferencesContext';
import useActiveQueryId from 'views/hooks/useActiveQueryId';
import useSendTelemetry from 'logic/telemetry/useSendTelemetry';
import { TELEMETRY_EVENT_TYPE } from 'logic/telemetry/Constants';
import { getPathnameWithoutId } from 'util/URLUtils';
Expand All @@ -36,12 +35,14 @@ import type { SidebarAction } from './sidebarActions';
import sidebarActions from './sidebarActions';

type Props = {
children: React.ReactElement,
actions?: Array<SidebarAction>,
children?: React.ReactElement,
enableSidebarPinning?: boolean,
forceSideBarPinned?: boolean,
results?: QueryResult
searchPageLayout?: SearchPreferencesLayout,
sections?: Array<SidebarSection>,
actions?: Array<SidebarAction>,
forceSideBarPinned?: boolean,
title: string,
};

const Container = styled.div`
Expand Down Expand Up @@ -77,10 +78,13 @@ const _selectSidebarSection = (sectionKey, activeSectionKey, setActiveSectionKey
setActiveSectionKey(sectionKey);
};

const Sidebar = ({ searchPageLayout, results, children, sections = sidebarSections, actions = sidebarActions, forceSideBarPinned = false }: Props) => {
const Sidebar = ({
searchPageLayout = undefined, results = undefined, children = undefined, title,
sections = sidebarSections, actions = sidebarActions, forceSideBarPinned = false,
enableSidebarPinning = true,
}: Props) => {
const sendTelemetry = useSendTelemetry();
const location = useLocation();
const queryId = useActiveQueryId();
const sidebarIsPinned = searchPageLayout?.config.sidebar.isPinned || forceSideBarPinned;
const initialSectionKey = sections[0].key;
const [activeSectionKey, setActiveSectionKey] = useState<string | undefined>(searchPageLayout?.config.sidebar.isPinned ? initialSectionKey : null);
Expand Down Expand Up @@ -109,11 +113,12 @@ const Sidebar = ({ searchPageLayout, results, children, sections = sidebarSectio
actions={actions} />
{activeSection && !!SectionContent && (
<ContentColumn closeSidebar={toggleSidebar}
title={title}
enableSidebarPinning={enableSidebarPinning}
searchPageLayout={searchPageLayout}
sectionTitle={activeSection.title}
forceSideBarPinned={forceSideBarPinned}>
<SectionContent results={results}
queryId={queryId}
sidebarChildren={children}
sidebarIsPinned={sidebarIsPinned}
toggleSidebar={toggleSidebar} />
Expand All @@ -126,7 +131,7 @@ const Sidebar = ({ searchPageLayout, results, children, sections = sidebarSectio
);
};

const SidebarWithContext = ({ children, ...props }: React.ComponentProps<typeof Sidebar>) => (
const SidebarWithContext = ({ children = undefined, ...props }: React.ComponentProps<typeof Sidebar>) => (
<SearchPagePreferencesContext.Consumer>
{(searchPageLayout) => <Sidebar {...props} searchPageLayout={searchPageLayout}>{children}</Sidebar>}
</SearchPagePreferencesContext.Consumer>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,12 +120,16 @@ const SidebarNavigation = ({ sections, activeSection, selectSidebarSection, side
);
})}
</Section>
<HorizontalRuleWrapper><hr /></HorizontalRuleWrapper>
<Section>
{actions.map(({ key, Component }) => (
<Component key={key} sidebarIsPinned={sidebarIsPinned} />
))}
</Section>
{actions?.length > 0 && (
<>
<HorizontalRuleWrapper><hr /></HorizontalRuleWrapper>
<Section>
{actions.map(({ key, Component }) => (
<Component key={key} sidebarIsPinned={sidebarIsPinned} />
))}
</Section>
</>
)}
</Container>
);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,6 @@ import FieldTypeMapping from 'views/logic/fieldtypes/FieldTypeMapping';
import FieldType, { Properties } from 'views/logic/fieldtypes/FieldType';
import TestStoreProvider from 'views/test/TestStoreProvider';
import useViewsPlugin from 'views/test/testViewsPlugin';
import { updateHighlightingRule } from 'views/logic/slices/highlightActions';

jest.mock('views/logic/slices/highlightActions', () => ({
addHighlightingRule: jest.fn(() => () => Promise.resolve()),
updateHighlightingRule: jest.fn(() => () => Promise.resolve()),
}));

const rule = HighlightingRule.builder()
.color(StaticColor.create('#333333'))
Expand All @@ -54,10 +48,10 @@ describe('HighlightForm', () => {
all: Immutable.List([FieldTypeMapping.create('foob', FieldType.create('long', [Properties.Numeric]))]),
queryFields: Immutable.Map(),
};
const HighlightFormWithContext = (props: React.ComponentProps<typeof HighlightForm>) => (
const SUT = (props: Partial<React.ComponentProps<typeof HighlightForm>>) => (
<TestStoreProvider>
<FieldTypesContext.Provider value={fieldTypes}>
<HighlightForm {...props} />
<HighlightForm onClose={() => {}} rule={undefined} onSubmit={() => Promise.resolve()} {...props} />
</FieldTypesContext.Provider>
</TestStoreProvider>
);
Expand All @@ -70,7 +64,7 @@ describe('HighlightForm', () => {
useViewsPlugin();

it('should render for edit', async () => {
const { findByText } = render(<HighlightFormWithContext onClose={() => {}} rule={rule} />);
const { findByText } = render(<SUT rule={rule} />);

const form = await findByText('Edit Highlighting Rule');
const input = await screen.findByLabelText('Value');
Expand All @@ -80,15 +74,14 @@ describe('HighlightForm', () => {
});

it('should render for new', async () => {
const { findByText } = render(<HighlightFormWithContext onClose={() => {}} />);
const { findByText } = render(<SUT rule={undefined} />);

await findByText('Create Highlighting Rule');
});

it('should fire onClose on cancel', async () => {
const onClose = jest.fn();
const { findByText } = render(<HighlightFormWithContext onClose={onClose} />);

const { findByText } = render(<SUT onClose={onClose} />);
const elem = await findByText('Cancel');

fireEvent.click(elem);
Expand All @@ -97,29 +90,28 @@ describe('HighlightForm', () => {
});

it('should fire update action when saving a existing rule', async () => {
render(<HighlightFormWithContext onClose={() => {}} rule={rule} />);
const onSubmit = jest.fn(() => Promise.resolve());
render(<SUT rule={rule} onSubmit={onSubmit} />);

await triggerSaveButtonClick();

await waitFor(() => expect(updateHighlightingRule)
.toHaveBeenCalledWith(rule, { field: rule.field, value: rule.value, condition: rule.condition, color: rule.color }));
await waitFor(() => expect(onSubmit).toHaveBeenCalledWith(rule.field, rule.value, rule.condition, rule.color));
});

it('assigns a new static color when type is selected', async () => {
render(<HighlightFormWithContext onClose={() => {}} rule={rule} />);
const onSubmit = jest.fn(() => Promise.resolve());
render(<SUT rule={rule} onSubmit={onSubmit} />);

userEvent.click(screen.getByLabelText('Static Color'));

await triggerSaveButtonClick();

await waitFor(() => expect(updateHighlightingRule)
.toHaveBeenCalledWith(rule, expect.objectContaining({
color: expect.objectContaining({ type: 'static', color: expect.any(String) }),
})));
await waitFor(() => expect(onSubmit).toHaveBeenCalledWith(rule.field, rule.value, rule.condition, expect.objectContaining({ type: 'static', color: expect.any(String) })));
});

it('creates a new gradient when type is selected', async () => {
render(<HighlightFormWithContext onClose={() => {}} rule={rule} />);
const onSubmit = jest.fn(() => Promise.resolve());
render(<SUT rule={rule} onSubmit={onSubmit} />);

userEvent.click(screen.getByLabelText('Gradient'));

Expand All @@ -129,27 +121,23 @@ describe('HighlightForm', () => {

await triggerSaveButtonClick();

await waitFor(() => expect(updateHighlightingRule)
.toHaveBeenCalledWith(rule, expect.objectContaining({
color: expect.objectContaining({ gradient: 'Viridis' }),
})));
await waitFor(() => expect(onSubmit).toHaveBeenCalledWith(rule.field, rule.value, rule.condition, expect.objectContaining({ gradient: 'Viridis' })));
});

it('should be able to click submit when has value 0 with type number', async () => {
render(<HighlightFormWithContext onClose={() => {}} rule={ruleWithValueZero} />);
const onSubmit = jest.fn(() => Promise.resolve());
render(<SUT rule={ruleWithValueZero} onSubmit={onSubmit} />);

await triggerSaveButtonClick();

await waitFor(() => expect(updateHighlightingRule)
.toHaveBeenCalledWith(ruleWithValueZero, { field: ruleWithValueZero.field, value: '0', condition: ruleWithValueZero.condition, color: ruleWithValueZero.color }));
await waitFor(() => expect(onSubmit).toHaveBeenCalledWith(ruleWithValueZero.field, '0', ruleWithValueZero.condition, ruleWithValueZero.color));
});

it('should be able to click submit when has value false with type boolean', async () => {
render(<HighlightFormWithContext onClose={() => {}} rule={ruleWithValueFalse} />);
it('should be able to click submit when has value false with type boolean', async () => {
const onSubmit = jest.fn(() => Promise.resolve());
render(<SUT rule={ruleWithValueFalse} onSubmit={onSubmit} />);

await triggerSaveButtonClick();

await waitFor(() => expect(updateHighlightingRule)
.toHaveBeenCalledWith(ruleWithValueFalse, { field: ruleWithValueFalse.field, value: 'false', condition: ruleWithValueFalse.condition, color: ruleWithValueFalse.color }));
await waitFor(() => expect(onSubmit).toHaveBeenCalledWith(ruleWithValueFalse.field, 'false', ruleWithValueFalse.condition, ruleWithValueFalse.color));
});
});
Loading

0 comments on commit cd07b1e

Please sign in to comment.