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

Improve reusability of search sidebar components. #21314

Merged
merged 5 commits into from
Jan 10, 2025
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
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
Loading