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.

Sign up for GitHub

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

fix(tags): Update loading + pagination for Tags Page #25473

Merged
merged 4 commits into from
Oct 5, 2023
Merged

fix(tags): Update loading + pagination for Tags Page #25473

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a1fca2a
update loading + pagination
hughhhh Sep 30, 2023
1fa758f
ok
hughhhh Oct 4, 2023
fdd8541
update loading states
hughhhh Oct 5, 2023
6fa3172
update styles
hughhhh Oct 5, 2023
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
46 changes: 7 additions & 39 deletions superset-frontend/src/features/allEntities/AllEntitiesTable.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,20 +16,18 @@
* specific language governing permissions and limitations
* under the License.
*/
import React, { useState, useEffect } from 'react';
import React from 'react';
import moment from 'moment';
import { t, styled, logging } from '@superset-ui/core';
import { t, styled } from '@superset-ui/core';
import TableView, { EmptyWrapperType } from 'src/components/TableView';
import { addDangerToast } from 'src/components/MessageToasts/actions';
import Loading from 'src/components/Loading';
import { TagsList } from 'src/components/Tags';
import FacePile from 'src/components/FacePile';
import Tag from 'src/types/TagType';
import Owner from 'src/types/Owner';
import { EmptyStateBig } from 'src/components/EmptyState';
import { fetchObjects } from '../tags/tags';

const MAX_TAGS_TO_SHOW = 3;
const PAGE_SIZE = 5;
Copy link
Member

Choose a reason for hiding this comment

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

Should this be 50, like before?

Copy link
Member Author

Choose a reason for hiding this comment

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

design wants this to be 10 since there are 3 tables on the 1 page


const AllEntitiesTableContainer = styled.div`
text-align: left;
Expand Down Expand Up @@ -63,7 +61,7 @@ interface TaggedObject {
tags: Tag[];
}

interface TaggedObjects {
export interface TaggedObjects {
dashboard: TaggedObject[];
chart: TaggedObject[];
query: TaggedObject[];
Expand All @@ -72,50 +70,21 @@ interface TaggedObjects {
interface AllEntitiesTableProps {
search?: string;
setShowTagModal: (show: boolean) => void;
objects: TaggedObjects;
}

export default function AllEntitiesTable({
search = '',
setShowTagModal,
objects,
}: AllEntitiesTableProps) {
type objectType = 'dashboard' | 'chart' | 'query';

const [objects, setObjects] = useState<TaggedObjects>({
dashboard: [],
chart: [],
query: [],
});
const [isLoading, setLoading] = useState<boolean>(true);
const showListViewObjs =
objects.dashboard.length > 0 ||
objects.chart.length > 0 ||
objects.query.length > 0;

useEffect(() => {
if (search === '') {
return;
}

setLoading(true);

fetchObjects(
{ tags: search, types: null },
(data: TaggedObject[]) => {
const objects = { dashboard: [], chart: [], query: [] };
data.forEach(function (object) {
const object_type = object.type;
objects[object_type].push(object);
});
setObjects(objects);
setLoading(false);
},
(error: Response) => {
addDangerToast('Error Fetching Tagged Objects');
logging.log(error.text);
},
);
}, [search]);

const renderTable = (type: objectType) => {
const data = objects[type].map((o: TaggedObject) => ({
[type]: <a href={o.url}>{o.name}</a>,
Expand All @@ -129,7 +98,7 @@ export default function AllEntitiesTable({
className="table-condensed"
emptyWrapperType={EmptyWrapperType.Small}
data={data}
pageSize={50}
pageSize={PAGE_SIZE}
columns={[
{
accessor: type,
Expand Down Expand Up @@ -176,7 +145,6 @@ export default function AllEntitiesTable({
);
};

if (isLoading) return <Loading />;
return (
<AllEntitiesTableContainer>
{showListViewObjs ? (
Expand Down
59 changes: 56 additions & 3 deletions superset-frontend/src/pages/AllEntities/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@
import React, { useEffect, useState } from 'react';
import { styled, t, css, SupersetTheme } from '@superset-ui/core';
import { NumberParam, useQueryParam } from 'use-query-params';
import AllEntitiesTable from 'src/features/allEntities/AllEntitiesTable';
import AllEntitiesTable, {
TaggedObjects,
} from 'src/features/allEntities/AllEntitiesTable';
import Button from 'src/components/Button';
import MetadataBar, {
MetadataType,
Expand All @@ -28,10 +30,23 @@ import MetadataBar, {
LastModified,
} from 'src/components/MetadataBar';
import { PageHeaderWithActions } from 'src/components/PageHeaderWithActions';
import { fetchSingleTag } from 'src/features/tags/tags';
import { Tag } from 'src/views/CRUD/types';
import TagModal from 'src/features/tags/TagModal';
import withToasts, { useToasts } from 'src/components/MessageToasts/withToasts';
import { fetchObjects, fetchSingleTag } from 'src/features/tags/tags';
import Loading from 'src/components/Loading';

interface TaggedObject {
id: number;
type: string;
name: string;
url: string;
changed_on: moment.MomentInput;
created_by: number | undefined;
creator: string;
owners: Owner[];
tags: Tag[];
}

const additionalItemsStyles = (theme: SupersetTheme) => css`
display: flex;
Expand Down Expand Up @@ -59,6 +74,9 @@ const AllEntitiesContainer = styled.div`
.entities {
margin: ${theme.gridUnit * 6}px; 0px;
}
.pagination-container {
background-color: transparent;
}
`}
`;

Expand Down Expand Up @@ -88,6 +106,12 @@ function AllEntities() {
const [tag, setTag] = useState<Tag | null>(null);
const [showTagModal, setShowTagModal] = useState<boolean>(false);
const { addSuccessToast, addDangerToast } = useToasts();
const [isLoading, setLoading] = useState<boolean>(false);
const [objects, setObjects] = useState<TaggedObjects>({
dashboard: [],
chart: [],
query: [],
});

const editableTitleProps = {
title: tag?.name || '',
Expand All @@ -114,21 +138,49 @@ function AllEntities() {
};
const items = [description, owner, lastModified];

const fetchTaggedObjects = () => {
setLoading(true);
fetchObjects(
{ tags: tag?.name || '', types: null },
(data: TaggedObject[]) => {
const objects = { dashboard: [], chart: [], query: [] };
data.forEach(function (object) {
const object_type = object.type;
objects[object_type].push(object);
});
setObjects(objects);
setLoading(false);
},
(error: Response) => {
addDangerToast('Error Fetching Tagged Objects');
Copy link
Member

Choose a reason for hiding this comment

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

you might need to add setLoading(false); here to prevent the spinner kept spinning

setLoading(false);
},
);
};

useEffect(() => {
// fetch single tag met
if (tagId) {
setLoading(true);
fetchSingleTag(
tagId,
(tag: Tag) => {
setTag(tag);
setLoading(false);
},
(error: Response) => {
addDangerToast(t('Error Fetching Tagged Objects'));
Copy link
Member

@lilykuang lilykuang Oct 4, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

same as above

setLoading(false);
},
);
}
}, [tagId]);

useEffect(() => {
if (tag) fetchTaggedObjects();
}, [tag]);

if (isLoading) return <Loading />;
return (
<AllEntitiesContainer>
<TagModal
Expand All @@ -139,7 +191,7 @@ function AllEntities() {
editTag={tag}
addSuccessToast={addSuccessToast}
addDangerToast={addDangerToast}
refreshData={() => {}} // todo(hugh): implement refreshData on table reload
refreshData={fetchTaggedObjects}
/>
<AllEntitiesNav>
<PageHeaderWithActions
Expand Down Expand Up @@ -174,6 +226,7 @@ function AllEntities() {
<AllEntitiesTable
search={tag?.name || ''}
setShowTagModal={setShowTagModal}
objects={objects}
/>
</div>
</AllEntitiesContainer>
Expand Down