From daf076a57ea9ef9c63346765caf50e459ca53dc6 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 6 Nov 2021 10:55:01 +0100 Subject: [PATCH 1/8] Moved logic to sort tags to TagsList component, to allow sorting on any context --- src/api/types/index.ts | 5 +-- src/settings/RealTimeUpdates.tsx | 2 +- src/tags/TagCard.tsx | 55 ++++++++++++-------------- src/tags/TagsCards.tsx | 13 +++--- src/tags/TagsList.tsx | 39 +++++++++++++++--- src/tags/TagsTable.tsx | 39 +++++------------- src/tags/data/TagsListChildrenProps.ts | 9 ++++- 7 files changed, 86 insertions(+), 76 deletions(-) diff --git a/src/api/types/index.ts b/src/api/types/index.ts index b49e4338..b16111b9 100644 --- a/src/api/types/index.ts +++ b/src/api/types/index.ts @@ -25,12 +25,12 @@ interface ShlinkTagsStats { export interface ShlinkTags { tags: string[]; - stats?: ShlinkTagsStats[]; // Is only optional in Shlink older than v2.2 + stats: ShlinkTagsStats[]; } export interface ShlinkTagsResponse { data: string[]; - stats?: ShlinkTagsStats[]; // Is only optional in Shlink older than v2.2 + stats: ShlinkTagsStats[]; } export interface ShlinkPaginator { @@ -90,7 +90,6 @@ export interface ProblemDetailsError { detail: string; title: string; status: number; - [extraProps: string]: any; } diff --git a/src/settings/RealTimeUpdates.tsx b/src/settings/RealTimeUpdates.tsx index 8284eeaf..243a7d0c 100644 --- a/src/settings/RealTimeUpdates.tsx +++ b/src/settings/RealTimeUpdates.tsx @@ -18,7 +18,7 @@ const RealTimeUpdates = ( - Enable or disable real-time updates, when using Shlink v2.2.0 or newer. + Enable or disable real-time updates. Real-time updates are currently being {realTimeUpdates.enabled ? 'processed' : 'ignored'}. diff --git a/src/tags/TagCard.tsx b/src/tags/TagCard.tsx index e3cd220e..df0fd26a 100644 --- a/src/tags/TagCard.tsx +++ b/src/tags/TagCard.tsx @@ -8,12 +8,11 @@ import { useToggle } from '../utils/helpers/hooks'; import ColorGenerator from '../utils/services/ColorGenerator'; import { isServerWithId, SelectedServer } from '../servers/data'; import TagBullet from './helpers/TagBullet'; -import { TagModalProps, TagStats } from './data'; +import { NormalizedTag, TagModalProps } from './data'; import './TagCard.scss'; export interface TagCardProps { - tag: string; - tagStats?: TagStats; + tag: NormalizedTag; selectedServer: SelectedServer; displayed: boolean; toggle: () => void; @@ -25,7 +24,7 @@ const TagCard = ( DeleteTagConfirmModal: FC, EditTagModal: FC, colorGenerator: ColorGenerator, -) => ({ tag, tagStats, selectedServer, displayed, toggle }: TagCardProps) => { +) => ({ tag, selectedServer, displayed, toggle }: TagCardProps) => { const [ isDeleteModalOpen, toggleDelete ] = useToggle(); const [ isEditModalOpen, toggleEdit ] = useToggle(); const [ hasTitle,, displayTitle ] = useToggle(); @@ -49,39 +48,37 @@ const TagCard = (
{ titleRef.current = el ?? undefined; }} > - - {tag} + + {tag.tag}
- {tagStats && ( - - - - Short URLs - {prettify(tagStats.shortUrlsCount)} - - - Visits - {prettify(tagStats.visitsCount)} - - - - )} + + + + Short URLs + {prettify(tag.shortUrls)} + + + Visits + {prettify(tag.visits)} + + + - - + + ); }; diff --git a/src/tags/TagsCards.tsx b/src/tags/TagsCards.tsx index 3c1408cb..9e6d0b54 100644 --- a/src/tags/TagsCards.tsx +++ b/src/tags/TagsCards.tsx @@ -7,10 +7,10 @@ import { TagsListChildrenProps } from './data/TagsListChildrenProps'; const { ceil } = Math; const TAGS_GROUPS_AMOUNT = 4; -export const TagsCards = (TagCard: FC): FC => ({ tagsList, selectedServer }) => { +export const TagsCards = (TagCard: FC): FC => ({ sortedTags, selectedServer }) => { const [ displayedTag, setDisplayedTag ] = useState(); - const tagsCount = tagsList.filteredTags.length; - const tagsGroups = splitEvery(ceil(tagsCount / TAGS_GROUPS_AMOUNT), tagsList.filteredTags); + const tagsCount = sortedTags.length; + const tagsGroups = splitEvery(ceil(tagsCount / TAGS_GROUPS_AMOUNT), sortedTags); return ( @@ -18,12 +18,11 @@ export const TagsCards = (TagCard: FC): FC
{group.map((tag) => ( setDisplayedTag(displayedTag !== tag ? tag : undefined)} + displayed={displayedTag === tag.tag} + toggle={() => setDisplayedTag(displayedTag !== tag.tag ? tag.tag : undefined)} /> ))}
diff --git a/src/tags/TagsList.tsx b/src/tags/TagsList.tsx index 4fdbf442..252ddfe2 100644 --- a/src/tags/TagsList.tsx +++ b/src/tags/TagsList.tsx @@ -1,5 +1,8 @@ -import { FC, useEffect, useState } from 'react'; +import { FC, useEffect, useMemo, useState } from 'react'; import { Row } from 'reactstrap'; +import { pipe } from 'ramda'; +import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'; +import { faCaretDown as caretDownIcon, faCaretUp as caretUpIcon } from '@fortawesome/free-solid-svg-icons'; import Message from '../utils/Message'; import SearchField from '../utils/SearchField'; import { SelectedServer } from '../servers/data'; @@ -8,9 +11,12 @@ import { Result } from '../utils/Result'; import { ShlinkApiError } from '../api/ShlinkApiError'; import { Topics } from '../mercure/helpers/Topics'; import { Settings, TagsMode } from '../settings/reducers/settings'; +import { determineOrderDir, sortList } from '../utils/helpers/ordering'; import { TagsList as TagsListState } from './reducers/tagsList'; -import { TagsListChildrenProps } from './data/TagsListChildrenProps'; +import { OrderableFields, TagsListChildrenProps, TagsOrder } from './data/TagsListChildrenProps'; import { TagsModeDropdown } from './TagsModeDropdown'; +import { NormalizedTag } from './data'; +import { TagsTableProps } from './TagsTable'; export interface TagsListProps { filterTags: (searchTerm: string) => void; @@ -20,10 +26,22 @@ export interface TagsListProps { settings: Settings; } -const TagsList = (TagsCards: FC, TagsTable: FC) => boundToMercureHub(( +const TagsList = (TagsCards: FC, TagsTable: FC) => boundToMercureHub(( { filterTags, forceListTags, tagsList, selectedServer, settings }: TagsListProps, ) => { const [ mode, setMode ] = useState(settings.ui?.tagsMode ?? 'cards'); + const [ order, setOrder ] = useState({}); + const sortedTags = useMemo( + pipe( + () => tagsList.filteredTags.map((tag): NormalizedTag => ({ + tag, + shortUrls: tagsList.stats[tag]?.shortUrlsCount ?? 0, + visits: tagsList.stats[tag]?.visitsCount ?? 0, + })), + (normalizedTags) => sortList(normalizedTags, order), + ), + [ tagsList.filteredTags, order ], + ); useEffect(() => { forceListTags(); @@ -33,6 +51,10 @@ const TagsList = (TagsCards: FC, TagsTable: FC; } + const orderByColumn = (field: OrderableFields) => + () => setOrder({ field, dir: determineOrderDir(field, order.field, order.dir) }); + const renderOrderIcon = (field: OrderableFields) => order.dir && order.field === field && + ; const renderContent = () => { if (tagsList.error) { return ( @@ -47,8 +69,15 @@ const TagsList = (TagsCards: FC, TagsTable: FC - : ; + ? + : ( + + ); }; return ( diff --git a/src/tags/TagsTable.tsx b/src/tags/TagsTable.tsx index e81d59c9..5ed1d861 100644 --- a/src/tags/TagsTable.tsx +++ b/src/tags/TagsTable.tsx @@ -1,54 +1,35 @@ -import { FC, useEffect, useMemo, useRef, useState } from 'react'; -import { pipe, splitEvery } from 'ramda'; +import { FC, ReactNode, useEffect, useRef } from 'react'; +import { splitEvery } from 'ramda'; import { RouteChildrenProps } from 'react-router'; -import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'; -import { faCaretDown as caretDownIcon, faCaretUp as caretUpIcon } from '@fortawesome/free-solid-svg-icons'; import { SimpleCard } from '../utils/SimpleCard'; import SimplePaginator from '../common/SimplePaginator'; import { useQueryState } from '../utils/helpers/hooks'; import { parseQuery } from '../utils/helpers/query'; -import { determineOrderDir, Order, sortList } from '../utils/helpers/ordering'; -import { TagsListChildrenProps } from './data/TagsListChildrenProps'; +import { OrderableFields, TagsListChildrenProps } from './data/TagsListChildrenProps'; import { TagsTableRowProps } from './TagsTableRow'; -import { NormalizedTag } from './data'; import './TagsTable.scss'; +export interface TagsTableProps extends TagsListChildrenProps { + orderByColumn: (field: OrderableFields) => () => void; + renderOrderIcon: (field: OrderableFields) => ReactNode; +} + const TAGS_PER_PAGE = 20; // TODO Allow customizing this value in settings -type OrderableFields = 'tag' | 'shortUrls' | 'visits'; -type TagsOrder = Order; - export const TagsTable = (TagsTableRow: FC) => ( - { tagsList, selectedServer, location }: TagsListChildrenProps & RouteChildrenProps, + { sortedTags, selectedServer, location, orderByColumn, renderOrderIcon }: TagsTableProps & RouteChildrenProps, ) => { const isFirstLoad = useRef(true); const { page: pageFromQuery = 1 } = parseQuery<{ page?: number | string }>(location.search); const [ page, setPage ] = useQueryState('page', Number(pageFromQuery)); - const [ order, setOrder ] = useState({}); - const sortedTags = useMemo( - pipe( - () => tagsList.filteredTags.map((tag): NormalizedTag => ({ - tag, - shortUrls: tagsList.stats[tag]?.shortUrlsCount ?? 0, - visits: tagsList.stats[tag]?.visitsCount ?? 0, - })), - (normalizedTags) => sortList(normalizedTags, order), - ), - [ tagsList.filteredTags, order ], - ); const pages = splitEvery(TAGS_PER_PAGE, sortedTags); const showPaginator = pages.length > 1; const currentPage = pages[page - 1] ?? []; - const orderByColumn = (field: OrderableFields) => - () => setOrder({ field, dir: determineOrderDir(field, order.field, order.dir) }); - const renderOrderIcon = (field: OrderableFields) => order.dir && order.field === field && - ; - useEffect(() => { !isFirstLoad.current && setPage(1); isFirstLoad.current = false; - }, [ tagsList.filteredTags ]); + }, [ sortedTags ]); useEffect(() => { scrollTo(0, 0); }, [ page ]); diff --git a/src/tags/data/TagsListChildrenProps.ts b/src/tags/data/TagsListChildrenProps.ts index 5777c290..17152d4d 100644 --- a/src/tags/data/TagsListChildrenProps.ts +++ b/src/tags/data/TagsListChildrenProps.ts @@ -1,7 +1,12 @@ -import { TagsList as TagsListState } from '../reducers/tagsList'; import { SelectedServer } from '../../servers/data'; +import { Order } from '../../utils/helpers/ordering'; +import { NormalizedTag } from './index'; + +export type OrderableFields = 'tag' | 'shortUrls' | 'visits'; + +export type TagsOrder = Order; export interface TagsListChildrenProps { - tagsList: TagsListState; + sortedTags: NormalizedTag[]; selectedServer: SelectedServer; } From 7a2d0e5dee6e022aa43e14a49076f7087d30f6db Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 6 Nov 2021 11:03:56 +0100 Subject: [PATCH 2/8] Added sorting dropdown for tags, that can be used regardless the display mode --- src/tags/TagsList.tsx | 14 ++++++++++++-- src/tags/data/TagsListChildrenProps.ts | 8 +++++++- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/src/tags/TagsList.tsx b/src/tags/TagsList.tsx index 252ddfe2..4f32f9bb 100644 --- a/src/tags/TagsList.tsx +++ b/src/tags/TagsList.tsx @@ -12,8 +12,9 @@ import { ShlinkApiError } from '../api/ShlinkApiError'; import { Topics } from '../mercure/helpers/Topics'; import { Settings, TagsMode } from '../settings/reducers/settings'; import { determineOrderDir, sortList } from '../utils/helpers/ordering'; +import SortingDropdown from '../utils/SortingDropdown'; import { TagsList as TagsListState } from './reducers/tagsList'; -import { OrderableFields, TagsListChildrenProps, TagsOrder } from './data/TagsListChildrenProps'; +import { OrderableFields, SORTABLE_FIELDS, TagsListChildrenProps, TagsOrder } from './data/TagsListChildrenProps'; import { TagsModeDropdown } from './TagsModeDropdown'; import { NormalizedTag } from './data'; import { TagsTableProps } from './TagsTable'; @@ -55,6 +56,7 @@ const TagsList = (TagsCards: FC, TagsTable: FC setOrder({ field, dir: determineOrderDir(field, order.field, order.dir) }); const renderOrderIcon = (field: OrderableFields) => order.dir && order.field === field && ; + const renderContent = () => { if (tagsList.error) { return ( @@ -84,9 +86,17 @@ const TagsList = (TagsCards: FC, TagsTable: FC -
+
+
+ setOrder({ field, dir })} + /> +
{renderContent()} diff --git a/src/tags/data/TagsListChildrenProps.ts b/src/tags/data/TagsListChildrenProps.ts index 17152d4d..e8e476a8 100644 --- a/src/tags/data/TagsListChildrenProps.ts +++ b/src/tags/data/TagsListChildrenProps.ts @@ -2,7 +2,13 @@ import { SelectedServer } from '../../servers/data'; import { Order } from '../../utils/helpers/ordering'; import { NormalizedTag } from './index'; -export type OrderableFields = 'tag' | 'shortUrls' | 'visits'; +export const SORTABLE_FIELDS = { + tag: 'Tag', + shortUrls: 'Short URLs', + visits: 'Visits', +}; + +export type OrderableFields = keyof typeof SORTABLE_FIELDS; export type TagsOrder = Order; From e6737ff1f2819e825f71d854134fc788c9aa4d51 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 6 Nov 2021 11:11:09 +0100 Subject: [PATCH 3/8] Moved logic to render sorting icon to tags list, as it's too specific --- src/tags/TagsList.tsx | 6 +----- src/tags/TagsTable.tsx | 12 ++++++++---- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/tags/TagsList.tsx b/src/tags/TagsList.tsx index 4f32f9bb..6b5656fc 100644 --- a/src/tags/TagsList.tsx +++ b/src/tags/TagsList.tsx @@ -1,8 +1,6 @@ import { FC, useEffect, useMemo, useState } from 'react'; import { Row } from 'reactstrap'; import { pipe } from 'ramda'; -import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'; -import { faCaretDown as caretDownIcon, faCaretUp as caretUpIcon } from '@fortawesome/free-solid-svg-icons'; import Message from '../utils/Message'; import SearchField from '../utils/SearchField'; import { SelectedServer } from '../servers/data'; @@ -54,8 +52,6 @@ const TagsList = (TagsCards: FC, TagsTable: FC () => setOrder({ field, dir: determineOrderDir(field, order.field, order.dir) }); - const renderOrderIcon = (field: OrderableFields) => order.dir && order.field === field && - ; const renderContent = () => { if (tagsList.error) { @@ -76,8 +72,8 @@ const TagsList = (TagsCards: FC, TagsTable: FC ); }; diff --git a/src/tags/TagsTable.tsx b/src/tags/TagsTable.tsx index 5ed1d861..e313ebd4 100644 --- a/src/tags/TagsTable.tsx +++ b/src/tags/TagsTable.tsx @@ -1,23 +1,25 @@ -import { FC, ReactNode, useEffect, useRef } from 'react'; +import { FC, useEffect, useRef } from 'react'; import { splitEvery } from 'ramda'; +import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'; +import { faCaretDown as caretDownIcon, faCaretUp as caretUpIcon } from '@fortawesome/free-solid-svg-icons'; import { RouteChildrenProps } from 'react-router'; import { SimpleCard } from '../utils/SimpleCard'; import SimplePaginator from '../common/SimplePaginator'; import { useQueryState } from '../utils/helpers/hooks'; import { parseQuery } from '../utils/helpers/query'; -import { OrderableFields, TagsListChildrenProps } from './data/TagsListChildrenProps'; +import { OrderableFields, TagsListChildrenProps, TagsOrder } from './data/TagsListChildrenProps'; import { TagsTableRowProps } from './TagsTableRow'; import './TagsTable.scss'; export interface TagsTableProps extends TagsListChildrenProps { orderByColumn: (field: OrderableFields) => () => void; - renderOrderIcon: (field: OrderableFields) => ReactNode; + currentOrder: TagsOrder; } const TAGS_PER_PAGE = 20; // TODO Allow customizing this value in settings export const TagsTable = (TagsTableRow: FC) => ( - { sortedTags, selectedServer, location, orderByColumn, renderOrderIcon }: TagsTableProps & RouteChildrenProps, + { sortedTags, selectedServer, location, orderByColumn, currentOrder }: TagsTableProps & RouteChildrenProps, ) => { const isFirstLoad = useRef(true); const { page: pageFromQuery = 1 } = parseQuery<{ page?: number | string }>(location.search); @@ -25,6 +27,8 @@ export const TagsTable = (TagsTableRow: FC) => ( const pages = splitEvery(TAGS_PER_PAGE, sortedTags); const showPaginator = pages.length > 1; const currentPage = pages[page - 1] ?? []; + const renderOrderIcon = (field: OrderableFields) => currentOrder.dir && currentOrder.field === field && + ; useEffect(() => { !isFirstLoad.current && setPage(1); From 765c4713a2784e5424af0d35c967f4ccc2579dbb Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 6 Nov 2021 11:30:42 +0100 Subject: [PATCH 4/8] Fixed all tests to work with new tags sorting approach --- src/tags/TagsList.tsx | 37 ++++++++++++++++++------------------ test/tags/TagCard.test.tsx | 7 +------ test/tags/TagsCards.test.tsx | 6 +++--- test/tags/TagsList.test.tsx | 4 ++++ test/tags/TagsTable.test.tsx | 30 +++++++++++------------------ 5 files changed, 37 insertions(+), 47 deletions(-) diff --git a/src/tags/TagsList.tsx b/src/tags/TagsList.tsx index 6b5656fc..4aae2063 100644 --- a/src/tags/TagsList.tsx +++ b/src/tags/TagsList.tsx @@ -1,4 +1,4 @@ -import { FC, useEffect, useMemo, useState } from 'react'; +import { FC, useEffect, useState } from 'react'; import { Row } from 'reactstrap'; import { pipe } from 'ramda'; import Message from '../utils/Message'; @@ -30,16 +30,13 @@ const TagsList = (TagsCards: FC, TagsTable: FC { const [ mode, setMode ] = useState(settings.ui?.tagsMode ?? 'cards'); const [ order, setOrder ] = useState({}); - const sortedTags = useMemo( - pipe( - () => tagsList.filteredTags.map((tag): NormalizedTag => ({ - tag, - shortUrls: tagsList.stats[tag]?.shortUrlsCount ?? 0, - visits: tagsList.stats[tag]?.visitsCount ?? 0, - })), - (normalizedTags) => sortList(normalizedTags, order), - ), - [ tagsList.filteredTags, order ], + const resolveSortedTags = pipe( + () => tagsList.filteredTags.map((tag): NormalizedTag => ({ + tag, + shortUrls: tagsList.stats[tag]?.shortUrlsCount ?? 0, + visits: tagsList.stats[tag]?.visitsCount ?? 0, + })), + (normalizedTags) => sortList(normalizedTags, order), ); useEffect(() => { @@ -50,22 +47,24 @@ const TagsList = (TagsCards: FC, TagsTable: FC; } + if (tagsList.error) { + return ( + + + + ); + } + const orderByColumn = (field: OrderableFields) => () => setOrder({ field, dir: determineOrderDir(field, order.field, order.dir) }); const renderContent = () => { - if (tagsList.error) { - return ( - - - - ); - } - if (tagsList.filteredTags.length < 1) { return No tags found; } + const sortedTags = resolveSortedTags(); + return mode === 'cards' ? : ( diff --git a/test/tags/TagCard.test.tsx b/test/tags/TagCard.test.tsx index 8bdd6c6c..a3c96b75 100644 --- a/test/tags/TagCard.test.tsx +++ b/test/tags/TagCard.test.tsx @@ -8,19 +8,14 @@ import { ReachableServer } from '../../src/servers/data'; describe('', () => { let wrapper: ShallowWrapper; - const tagStats = { - shortUrlsCount: 48, - visitsCount: 23257, - }; const DeleteTagConfirmModal = jest.fn(); const EditTagModal = jest.fn(); const TagCard = createTagCard(DeleteTagConfirmModal, EditTagModal, Mock.all()); const createWrapper = (tag = 'ssr') => { wrapper = shallow( ({ id: '1' })} - tagStats={tagStats} displayed={true} toggle={() => {}} />, diff --git a/test/tags/TagsCards.test.tsx b/test/tags/TagsCards.test.tsx index 8b5d8db3..e41e7d5e 100644 --- a/test/tags/TagsCards.test.tsx +++ b/test/tags/TagsCards.test.tsx @@ -1,19 +1,19 @@ import { shallow, ShallowWrapper } from 'enzyme'; import { Mock } from 'ts-mockery'; import { TagsCards as createTagsCards } from '../../src/tags/TagsCards'; -import { TagsList } from '../../src/tags/reducers/tagsList'; import { SelectedServer } from '../../src/servers/data'; import { rangeOf } from '../../src/utils/utils'; +import { NormalizedTag } from '../../src/tags/data'; describe('', () => { const amountOfTags = 10; - const tagsList = Mock.of({ filteredTags: rangeOf(amountOfTags, (i) => `tag_${i}`), stats: {} }); + const sortedTags = rangeOf(amountOfTags, (i) => Mock.of({ tag: `tag_${i}` })); const TagCard = () => null; const TagsCards = createTagsCards(TagCard); let wrapper: ShallowWrapper; beforeEach(() => { - wrapper = shallow(()} />); + wrapper = shallow(()} />); }); afterEach(() => wrapper?.unmount()); diff --git a/test/tags/TagsList.test.tsx b/test/tags/TagsList.test.tsx index 85932fb6..47618322 100644 --- a/test/tags/TagsList.test.tsx +++ b/test/tags/TagsList.test.tsx @@ -37,17 +37,21 @@ describe('', () => { it('shows a loading message when tags are being loaded', () => { const wrapper = createWrapper({ loading: true }); const loadingMsg = wrapper.find(Message); + const searchField = wrapper.find(SearchField); expect(loadingMsg).toHaveLength(1); expect(loadingMsg.html()).toContain('Loading...'); + expect(searchField).toHaveLength(0); }); it('shows an error when tags failed to be loaded', () => { const wrapper = createWrapper({ error: true }); const errorMsg = wrapper.find(Result).filterWhere((result) => result.prop('type') === 'error'); + const searchField = wrapper.find(SearchField); expect(errorMsg).toHaveLength(1); expect(errorMsg.html()).toContain('Error loading tags :('); + expect(searchField).toHaveLength(0); }); it('shows a message when the list of tags is empty', () => { diff --git a/test/tags/TagsTable.test.tsx b/test/tags/TagsTable.test.tsx index a1a2e52c..4d5dc344 100644 --- a/test/tags/TagsTable.test.tsx +++ b/test/tags/TagsTable.test.tsx @@ -4,24 +4,26 @@ import { match } from 'react-router'; import { Location, History } from 'history'; import { TagsTable as createTagsTable } from '../../src/tags/TagsTable'; import { SelectedServer } from '../../src/servers/data'; -import { TagsList } from '../../src/tags/reducers/tagsList'; import { rangeOf } from '../../src/utils/utils'; import SimplePaginator from '../../src/common/SimplePaginator'; import { NormalizedTag } from '../../src/tags/data'; describe('', () => { const TagsTableRow = () => null; + const orderByColumn = jest.fn(); const TagsTable = createTagsTable(TagsTableRow); const tags = (amount: number) => rangeOf(amount, (i) => `tag_${i}`); let wrapper: ShallowWrapper; - const createWrapper = (filteredTags: string[] = [], search = '') => { + const createWrapper = (sortedTags: string[] = [], search = '') => { wrapper = shallow( ({ stats: {}, filteredTags })} + sortedTags={sortedTags.map((tag) => Mock.of({ tag }))} selectedServer={Mock.all()} + currentOrder={{}} history={Mock.all()} location={Mock.of({ search })} match={Mock.all()} + orderByColumn={() => orderByColumn} />, ); @@ -33,6 +35,7 @@ describe('', () => { (global as any).history = { pushState: jest.fn() }; }); + afterEach(jest.clearAllMocks); afterEach(() => wrapper?.unmount()); it('renders empty result if there are no tags', () => { @@ -99,22 +102,11 @@ describe('', () => { it('orders tags when column is clicked', () => { const wrapper = createWrapper(tags(100)); - const firstRowText = () => (wrapper.find('tbody').find(TagsTableRow).first().prop('tag') as NormalizedTag).tag; - expect(firstRowText()).toEqual('tag_1'); - wrapper.find('thead').find('th').first().simulate('click'); // Tag column ASC - expect(firstRowText()).toEqual('tag_1'); - wrapper.find('thead').find('th').first().simulate('click'); // Tag column DESC - expect(firstRowText()).toEqual('tag_99'); - wrapper.find('thead').find('th').at(2).simulate('click'); // Visits column - ASC - expect(firstRowText()).toEqual('tag_100'); - wrapper.find('thead').find('th').at(2).simulate('click'); // Visits column - DESC - expect(firstRowText()).toEqual('tag_1'); - wrapper.find('thead').find('th').at(2).simulate('click'); // Visits column - reset - expect(firstRowText()).toEqual('tag_1'); - wrapper.find('thead').find('th').at(1).simulate('click'); // Short URLs column - ASC - expect(firstRowText()).toEqual('tag_100'); - wrapper.find('thead').find('th').at(1).simulate('click'); // Short URLs column - DESC - expect(firstRowText()).toEqual('tag_1'); + expect(orderByColumn).not.toHaveBeenCalled(); + wrapper.find('thead').find('th').first().simulate('click'); + wrapper.find('thead').find('th').at(2).simulate('click'); + wrapper.find('thead').find('th').at(1).simulate('click'); + expect(orderByColumn).toHaveBeenCalledTimes(3); }); }); From a6892b8a1248d24b95ba680174d51747148ffe22 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 6 Nov 2021 11:58:59 +0100 Subject: [PATCH 5/8] Covered ordering use cases on TagsList test --- test/tags/TagsList.test.tsx | 37 ++++++++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/test/tags/TagsList.test.tsx b/test/tags/TagsList.test.tsx index 47618322..86cf429e 100644 --- a/test/tags/TagsList.test.tsx +++ b/test/tags/TagsList.test.tsx @@ -9,6 +9,8 @@ import { Result } from '../../src/utils/Result'; import { TagsModeDropdown } from '../../src/tags/TagsModeDropdown'; import SearchField from '../../src/utils/SearchField'; import { Settings } from '../../src/settings/reducers/settings'; +import { OrderableFields } from '../../src/tags/data/TagsListChildrenProps'; +import SortingDropdown from '../../src/utils/SortingDropdown'; describe('', () => { let wrapper: ShallowWrapper; @@ -76,9 +78,42 @@ describe('', () => { it('triggers tags filtering when search field changes', () => { const wrapper = createWrapper({ filteredTags: [] }); + const searchField = wrapper.find(SearchField); + expect(searchField).toHaveLength(1); expect(filterTags).not.toHaveBeenCalled(); - wrapper.find(SearchField).simulate('change'); + searchField.simulate('change'); expect(filterTags).toHaveBeenCalledTimes(1); }); + + it('triggers ordering when sorting dropdown changes', () => { + const wrapper = createWrapper({ filteredTags: [] }); + + expect(wrapper.find(SortingDropdown).prop('orderField')).not.toBeDefined(); + expect(wrapper.find(SortingDropdown).prop('orderDir')).not.toBeDefined(); + wrapper.find(SortingDropdown).simulate('change', 'tag', 'DESC'); + expect(wrapper.find(SortingDropdown).prop('orderField')).toEqual('tag'); + expect(wrapper.find(SortingDropdown).prop('orderDir')).toEqual('DESC'); + wrapper.find(SortingDropdown).simulate('change', 'visits', 'ASC'); + expect(wrapper.find(SortingDropdown).prop('orderField')).toEqual('visits'); + expect(wrapper.find(SortingDropdown).prop('orderDir')).toEqual('ASC'); + }); + + it('can update current order via orderByColumn from table component', () => { + const wrapper = createWrapper({ filteredTags: [ 'foo', 'bar' ], stats: {} }); + const callOrderBy = (field: OrderableFields) => { + ((wrapper.find(TagsTable).prop('orderByColumn') as Function)(field) as Function)(); + }; + + wrapper.find(TagsModeDropdown).simulate('change'); // Make sure table is rendered + + callOrderBy('visits'); + expect(wrapper.find(TagsTable).prop('currentOrder')).toEqual({ field: 'visits', dir: 'ASC' }); + callOrderBy('visits'); + expect(wrapper.find(TagsTable).prop('currentOrder')).toEqual({ field: 'visits', dir: 'DESC' }); + callOrderBy('tag'); + expect(wrapper.find(TagsTable).prop('currentOrder')).toEqual({ field: 'tag', dir: 'ASC' }); + callOrderBy('shortUrls'); + expect(wrapper.find(TagsTable).prop('currentOrder')).toEqual({ field: 'shortUrls', dir: 'ASC' }); + }); }); From 0bb5c7d8af819c3764fd35880a669e85e64f2bb0 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 6 Nov 2021 12:04:26 +0100 Subject: [PATCH 6/8] Simplified branches while resolving server Id --- src/common/AsideMenu.tsx | 5 +++-- src/servers/Overview.tsx | 4 ++-- src/tags/TagCard.tsx | 4 ++-- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/common/AsideMenu.tsx b/src/common/AsideMenu.tsx index a301d4ef..9c086e27 100644 --- a/src/common/AsideMenu.tsx +++ b/src/common/AsideMenu.tsx @@ -40,7 +40,8 @@ const AsideMenuItem: FC = ({ children, to, className, ...res const AsideMenu = (DeleteServerButton: FC) => ( { selectedServer, showOnMobile = false }: AsideMenuProps, ) => { - const serverId = isServerWithId(selectedServer) ? selectedServer.id : ''; + const hasId = isServerWithId(selectedServer); + const serverId = hasId ? selectedServer.id : ''; const addManageDomainsLink = supportsDomainRedirects(selectedServer); const asideClass = classNames('aside-menu', { 'aside-menu--hidden': !showOnMobile, @@ -77,7 +78,7 @@ const AsideMenu = (DeleteServerButton: FC) => ( Edit this server - {isServerWithId(selectedServer) && ( + {hasId && ( { diff --git a/src/tags/TagCard.tsx b/src/tags/TagCard.tsx index df0fd26a..482a998c 100644 --- a/src/tags/TagCard.tsx +++ b/src/tags/TagCard.tsx @@ -6,7 +6,7 @@ import { Link } from 'react-router-dom'; import { prettify } from '../utils/helpers/numbers'; import { useToggle } from '../utils/helpers/hooks'; import ColorGenerator from '../utils/services/ColorGenerator'; -import { isServerWithId, SelectedServer } from '../servers/data'; +import { getServerId, SelectedServer } from '../servers/data'; import TagBullet from './helpers/TagBullet'; import { NormalizedTag, TagModalProps } from './data'; import './TagCard.scss'; @@ -29,7 +29,7 @@ const TagCard = ( const [ isEditModalOpen, toggleEdit ] = useToggle(); const [ hasTitle,, displayTitle ] = useToggle(); const titleRef = useRef(); - const serverId = isServerWithId(selectedServer) ? selectedServer.id : ''; + const serverId = getServerId(selectedServer); useEffect(() => { if (isTruncated(titleRef.current)) { From 7169c6e083954b75fe43e8cde9440ab90892d084 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 6 Nov 2021 12:05:49 +0100 Subject: [PATCH 7/8] Updated changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 808fbb81..33b49f27 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), * [#508](https://github.com/shlinkio/shlink-web-client/issues/508) Added new servers management section. * [#490](https://github.com/shlinkio/shlink-web-client/issues/490) Now a server can be marked as auto-connect, skipping home screen when that happens. * [#492](https://github.com/shlinkio/shlink-web-client/issues/492) Improved tags table, by supporting sorting by column and making the header sticky. +* [#515](https://github.com/shlinkio/shlink-web-client/issues/515) Allowed to sort tags even when using the cards display mode. ### Changed * Moved ci workflow to external repo and reused From ee826458becac5c5e94d9d741eea194a830c17bf Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 6 Nov 2021 12:26:20 +0100 Subject: [PATCH 8/8] Updated sorting dropdown to accept an order object instead of two individual props --- src/short-urls/ShortUrlsList.tsx | 7 +------ src/tags/TagsList.tsx | 14 ++++++-------- src/utils/SortingDropdown.tsx | 21 ++++++++++----------- src/visits/charts/SortableBarChartCard.tsx | 20 ++++++++------------ test/short-urls/ShortUrlsList.test.tsx | 20 ++++++-------------- test/tags/TagsList.test.tsx | 9 +++------ test/utils/SortingDropdown.test.tsx | 14 +++++++------- 7 files changed, 41 insertions(+), 64 deletions(-) diff --git a/src/short-urls/ShortUrlsList.tsx b/src/short-urls/ShortUrlsList.tsx index e6a5eedc..8f3ed9e2 100644 --- a/src/short-urls/ShortUrlsList.tsx +++ b/src/short-urls/ShortUrlsList.tsx @@ -67,12 +67,7 @@ const ShortUrlsList = (ShortUrlsTable: FC) => boundToMercur return ( <>
- +
, TagsTable: FC - () => setOrder({ field, dir: determineOrderDir(field, order.field, order.dir) }); + const orderByColumn = (field: OrderableFields) => () => { + const dir = determineOrderDir(field, order.field, order.dir); + + setOrder({ field: dir ? field : undefined, dir }); + }; const renderContent = () => { if (tagsList.filteredTags.length < 1) { @@ -85,12 +88,7 @@ const TagsList = (TagsCards: FC, TagsTable: FC
- setOrder({ field, dir })} - /> + setOrder({ field, dir })} />
{renderContent()} diff --git a/src/utils/SortingDropdown.tsx b/src/utils/SortingDropdown.tsx index bba0bed7..ebc04d2c 100644 --- a/src/utils/SortingDropdown.tsx +++ b/src/utils/SortingDropdown.tsx @@ -3,23 +3,22 @@ import { toPairs } from 'ramda'; import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'; import { faSortAmountUp as sortAscIcon, faSortAmountDown as sortDescIcon } from '@fortawesome/free-solid-svg-icons'; import classNames from 'classnames'; -import { determineOrderDir, OrderDir } from './helpers/ordering'; +import { determineOrderDir, Order, OrderDir } from './helpers/ordering'; import './SortingDropdown.scss'; export interface SortingDropdownProps { items: Record; - orderField?: T; - orderDir?: OrderDir; + order: Order; onChange: (orderField?: T, orderDir?: OrderDir) => void; isButton?: boolean; right?: boolean; } export default function SortingDropdown( - { items, orderField, orderDir, onChange, isButton = true, right = false }: SortingDropdownProps, + { items, order, onChange, isButton = true, right = false }: SortingDropdownProps, ) { const handleItemClick = (fieldKey: T) => () => { - const newOrderDir = determineOrderDir(fieldKey, orderField, orderDir); + const newOrderDir = determineOrderDir(fieldKey, order.field, order.dir); onChange(newOrderDir ? fieldKey : undefined, newOrderDir); }; @@ -32,26 +31,26 @@ export default function SortingDropdown( className={classNames({ 'dropdown-btn__toggle btn-block': isButton, 'btn-sm p-0': !isButton })} > {!isButton && <>Order by} - {isButton && !orderField && <>Order by...} - {isButton && orderField && `Order by: "${items[orderField]}" - "${orderDir ?? 'DESC'}"`} + {isButton && !order.field && <>Order by...} + {isButton && order.field && `Order by: "${items[order.field]}" - "${order.dir ?? 'DESC'}"`} {toPairs(items).map(([ fieldKey, fieldValue ]) => ( - + {fieldValue} - {orderField === fieldKey && ( + {order.field === fieldKey && ( )} ))} - onChange()}> + onChange()}> Clear selection diff --git a/src/visits/charts/SortableBarChartCard.tsx b/src/visits/charts/SortableBarChartCard.tsx index b300af18..a627e514 100644 --- a/src/visits/charts/SortableBarChartCard.tsx +++ b/src/visits/charts/SortableBarChartCard.tsx @@ -1,7 +1,7 @@ import { FC, useState } from 'react'; import { fromPairs, pipe, reverse, sortBy, splitEvery, toLower, toPairs, type, zipObj } from 'ramda'; import { rangeOf } from '../../utils/utils'; -import { OrderDir } from '../../utils/helpers/ordering'; +import { Order } from '../../utils/helpers/ordering'; import SimplePaginator from '../../common/SimplePaginator'; import { roundTen } from '../../utils/helpers/numbers'; import SortingDropdown from '../../utils/SortingDropdown'; @@ -30,24 +30,21 @@ export const SortableBarChartCard: FC = ({ withPagination = true, ...rest }) => { - const [ order, setOrder ] = useState<{ orderField?: string; orderDir?: OrderDir }>({ - orderField: undefined, - orderDir: undefined, - }); + const [ order, setOrder ] = useState>({}); const [ currentPage, setCurrentPage ] = useState(1); const [ itemsPerPage, setItemsPerPage ] = useState(50); const getSortedPairsForStats = (stats: Stats, sortingItems: Record) => { const pairs = toPairs(stats); - const sortedPairs = !order.orderField ? pairs : sortBy( + const sortedPairs = !order.field ? pairs : sortBy( pipe( - order.orderField === Object.keys(sortingItems)[0] ? pickKeyFromPair : pickValueFromPair, + order.field === Object.keys(sortingItems)[0] ? pickKeyFromPair : pickValueFromPair, toLowerIfString, ), pairs, ); - return !order.orderDir || order.orderDir === 'ASC' ? sortedPairs : reverse(sortedPairs); + return !order.dir || order.dir === 'ASC' ? sortedPairs : reverse(sortedPairs); }; const determineCurrentPagePairs = (pages: StatsRow[][]): StatsRow[] => { const page = pages[currentPage - 1]; @@ -103,10 +100,9 @@ export const SortableBarChartCard: FC = ({ isButton={false} right items={sortingItems} - orderField={order.orderField} - orderDir={order.orderDir} - onChange={(orderField, orderDir) => { - setOrder({ orderField, orderDir }); + order={order} + onChange={(field, dir) => { + setOrder({ field, dir }); setCurrentPage(1); }} /> diff --git a/test/short-urls/ShortUrlsList.test.tsx b/test/short-urls/ShortUrlsList.test.tsx index 09ff55d4..e000a09f 100644 --- a/test/short-urls/ShortUrlsList.test.tsx +++ b/test/short-urls/ShortUrlsList.test.tsx @@ -108,23 +108,16 @@ describe('', () => { }); it('handles order by through dropdown', () => { - expect(wrapper.find(SortingDropdown).prop('orderField')).not.toBeDefined(); - expect(wrapper.find(SortingDropdown).prop('orderDir')).not.toBeDefined(); + expect(wrapper.find(SortingDropdown).prop('order')).toEqual({}); wrapper.find(SortingDropdown).simulate('change', 'visits', 'ASC'); - - expect(wrapper.find(SortingDropdown).prop('orderField')).toEqual('visits'); - expect(wrapper.find(SortingDropdown).prop('orderDir')).toEqual('ASC'); + expect(wrapper.find(SortingDropdown).prop('order')).toEqual({ field: 'visits', dir: 'ASC' }); wrapper.find(SortingDropdown).simulate('change', 'shortCode', 'DESC'); - - expect(wrapper.find(SortingDropdown).prop('orderField')).toEqual('shortCode'); - expect(wrapper.find(SortingDropdown).prop('orderDir')).toEqual('DESC'); + expect(wrapper.find(SortingDropdown).prop('order')).toEqual({ field: 'shortCode', dir: 'DESC' }); wrapper.find(SortingDropdown).simulate('change', undefined, undefined); - - expect(wrapper.find(SortingDropdown).prop('orderField')).toEqual(undefined); - expect(wrapper.find(SortingDropdown).prop('orderDir')).toEqual(undefined); + expect(wrapper.find(SortingDropdown).prop('order')).toEqual({}); expect(listShortUrlsMock).toHaveBeenCalledTimes(3); expect(listShortUrlsMock).toHaveBeenNthCalledWith(1, expect.objectContaining({ @@ -140,10 +133,9 @@ describe('', () => { [ Mock.of({ visits: 'ASC' }), 'visits', 'ASC' ], [ Mock.of({ title: 'DESC' }), 'title', 'DESC' ], [ Mock.of(), undefined, undefined ], - ])('has expected initial ordering', (initialOrderBy, expectedField, expectedDir) => { + ])('has expected initial ordering', (initialOrderBy, field, dir) => { const wrapper = createWrapper(initialOrderBy); - expect(wrapper.find(SortingDropdown).prop('orderField')).toEqual(expectedField); - expect(wrapper.find(SortingDropdown).prop('orderDir')).toEqual(expectedDir); + expect(wrapper.find(SortingDropdown).prop('order')).toEqual({ field, dir }); }); }); diff --git a/test/tags/TagsList.test.tsx b/test/tags/TagsList.test.tsx index 86cf429e..4c5848b6 100644 --- a/test/tags/TagsList.test.tsx +++ b/test/tags/TagsList.test.tsx @@ -89,14 +89,11 @@ describe('', () => { it('triggers ordering when sorting dropdown changes', () => { const wrapper = createWrapper({ filteredTags: [] }); - expect(wrapper.find(SortingDropdown).prop('orderField')).not.toBeDefined(); - expect(wrapper.find(SortingDropdown).prop('orderDir')).not.toBeDefined(); + expect(wrapper.find(SortingDropdown).prop('order')).toEqual({}); wrapper.find(SortingDropdown).simulate('change', 'tag', 'DESC'); - expect(wrapper.find(SortingDropdown).prop('orderField')).toEqual('tag'); - expect(wrapper.find(SortingDropdown).prop('orderDir')).toEqual('DESC'); + expect(wrapper.find(SortingDropdown).prop('order')).toEqual({ field: 'tag', dir: 'DESC' }); wrapper.find(SortingDropdown).simulate('change', 'visits', 'ASC'); - expect(wrapper.find(SortingDropdown).prop('orderField')).toEqual('visits'); - expect(wrapper.find(SortingDropdown).prop('orderDir')).toEqual('ASC'); + expect(wrapper.find(SortingDropdown).prop('order')).toEqual({ field: 'visits', dir: 'ASC' }); }); it('can update current order via orderByColumn from table component', () => { diff --git a/test/utils/SortingDropdown.test.tsx b/test/utils/SortingDropdown.test.tsx index 518b5f12..934bbfb7 100644 --- a/test/utils/SortingDropdown.test.tsx +++ b/test/utils/SortingDropdown.test.tsx @@ -14,7 +14,7 @@ describe('', () => { baz: 'Hello World', }; const createWrapper = (props: Partial = {}) => { - wrapper = shallow(); + wrapper = shallow(); return wrapper; }; @@ -34,7 +34,7 @@ describe('', () => { }); it('properly marks selected field as active with proper icon', () => { - const wrapper = createWrapper({ orderField: 'bar', orderDir: 'DESC' }); + const wrapper = createWrapper({ order: { field: 'bar', dir: 'DESC' } }); const activeItem = wrapper.find('DropdownItem[active=true]'); const activeItemIcon = activeItem.first().find(FontAwesomeIcon); @@ -55,7 +55,7 @@ describe('', () => { it('triggers change function when item is clicked and an order field was provided', () => { const onChange = jest.fn(); - const wrapper = createWrapper({ onChange, orderField: 'baz', orderDir: 'ASC' }); + const wrapper = createWrapper({ onChange, order: { field: 'baz', dir: 'ASC' } }); const firstItem = wrapper.find(DropdownItem).first(); firstItem.simulate('click'); @@ -66,7 +66,7 @@ describe('', () => { it('updates order dir when already selected item is clicked', () => { const onChange = jest.fn(); - const wrapper = createWrapper({ onChange, orderField: 'foo', orderDir: 'ASC' }); + const wrapper = createWrapper({ onChange, order: { field: 'foo', dir: 'ASC' } }); const firstItem = wrapper.find(DropdownItem).first(); firstItem.simulate('click'); @@ -79,14 +79,14 @@ describe('', () => { [{ isButton: false }, <>Order by ], [{ isButton: true }, <>Order by... ], [ - { isButton: true, orderField: 'foo', orderDir: 'ASC' as OrderDir }, + { isButton: true, order: { field: 'foo', dir: 'ASC' as OrderDir } }, 'Order by: "Foo" - "ASC"', ], [ - { isButton: true, orderField: 'baz', orderDir: 'DESC' as OrderDir }, + { isButton: true, order: { field: 'baz', dir: 'DESC' as OrderDir } }, 'Order by: "Hello World" - "DESC"', ], - [{ isButton: true, orderField: 'baz' }, 'Order by: "Hello World" - "DESC"' ], + [{ isButton: true, order: { field: 'baz' } }, 'Order by: "Hello World" - "DESC"' ], ])('displays expected text in toggle', (props, expectedText) => { const wrapper = createWrapper(props); const toggle = wrapper.find(DropdownToggle);