[FC-0036] Refined taxonomy details page (#833)

* UX refinements on tag list table * Add page size to tag list table * fix Datatable pagination
openedx · Feb 27, 2024 · 608b2f7 · 608b2f7
1 parent 6b57ce3
commit 608b2f7
Show file tree

Hide file tree

Showing 6 changed files with 103 additions and 54 deletions.
diff --git a/src/taxonomy/index.scss b/src/taxonomy/index.scss
@@ -3,3 +3,4 @@
 @import "taxonomy/delete-dialog/DeleteDialog";
 @import "taxonomy/system-defined-badge/SystemDefinedBadge";
 @import "taxonomy/export-modal/ExportModal";
+@import "taxonomy/tag-list/TagListTable";
diff --git a/src/taxonomy/tag-list/TagListTable.jsx b/src/taxonomy/tag-list/TagListTable.jsx
@@ -39,7 +39,9 @@ SubTagsExpanded.propTypes = {
 /**
  * An "Expand" toggle to show/hide subtags, but one which is hidden if the given tag row has no subtags.
  */
-const OptionalExpandLink = ({ row }) => (row.original.childCount > 0 ? <DataTable.ExpandRow row={row} /> : null);
+const OptionalExpandLink = ({ row }) => (
+  row.original.childCount > 0 ? <div className="d-flex justify-content-end"><DataTable.ExpandRow row={row} /></div> : null
+);
 OptionalExpandLink.propTypes = DataTable.ExpandRow.propTypes;
 
 /**
@@ -65,6 +67,7 @@ const TagListTable = ({ taxonomyId }) => {
   const intl = useIntl();
   const [options, setOptions] = useState({
     pageIndex: 0,
+    pageSize: 100,
   });
   const { isLoading } = useTagListDataStatus(taxonomyId, options);
   const tagList = useTagListDataResponse(taxonomyId, options);
@@ -76,38 +79,40 @@ const TagListTable = ({ taxonomyId }) => {
   };
 
   return (
-    <DataTable
-      isLoading={isLoading}
-      isPaginated
-      manualPagination
-      fetchData={fetchData}
-      data={tagList?.results || []}
-      itemCount={tagList?.count || 0}
-      pageCount={tagList?.numPages || 0}
-      initialState={options}
-      isExpandable
-      // This is a temporary "bare bones" solution for brute-force loading all the child tags. In future we'll match
-      // the Figma design and do something more sophisticated.
-      renderRowSubComponent={({ row }) => (
-        <SubTagsExpanded taxonomyId={taxonomyId} parentTagValue={row.original.value} />
-      )}
-      columns={[
-        {
-          Header: intl.formatMessage(messages.tagListColumnValueHeader),
-          Cell: TagValue,
-        },
-        {
-          id: 'expander',
-          Header: DataTable.ExpandAll,
-          Cell: OptionalExpandLink,
-        },
-      ]}
-    >
-      <DataTable.TableControlBar />
-      <DataTable.Table />
-      <DataTable.EmptyTable content={intl.formatMessage(messages.noResultsFoundMessage)} />
-      <DataTable.TableFooter />
-    </DataTable>
+    <div className="tag-list-table">
+      <DataTable
+        isLoading={isLoading}
+        isPaginated
+        manualPagination
+        fetchData={fetchData}
+        data={tagList?.results || []}
+        itemCount={tagList?.count || 0}
+        pageCount={tagList?.numPages || 0}
+        initialState={options}
+        isExpandable
+        // This is a temporary "bare bones" solution for brute-force loading all the child tags. In future we'll match
+        // the Figma design and do something more sophisticated.
+        renderRowSubComponent={({ row }) => (
+          <SubTagsExpanded taxonomyId={taxonomyId} parentTagValue={row.original.value} />
+        )}
+        columns={[
+          {
+            Header: intl.formatMessage(messages.tagListColumnValueHeader),
+            Cell: TagValue,
+          },
+          {
+            id: 'expander',
+            Header: DataTable.ExpandAll,
+            Cell: OptionalExpandLink,
+          },
+        ]}
+      >
+        <DataTable.Table />
+        <DataTable.EmptyTable content={intl.formatMessage(messages.noResultsFoundMessage)} />
+        {tagList?.numPages !== undefined && tagList?.numPages > 1
+          && <DataTable.TableFooter />}
+      </DataTable>
+    </div>
   );
 };
 

diff --git a/src/taxonomy/tag-list/TagListTable.scss b/src/taxonomy/tag-list/TagListTable.scss
@@ -0,0 +1,12 @@
+.tag-list-table {
+  table tr:first-child > th:nth-child(2) > span {
+    // Used to move "Expand all" button to the right.
+    // Find the first <span> of the second <th> of the first <tr> of the <table>.
+    //
+    // The approach of the expand buttons cannot be applied here since the
+    // table headers are rendered differently and at the component level
+    // there is no control of this style.
+    display: flex;
+    justify-content: flex-end;
+  }
+}
diff --git a/src/taxonomy/tag-list/TagListTable.test.jsx b/src/taxonomy/tag-list/TagListTable.test.jsx
@@ -3,7 +3,9 @@ import { IntlProvider } from '@edx/frontend-platform/i18n';
 import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth';
 import { initializeMockApp } from '@edx/frontend-platform';
 import { AppProvider } from '@edx/frontend-platform/react';
-import { render, waitFor, within } from '@testing-library/react';
+import {
+  render, waitFor, screen, within,
+} from '@testing-library/react';
 import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
 import MockAdapter from 'axios-mock-adapter';
 
@@ -59,7 +61,16 @@ const mockTagsResponse = {
     },
   ],
 };
-const rootTagsListUrl = 'http://localhost:18010/api/content_tagging/v1/taxonomies/1/tags/?page=1';
+const mockTagsPaginationResponse = {
+  next: null,
+  previous: null,
+  count: 103,
+  num_pages: 2,
+  current_page: 1,
+  start: 0,
+  results: [],
+};
+const rootTagsListUrl = 'http://localhost:18010/api/content_tagging/v1/taxonomies/1/tags/?page=1&page_size=100';
 const subTagsResponse = {
   next: null,
   previous: null,
@@ -102,22 +113,21 @@ describe('<TagListTable />', () => {
     let resolveResponse;
     const promise = new Promise(resolve => { resolveResponse = resolve; });
     axiosMock.onGet(rootTagsListUrl).reply(() => promise);
-    const result = render(<RootWrapper />);
-    const spinner = result.getByRole('status');
+    render(<RootWrapper />);
+    const spinner = screen.getByRole('status');
     expect(spinner.textContent).toEqual('loading');
     resolveResponse([200, {}]);
-    await waitFor(() => {
-      expect(result.getByText('No results found')).toBeInTheDocument();
-    });
+    const noFoundComponent = await screen.findByText('No results found');
+    expect(noFoundComponent).toBeInTheDocument();
   });
 
   it('should render page correctly', async () => {
     axiosMock.onGet(rootTagsListUrl).reply(200, mockTagsResponse);
-    const result = render(<RootWrapper />);
-    await waitFor(() => {
-      expect(result.getByText('root tag 1')).toBeInTheDocument();
-    });
-    const rows = result.getAllByRole('row');
+    render(<RootWrapper />);
+    const tag = await screen.findByText('root tag 1');
+    expect(tag).toBeInTheDocument();
+
+    const rows = screen.getAllByRole('row');
     expect(rows.length).toBe(3 + 1); // 3 items plus header
     expect(within(rows[0]).getAllByRole('columnheader')[0].textContent).toEqual('Tag name');
     expect(within(rows[1]).getAllByRole('cell')[0].textContent).toEqual('root tag 1 (14)');
@@ -126,11 +136,29 @@ describe('<TagListTable />', () => {
   it('should render page correctly with subtags', async () => {
     axiosMock.onGet(rootTagsListUrl).reply(200, mockTagsResponse);
     axiosMock.onGet(subTagsUrl).reply(200, subTagsResponse);
-    const result = render(<RootWrapper />);
-    const expandButton = result.getAllByLabelText('Expand row')[0];
+    render(<RootWrapper />);
+    const expandButton = screen.getAllByLabelText('Expand row')[0];
     expandButton.click();
+    const childTag = await screen.findByText('the child tag');
+    expect(childTag).toBeInTheDocument();
+  });
+
+  it('should not render pagination footer', async () => {
+    axiosMock.onGet(rootTagsListUrl).reply(200, mockTagsResponse);
+    render(<RootWrapper />);
     await waitFor(() => {
-      expect(result.getByText('the child tag')).toBeInTheDocument();
+      expect(screen.queryByRole('navigation', {
+        name: /table pagination/i,
+      })).not.toBeInTheDocument();
+    });
+  });
+
+  it('should render pagination footer', async () => {
+    axiosMock.onGet(rootTagsListUrl).reply(200, mockTagsPaginationResponse);
+    render(<RootWrapper />);
+    const tableFooter = await screen.findByRole('navigation', {
+      name: /table pagination/i,
     });
+    expect(tableFooter).toBeInTheDocument();
   });
 });
diff --git a/src/taxonomy/tag-list/data/api.js b/src/taxonomy/tag-list/data/api.js
@@ -9,22 +9,24 @@ import { camelCaseObject, getConfig } from '@edx/frontend-platform';
 import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth';
 
 const getApiBaseUrl = () => getConfig().STUDIO_BASE_URL;
-const getTagListApiUrl = (taxonomyId, page) => new URL(
-  `api/content_tagging/v1/taxonomies/${taxonomyId}/tags/?page=${page + 1}`,
-  getApiBaseUrl(),
-).href;
+const getTagListApiUrl = (taxonomyId, page, pageSize) => {
+  const url = new URL(`api/content_tagging/v1/taxonomies/${taxonomyId}/tags/`, getApiBaseUrl());
+  url.searchParams.append('page', page + 1);
+  url.searchParams.append('page_size', pageSize);
+  return url.href;
+};
 
 /**
  * @param {number} taxonomyId
  * @param {import('./types.mjs').QueryOptions} options
  * @returns {import('@tanstack/react-query').UseQueryResult<import('./types.mjs').TagListData>}
  */
 export const useTagListData = (taxonomyId, options) => {
-  const { pageIndex } = options;
+  const { pageIndex, pageSize } = options;
   return useQuery({
     queryKey: ['tagList', taxonomyId, pageIndex],
     queryFn: async () => {
-      const { data } = await getAuthenticatedHttpClient().get(getTagListApiUrl(taxonomyId, pageIndex));
+      const { data } = await getAuthenticatedHttpClient().get(getTagListApiUrl(taxonomyId, pageIndex, pageSize));
       return camelCaseObject(data);
     },
   });

diff --git a/src/taxonomy/tag-list/data/types.mjs b/src/taxonomy/tag-list/data/types.mjs
@@ -7,6 +7,7 @@
 /**
  * @typedef {Object} QueryOptions
  * @property {number} pageIndex
+ * @property {number} pageSize
  */
 
 /**
-Original file line number
+Diff line change
@@ Expand Up / @@ -7,6 +7,7 @@ @@
     /**
      * @typedef {Object} QueryOptions
      * @property {number} pageIndex
+     * @property {number} pageSize
      */
     /**
@@ Expand Down @@