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: scroll to bottom when adding a new native filter and the page is filled #19053

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
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ const DragIcon = styled(Icons.Drag, {
interface FilterTabTitleProps {
index: number;
filterIds: string[];
onRearrage: (dragItemIndex: number, targetIndex: number) => void;
onRearrange: (dragItemIndex: number, targetIndex: number) => void;
}

interface DragItem {
Expand All @@ -68,7 +68,7 @@ interface DragItem {

export const DraggableFilter: React.FC<FilterTabTitleProps> = ({
index,
onRearrage,
onRearrange,
filterIds,
children,
}) => {
Expand Down Expand Up @@ -120,7 +120,7 @@ export const DraggableFilter: React.FC<FilterTabTitleProps> = ({
return;
}

onRearrage(dragIndex, hoverIndex);
onRearrange(dragIndex, hoverIndex);
// Note: we're mutating the monitor item here.
// Generally it's better to avoid mutations,
// but it's good here for the sake of performance
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ import { buildNativeFilter } from 'spec/fixtures/mockNativeFilters';
import { act, fireEvent, render, screen } from 'spec/helpers/testing-library';
import FilterConfigPane from './FilterConfigurePane';

const scrollMock = jest.fn();
Element.prototype.scroll = scrollMock;

const defaultProps = {
children: jest.fn(),
getFilterTitle: (id: string) => id,
Expand Down Expand Up @@ -56,6 +59,10 @@ function defaultRender(initialState: any = defaultState, props = defaultProps) {
});
}

beforeEach(() => {
scrollMock.mockClear();
});

test('renders form', async () => {
await act(async () => {
defaultRender();
Expand All @@ -65,7 +72,7 @@ test('renders form', async () => {

test('drag and drop', async () => {
defaultRender();
// Drag the state and contry filter above the product filter
// Drag the state and country filter above the product filter
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for cleaning up all the typos and things along the way. You're making the world a better place! 🧹 ✨

const [countryStateFilter, productFilter] = document.querySelectorAll(
'div[draggable=true]',
);
Expand Down Expand Up @@ -132,3 +139,41 @@ test('add divider', async () => {
});
expect(defaultProps.onAdd).toHaveBeenCalledWith('DIVIDER');
});

test('filter container should scroll to bottom when adding items', async () => {
const state = {
dashboardInfo: {
metadata: {
native_filter_configuration: new Array(35)
.fill(0)
.map((_, index) =>
buildNativeFilter(`NATIVE_FILTER-${index}`, `filter-${index}`, []),
),
},
},
dashboardLayout,
};
const props = {
...defaultProps,
filters: new Array(35).fill(0).map((_, index) => `NATIVE_FILTER-${index}`),
};

defaultRender(state, props);

const addButton = screen.getByText('Add filters and dividers')!;
fireEvent.mouseOver(addButton);

const addFilterButton = await screen.findByText('Filter');
await act(async () => {
fireEvent(
addFilterButton,
new MouseEvent('click', {
bubbles: true,
cancelable: true,
}),
);
});

const containerElement = screen.getByTestId('filter-title-container');
expect(containerElement.scroll).toHaveBeenCalled();
});
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ const TitlesContainer = styled.div`
border-right: 1px solid ${({ theme }) => theme.colors.grayscale.light2};
`;

const FiltureConfigurePane: React.FC<Props> = ({
const FilterConfigurePane: React.FC<Props> = ({
getFilterTitle,
onChange,
onRemove,
Expand All @@ -75,7 +75,7 @@ const FiltureConfigurePane: React.FC<Props> = ({
getFilterTitle={getFilterTitle}
onChange={onChange}
onAdd={(type: NativeFilterType) => onAdd(type)}
onRearrage={onRearrange}
onRearrange={onRearrange}
onRemove={(id: string) => onRemove(id)}
restoreFilter={restoreFilter}
/>
Expand All @@ -98,4 +98,4 @@ const FiltureConfigurePane: React.FC<Props> = ({
);
};

export default FiltureConfigurePane;
export default FilterConfigurePane;
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
import React from 'react';
import React, { forwardRef } from 'react';
import { styled, t } from '@superset-ui/core';
import Icons from 'src/components/Icons';
import { FilterRemoval } from './types';
Expand Down Expand Up @@ -72,124 +72,134 @@ interface Props {
removedFilters: Record<string, FilterRemoval>;
onRemove: (id: string) => void;
restoreFilter: (id: string) => void;
onRearrage: (dragIndex: number, targetIndex: number) => void;
onRearrange: (dragIndex: number, targetIndex: number) => void;
filters: string[];
erroredFilters: string[];
}

const FilterTitleContainer: React.FC<Props> = ({
getFilterTitle,
onChange,
onRemove,
restoreFilter,
onRearrage,
currentFilterId,
removedFilters,
filters,
erroredFilters = [],
}) => {
const renderComponent = (id: string) => {
const isRemoved = !!removedFilters[id];
const isErrored = erroredFilters.includes(id);
const isActive = currentFilterId === id;
const classNames = [];
if (isErrored) {
classNames.push('errored');
}
if (isActive) {
classNames.push('active');
}
return (
<FilterTitle
role="tab"
key={`filter-title-tab-${id}`}
onClick={() => onChange(id)}
className={classNames.join(' ')}
>
<div css={{ display: 'flex', width: '100%' }}>
<div css={{ alignItems: 'center', display: 'flex' }}>
{isRemoved ? t('(Removed)') : getFilterTitle(id)}
const FilterTitleContainer = forwardRef<HTMLDivElement, Props>(
(
{
getFilterTitle,
onChange,
onRemove,
restoreFilter,
onRearrange,
currentFilterId,
removedFilters,
filters,
erroredFilters = [],
},
ref,
) => {
const renderComponent = (id: string) => {
const isRemoved = !!removedFilters[id];
const isErrored = erroredFilters.includes(id);
const isActive = currentFilterId === id;
const classNames = [];
if (isErrored) {
classNames.push('errored');
}
if (isActive) {
classNames.push('active');
}
return (
<FilterTitle
role="tab"
key={`filter-title-tab-${id}`}
onClick={() => onChange(id)}
className={classNames.join(' ')}
>
<div css={{ display: 'flex', width: '100%' }}>
<div css={{ alignItems: 'center', display: 'flex' }}>
{isRemoved ? t('(Removed)') : getFilterTitle(id)}
</div>
{!removedFilters[id] && isErrored && (
<StyledWarning className="warning" />
)}
{isRemoved && (
<span
css={{ alignSelf: 'flex-end', marginLeft: 'auto' }}
role="button"
data-test="undo-button"
tabIndex={0}
onClick={e => {
e.preventDefault();
restoreFilter(id);
}}
>
{t('Undo?')}
</span>
)}
</div>
{!removedFilters[id] && isErrored && (
<StyledWarning className="warning" />
)}
{isRemoved && (
<span
css={{ alignSelf: 'flex-end', marginLeft: 'auto' }}
role="button"
data-test="undo-button"
tabIndex={0}
onClick={e => {
e.preventDefault();
restoreFilter(id);
}}
>
{t('Undo?')}
</span>
)}
</div>
<div css={{ alignSelf: 'flex-start', marginLeft: 'auto' }}>
{isRemoved ? null : (
<StyledTrashIcon
onClick={event => {
event.stopPropagation();
onRemove(id);
}}
alt="RemoveFilter"
/>
)}
</div>
</FilterTitle>
);
};
const recursivelyRender = (
elementId: string,
nodeList: Array<{ id: string; parentId: string | null }>,
rendered: Array<string>,
): React.ReactNode => {
const didAlreadyRender = rendered.indexOf(elementId) >= 0;
if (didAlreadyRender) {
return null;
}
let parent = null;
const element = nodeList.filter(el => el.id === elementId)[0];
if (!element) {
return null;
}
<div css={{ alignSelf: 'flex-start', marginLeft: 'auto' }}>
{isRemoved ? null : (
<StyledTrashIcon
onClick={event => {
event.stopPropagation();
onRemove(id);
}}
alt="RemoveFilter"
/>
)}
</div>
</FilterTitle>
);
};
const recursivelyRender = (
elementId: string,
nodeList: Array<{ id: string; parentId: string | null }>,
rendered: Array<string>,
): React.ReactNode => {
const didAlreadyRender = rendered.indexOf(elementId) >= 0;
if (didAlreadyRender) {
return null;
}
let parent = null;
const element = nodeList.filter(el => el.id === elementId)[0];
if (!element) {
return null;
}

rendered.push(elementId);
if (element.parentId) {
parent = recursivelyRender(element.parentId, nodeList, rendered);
}
const children = nodeList
.filter(item => item.parentId === elementId)
.map(item => recursivelyRender(item.id, nodeList, rendered));
return (
<>
{parent}
{renderComponent(elementId)}
{children}
</>
);
};

const renderFilterGroups = () => {
const items: React.ReactNode[] = [];
filters.forEach((item, index) => {
items.push(
<DraggableFilter
key={index}
onRearrange={onRearrange}
index={index}
filterIds={[item]}
>
{renderComponent(item)}
</DraggableFilter>,
);
});
return items;
};

rendered.push(elementId);
if (element.parentId) {
parent = recursivelyRender(element.parentId, nodeList, rendered);
}
const children = nodeList
.filter(item => item.parentId === elementId)
.map(item => recursivelyRender(item.id, nodeList, rendered));
return (
<>
{parent}
{renderComponent(elementId)}
{children}
</>
<Container data-test="filter-title-container" ref={ref}>
{renderFilterGroups()}
</Container>
);
};

const renderFilterGroups = () => {
const items: React.ReactNode[] = [];
filters.forEach((item, index) => {
items.push(
<DraggableFilter
key={index}
onRearrage={onRearrage}
index={index}
filterIds={[item]}
>
{renderComponent(item)}
</DraggableFilter>,
);
});
return items;
};
return <Container>{renderFilterGroups()}</Container>;
};
},
);

export default FilterTitleContainer;
Loading