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

[FC-0036] Refined taxonomy details page #833

Merged
Show file tree
Hide file tree
Changes from 4 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
1 change: 1 addition & 0 deletions src/taxonomy/index.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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";
71 changes: 38 additions & 33 deletions src/taxonomy/tag-list/TagListTable.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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="tag-table-expand-row"><DataTable.ExpandRow row={row} /></div> : null
);
OptionalExpandLink.propTypes = DataTable.ExpandRow.propTypes;

/**
Expand All @@ -64,6 +66,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);
Expand All @@ -75,38 +78,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>
);
};

Expand Down
15 changes: 15 additions & 0 deletions src/taxonomy/tag-list/TagListTable.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
.tag-list-table {
.tag-table-expand-row {
float: right;
}

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.
float: right;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think instead of this you should be able to use d-flex justify-content-end for the first one and use display:flex; justify-content: flex-end for the second.

IMO using float can sometimes cause issues since it breaks the normal flow of the document.

56 changes: 41 additions & 15 deletions src/taxonomy/tag-list/TagListTable.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ 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 } from '@testing-library/react';
import { render, waitFor, screen } from '@testing-library/react';
import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
import MockAdapter from 'axios-mock-adapter';

Expand Down Expand Up @@ -56,7 +56,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,
Expand Down Expand Up @@ -99,33 +108,50 @@ 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('two level tag 1')).toBeInTheDocument();
});
const rows = result.getAllByRole('row');
render(<RootWrapper />);
const tag = await screen.findByText('two level tag 1');
expect(tag).toBeInTheDocument();

const rows = screen.getAllByRole('row');
expect(rows.length).toBe(3 + 1); // 3 items plus header
});

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();
});
});
8 changes: 4 additions & 4 deletions src/taxonomy/tag-list/data/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ 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}`,
const getTagListApiUrl = (taxonomyId, page, pageSize) => new URL(
`api/content_tagging/v1/taxonomies/${taxonomyId}/tags/?page=${page + 1}&page_size=${pageSize}`,
getApiBaseUrl(),
).href;
Copy link
Member

Choose a reason for hiding this comment

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

Just a suggestion that I think might help avoid any issues that would be hard to spot when needing to update/add queryparams in the future.

Suggested change
const getTagListApiUrl = (taxonomyId, page, pageSize) => new URL(
`api/content_tagging/v1/taxonomies/${taxonomyId}/tags/?page=${page + 1}&page_size=${pageSize}`,
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;
};

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks!


Expand All @@ -20,11 +20,11 @@ const getTagListApiUrl = (taxonomyId, page) => new URL(
* @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);
},
});
Expand Down
1 change: 1 addition & 0 deletions src/taxonomy/tag-list/data/types.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
/**
* @typedef {Object} QueryOptions
* @property {number} pageIndex
* @property {number} pageSize
*/

/**
Expand Down
Loading