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

Jump to bottom

fix: miscellaneous fixes to table components #610

Open
wants to merge 4 commits into
base: staging
Choose a base branch
from
Open

fix: miscellaneous fixes to table components #610

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
7b06891
show search filters
carissa-tang Oct 29, 2023
7fff2a6
pr comment: rename searchLength to resultsLength
carissa-tang Oct 30, 2023
89b3f32
miscellaneous table fixes
carissa-tang Oct 30, 2023
a101fce
Merge remote-tracking branch 'origin/staging' into carissa/fix-search…
carissa-tang Oct 30, 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
52 changes: 0 additions & 52 deletions frontend/src/components/admin/user-management/admin/AdminTab.tsx
View file
Open in desktop

This file was deleted.

5 changes: 4 additions & 1 deletion frontend/src/components/admin/user-management/admin/AdminUserTable.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,14 @@ import type {
AuthenticatedAdmin,
AuthenticatedTeacher,
} from "../../../../types/AuthTypes";
import type { AdminUser } from "../../../../types/UserTypes";
import type { TableRow } from "../../../common/table/Table";
import { Table } from "../../../common/table/Table";
import RemoveUserPopover from "../RemoveUserPopover";

import type { AdminTableProps } from "./AdminTab";
interface AdminTableProps {
users: AdminUser[];
}

const AdminUserTable = ({ users }: AdminTableProps): React.ReactElement => {
const { authenticatedUser } = useContext(AuthContext);
Expand Down
6 changes: 5 additions & 1 deletion frontend/src/components/admin/user-management/teacher/TeacherUserTable.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
import React from "react";

import type { TeacherUser } from "../../../../types/UserTypes";
import type { TableRow } from "../../../common/table/Table";
import { Table } from "../../../common/table/Table";
import type { TeacherTableProps } from "../admin/AdminTab";
import RemoveUserPopover from "../RemoveUserPopover";

export interface TeacherTableProps {
users: TeacherUser[];
}

const TeacherUserTable = ({ users }: TeacherTableProps): React.ReactElement => {
const headers = ["Name", "School", "Email"];
const rows: TableRow[] = users.map((user) => ({
Expand Down
21 changes: 21 additions & 0 deletions frontend/src/components/common/info/messages/EmptyUsersMessage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import React from "react";
import { Center, useStyleConfig } from "@chakra-ui/react";

import DisplayStudentsIllustration from "../../../../assets/illustrations/display-students.svg";
import MessageContainer from "../MessageContainer";

const EmptyUsersMessage = ({ role }: { role: string }): React.ReactElement => {
const styles = useStyleConfig("Center", { variant: "emptyMessage" });
return (
<Center __css={styles}>
<MessageContainer
image={DisplayStudentsIllustration}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

not the most fitting image, but i don't think we have another design for this

Copy link
Contributor

Choose a reason for hiding this comment

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

Fair! Let's add a backlog ticket to ask design for a better image, but I'm fine with keeping this image for the mvp

paragraphs={[]}
subtitle={`You currently have no ${role}s`}
textColor="blue.300"
/>
</Center>
);
};

export default EmptyUsersMessage;
6 changes: 3 additions & 3 deletions frontend/src/components/common/table/Pagination.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@ import {
PaginationContainer,
PaginationNext,
PaginationPage,
PaginationPageGroup,
PaginationPrevious,
PaginationSeparator,
usePagination,
} from "@ajna/pagination";
import { HStack } from "@chakra-ui/react";

const outerLimit = 1;
const innerLimit = 1;
Expand Down Expand Up @@ -44,7 +44,7 @@ const Pagination = ({
&lt; Previous
</PaginationPrevious>
)}
<PaginationPageGroup align="center" isInline>
<HStack align="center">
Copy link
Contributor

Choose a reason for hiding this comment

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

In order to maintain semantic HTML I think we should use the library's built-in components (according to here the component already sets isInline so we sidestep the deprecation), or else duplicate all the other props that the library already sets.

{pages.map((page: number, index: number) => {
if (
index === 0 ||
Expand Down Expand Up @@ -85,7 +85,7 @@ const Pagination = ({
}
return null;
})}
</PaginationPageGroup>
</HStack>

{currentPage !== pagesCount && (
<PaginationNext
Expand Down
4 changes: 3 additions & 1 deletion frontend/src/components/common/table/SearchBar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,11 @@ import { Input, InputGroup, InputRightElement } from "@chakra-ui/react";
import { SearchOutlineIcon } from "../../../assets/icons";

export interface SearchBarProps {
search: string;
onSearch: Dispatch<SetStateAction<string>>;
}

const SearchBar = ({ onSearch }: SearchBarProps): ReactElement => {
const SearchBar = ({ search, onSearch }: SearchBarProps): ReactElement => {
const handleInputChange = useCallback(
(event: ChangeEvent<HTMLInputElement>) => {
onSearch(event.target.value);
Expand All @@ -30,6 +31,7 @@ const SearchBar = ({ onSearch }: SearchBarProps): ReactElement => {
borderRadius="6px"
onChange={handleInputChange}
placeholder="Search bar"
value={search}
/>
<InputRightElement h="full" pointerEvents="none">
<SearchOutlineIcon />
Expand Down
14 changes: 8 additions & 6 deletions frontend/src/components/common/table/SearchableTablePage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,14 @@ const SearchableTablePage = <T, SortPropTypes extends readonly string[]>({
);
return (
<>
<VStack pt={4} spacing={6} w="full">
<HStack width="100%">
{searchBarComponent}
{sortMenuComponent}
{filterMenuComponent}
</HStack>
<VStack py={4} spacing={6} w="full">
{!noResults && (
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to make sure, have you checked that we never pass in filteredData.length for this prop anywhere? That would result in filters breaking the page, so you can probably just set the filters on every filter page to a combination with no results.

<HStack width="100%">
{searchBarComponent}
{sortMenuComponent}
{filterMenuComponent}
</HStack>
)}
{search && (
<Text color="grey.300" fontSize="16px" width="100%">
Showing {resultsLength} results for &quot;
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/components/common/table/Table.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export const Table = <T extends Nodes = Nodes>({
usePaginatedData(rows);

return (
<VStack alignItems="center" paddingBottom="6" spacing="6" width="100%">
<VStack alignItems="center" spacing="6" width="100%">
<TableContainer
border="1px solid"
borderColor="blue.50"
Expand Down
16 changes: 8 additions & 8 deletions frontend/src/components/pages/admin/DisplayAssessmentsPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,6 @@ const DisplayAssessmentsPage = (): React.ReactElement => {
const [regions, setRegions] = React.useState<Array<string>>([]);
const [status, setStatus] = React.useState("");

const [isEmpty, setEmpty] = React.useState(true);

const [filterOptions, setFilterOptions] = React.useState<FilterProp[]>([
{ label: "Grade", setState: setGrades, options: [] },
{ label: "Type", setState: setTestTypes, options: [] },
Expand Down Expand Up @@ -87,9 +85,6 @@ const DisplayAssessmentsPage = (): React.ReactElement => {

const filteredAssessments = React.useMemo(() => {
if (!data) return [];
if (data.tests.length) {
setEmpty(false);
}

const filterProps = [grades, testTypes, countries, regions, status];
return filterAssessments(data.tests, filterProps);
Expand All @@ -109,11 +104,13 @@ const DisplayAssessmentsPage = (): React.ReactElement => {
<SearchableTablePage
filterMenuComponent={<FilterMenu filterProps={filterOptions} />}
nameOfTableItems="assessments"
noResults={isEmpty}
noResults={(data?.tests?.length ?? 0) === 0}
Copy link
Contributor

Choose a reason for hiding this comment

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

You could also just do !data?.tests?.length

noResultsComponent={<EmptyTestsMessage />}
resultsLength={assessments.length}
search={search}
searchBarComponent={<SearchBar onSearch={setSearch} />}
searchBarComponent={
<SearchBar onSearch={setSearch} search={search} />
}
sortMenuComponent={
<SortMenu
initialSortOrder="descending"
Expand Down Expand Up @@ -147,7 +144,10 @@ const DisplayAssessmentsPage = (): React.ReactElement => {
<QueryStateHandler error={error} loading={loading}>
<Tabs
marginTop={3}
onChange={(index) => setStatus(STATUS_ORDER[index])}
onChange={(index) => {
setSearch("");
setStatus(STATUS_ORDER[index]);
}}
>
<TabList>
<Tab>All</Tab>
Expand Down
25 changes: 18 additions & 7 deletions frontend/src/components/pages/admin/UsersPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,12 @@ import {
filterAdminUsersBySearch,
filterTeacherUsersBySearch,
} from "../../../utils/UserUtils";
import AdminTab from "../../admin/user-management/admin/AdminTab";
import AdminUserTable from "../../admin/user-management/admin/AdminUserTable";
import TeacherUserTable from "../../admin/user-management/teacher/TeacherUserTable";
import UsersPageHeader from "../../admin/user-management/UsersPageHeader";
import EmptyUsersMessage from "../../common/info/messages/EmptyUsersMessage";
import QueryStateHandler from "../../common/QueryStateHandler";
import SearchableTablePage from "../../common/table/SearchableTablePage";
import SearchBar from "../../common/table/SearchBar";
import SortMenu, { type SortOrder } from "../../common/table/SortMenu";
import useSortProperty from "../../common/table/useSortProperty";
Expand Down Expand Up @@ -106,10 +107,15 @@ const UsersPage = (): React.ReactElement => {
</TabList>
<TabPanels>
<TabPanel padding="0">
<AdminTab
<SearchableTablePage
nameOfTableItems="admins"
noResults={(adminData?.usersByRole?.length ?? 0) === 0}
noResultsComponent={<EmptyUsersMessage role="admin" />}
resultsLength={admins.length}
search={search}
searchBarComponent={<SearchBar onSearch={setSearch} />}
searchBarComponent={
<SearchBar onSearch={setSearch} search={search} />
}
sortMenuComponent={
<SortMenu
labels={["name", "email"]}
Expand All @@ -118,14 +124,19 @@ const UsersPage = (): React.ReactElement => {
properties={ADMIN_SORT_PROPERTIES}
/>
}
UserTable={<AdminUserTable users={admins} />}
tableComponent={<AdminUserTable users={admins} />}
/>
</TabPanel>
<TabPanel padding="0">
<AdminTab
<SearchableTablePage
nameOfTableItems="teachers"
noResults={(teacherData?.teachers?.length ?? 0) === 0}
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

Suggested change
noResults={(teacherData?.teachers?.length ?? 0) === 0}
noResults={!teacherData?.teachers?.length}

noResultsComponent={<EmptyUsersMessage role="teacher" />}
resultsLength={teachers.length}
search={search}
searchBarComponent={<SearchBar onSearch={setSearch} />}
searchBarComponent={
<SearchBar onSearch={setSearch} search={search} />
}
sortMenuComponent={
<SortMenu
labels={["name", "email", "school"]}
Expand All @@ -134,7 +145,7 @@ const UsersPage = (): React.ReactElement => {
properties={TEACHER_SORT_PROPERTIES}
/>
}
UserTable={<TeacherUserTable users={teachers} />}
tableComponent={<TeacherUserTable users={teachers} />}
/>
</TabPanel>
</TabPanels>
Expand Down
4 changes: 2 additions & 2 deletions ...end/src/components/pages/teacher/DisplayClassroomPage/DisplayClassroomAssessmentsPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ const DisplayClassroomAssessmentsPage = () => {
<QueryStateHandler error={error} loading={loading}>
<SearchableTablePage
nameOfTableItems="assessments"
noResults={paginatedData.length === 0}
noResults={(data?.class?.testSessions?.length ?? 0) === 0}
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto

Suggested change
noResults={(data?.class?.testSessions?.length ?? 0) === 0}
noResults={!data?.class?.testSessions?.length}

noResultsComponent={
<EmptyClassSessionsMessage
classroomId={classroomId}
Expand All @@ -110,7 +110,7 @@ const DisplayClassroomAssessmentsPage = () => {
}
resultsLength={paginatedData.length}
search={search}
searchBarComponent={<SearchBar onSearch={setSearch} />}
searchBarComponent={<SearchBar onSearch={setSearch} search={search} />}
sortMenuComponent={
<SortMenu
initialSortOrder={sortOrder}
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/components/pages/teacher/DisplayClassroomPage/DisplayClassroomStudentsPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ const DisplayClassroomStudentsPage = ({
}
resultsLength={students.length}
search={search}
searchBarComponent={<SearchBar onSearch={setSearch} />}
searchBarComponent={<SearchBar onSearch={setSearch} search={search} />}
sortMenuComponent={
<SortMenu
initialSortOrder="descending"
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/components/teacher/results/StudentList/StudentList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ const StudentList = ({
return (
<Flex direction="column" height="100%" w="sm">
<Flex alignItems="center" justifyContent="space-between" mb={4} p={1}>
<SearchBar onSearch={setSearch} />
<SearchBar onSearch={setSearch} search={search} />
<SortMenu labels={[]} onSortOrder={setSortDirection} properties={[]} />
</Flex>
<OrderedList
Expand Down
Loading
Loading