From 5cf0c86a143b411df620df9aad595206309559c0 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 21 Dec 2020 09:22:13 +0100 Subject: [PATCH 1/7] Normalized Message component, making it autocontained --- src/servers/helpers/ServerError.tsx | 20 +++++----- src/servers/helpers/withSelectedServer.tsx | 2 +- src/tags/TagsList.tsx | 16 +++----- src/utils/Message.tsx | 29 +++++++------- src/visits/VisitsStats.tsx | 44 ++++++++++++---------- test/visits/VisitsStats.test.tsx | 6 +-- 6 files changed, 55 insertions(+), 62 deletions(-) diff --git a/src/servers/helpers/ServerError.tsx b/src/servers/helpers/ServerError.tsx index 17d14be6..504bb610 100644 --- a/src/servers/helpers/ServerError.tsx +++ b/src/servers/helpers/ServerError.tsx @@ -17,17 +17,15 @@ export const ServerError = (DeleteServerButton: FC): FC ) => (
-
- - {!isServerWithId(selectedServer) && 'Could not find this Shlink server.'} - {isServerWithId(selectedServer) && ( - <> -

Oops! Could not connect to this Shlink server.

- Make sure you have internet connection, and the server is properly configured and on-line. - - )} - -
+ + {!isServerWithId(selectedServer) && 'Could not find this Shlink server.'} + {isServerWithId(selectedServer) && ( + <> +

Oops! Could not connect to this Shlink server.

+ Make sure you have internet connection, and the server is properly configured and on-line. + + )} + These are the Shlink servers currently configured. Choose one of diff --git a/src/servers/helpers/withSelectedServer.tsx b/src/servers/helpers/withSelectedServer.tsx index 742bf160..2c91d344 100644 --- a/src/servers/helpers/withSelectedServer.tsx +++ b/src/servers/helpers/withSelectedServer.tsx @@ -20,7 +20,7 @@ export function withSelectedServer(WrappedComponent: FC - + ); } diff --git a/src/tags/TagsList.tsx b/src/tags/TagsList.tsx index 03b4d221..9d1cb38a 100644 --- a/src/tags/TagsList.tsx +++ b/src/tags/TagsList.tsx @@ -28,15 +28,11 @@ const TagsList = (TagCard: FC) => boundToMercureHub(( const renderContent = () => { if (tagsList.loading) { - return ; + return ; } if (tagsList.error) { - return ( -
-
Error loading tags :(
-
- ); + return
Error loading tags :(
; } const tagsCount = tagsList.filteredTags.length; @@ -48,7 +44,7 @@ const TagsList = (TagCard: FC) => boundToMercureHub(( const tagsGroups = splitEvery(ceil(tagsCount / TAGS_GROUPS_AMOUNT), tagsList.filteredTags); return ( - <> +
{tagsGroups.map((group, index) => (
{group.map((tag) => ( @@ -63,16 +59,14 @@ const TagsList = (TagCard: FC) => boundToMercureHub(( ))}
))} - +
); }; return ( <> {!tagsList.loading && } -
- {renderContent()} -
+ {renderContent()} ); }, () => 'https://shlink.io/new-visit'); diff --git a/src/utils/Message.tsx b/src/utils/Message.tsx index 70363983..1eb9c36d 100644 --- a/src/utils/Message.tsx +++ b/src/utils/Message.tsx @@ -1,5 +1,5 @@ import { FC } from 'react'; -import { Card } from 'reactstrap'; +import { Card, Row } from 'reactstrap'; import classNames from 'classnames'; import { faCircleNotch as preloader } from '@fortawesome/free-solid-svg-icons'; import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'; @@ -24,31 +24,30 @@ const getTextClassForType = (type: MessageType) => { }; interface MessageProps { - noMargin?: boolean; + className?: string; loading?: boolean; fullWidth?: boolean; type?: MessageType; } -const Message: FC = ( - { children, loading = false, noMargin = false, type = 'default', fullWidth = false }, -) => { - const cardClasses = classNames(getClassForType(type), { 'mt-4': !noMargin }); +const Message: FC = ({ className, children, loading = false, type = 'default', fullWidth = false }) => { const classes = classNames({ 'col-md-12': fullWidth, 'col-md-10 offset-md-1': !fullWidth, }); return ( -
- -

- {loading && } - {loading && {children ?? 'Loading...'}} - {!loading && children} -

- -
+ +
+ +

+ {loading && } + {loading && {children ?? 'Loading...'}} + {!loading && children} +

+ +
+ ); }; diff --git a/src/visits/VisitsStats.tsx b/src/visits/VisitsStats.tsx index f1b9936a..55dc9ed4 100644 --- a/src/visits/VisitsStats.tsx +++ b/src/visits/VisitsStats.tsx @@ -28,10 +28,16 @@ export interface VisitsStatsProps { domain?: string; } +interface VisitsNavLinkProps { + title: string; + subPath: string; + icon: IconDefinition; +} + type HighlightableProps = 'referer' | 'country' | 'city'; type Section = 'byTime' | 'byContext' | 'byLocation' | 'list'; -const sections: Record = { +const sections: Record = { byTime: { title: 'By time', subPath: '', icon: faCalendarAlt }, byContext: { title: 'By context', subPath: '/by-context', icon: faChartPie }, byLocation: { title: 'By location', subPath: '/by-location', icon: faMapMarkedAlt }, @@ -53,6 +59,19 @@ const highlightedVisitsToStats = ( let selectedBar: string | undefined; const initialInterval: DateInterval = 'last30Days'; +const VisitsNavLink: FC = ({ subPath, title, icon, children }) => ( + pathname.endsWith(`/visits${subPath}`)} + replace + > + + {title} + +); + const VisitsStats: FC = ({ children, visitsInfo, getVisits, cancelGetVisits, baseUrl, domain }) => { const [ dateRange, setDateRange ] = useState(intervalToDateRange(initialInterval)); const [ highlightedVisits, setHighlightedVisits ] = useState([]); @@ -112,7 +131,7 @@ const VisitsStats: FC = ({ children, visitsInfo, getVisits, ca if (error) { return ( - + An error occurred while loading visits :( ); @@ -124,23 +143,10 @@ const VisitsStats: FC = ({ children, visitsInfo, getVisits, ca return ( <> - +
@@ -259,7 +265,7 @@ const VisitsStats: FC = ({ children, visitsInfo, getVisits, ca
-
+
{renderVisitsContent()}
diff --git a/test/visits/VisitsStats.test.tsx b/test/visits/VisitsStats.test.tsx index f5f00952..ba79f7bf 100644 --- a/test/visits/VisitsStats.test.tsx +++ b/test/visits/VisitsStats.test.tsx @@ -1,5 +1,5 @@ import { shallow, ShallowWrapper } from 'enzyme'; -import { Card, NavLink, Progress } from 'reactstrap'; +import { Card, Progress } from 'reactstrap'; import { Mock } from 'ts-mockery'; import VisitStats from '../../src/visits/VisitsStats'; import Message from '../../src/utils/Message'; @@ -80,10 +80,6 @@ describe('', () => { it('holds the map button content generator on cities graph extraHeaderContent', () => { const wrapper = createComponent({ loading: false, error: false, visits }); - const locationNav = wrapper.find(NavLink).at(2); - - locationNav.simulate('click'); - const citiesGraph = wrapper.find(SortableBarGraph).find('[title="Cities"]'); const extraHeaderContent = citiesGraph.prop('extraHeaderContent'); From c25355c53181f85bbd9d473b092077665fd01851 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 21 Dec 2020 09:57:46 +0100 Subject: [PATCH 2/7] Added Message test --- src/utils/Message.tsx | 2 +- test/utils/Message.test.tsx | 64 +++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 1 deletion(-) create mode 100644 test/utils/Message.test.tsx diff --git a/src/utils/Message.tsx b/src/utils/Message.tsx index 1eb9c36d..51f4305a 100644 --- a/src/utils/Message.tsx +++ b/src/utils/Message.tsx @@ -23,7 +23,7 @@ const getTextClassForType = (type: MessageType) => { return map[type]; }; -interface MessageProps { +export interface MessageProps { className?: string; loading?: boolean; fullWidth?: boolean; diff --git a/test/utils/Message.test.tsx b/test/utils/Message.test.tsx new file mode 100644 index 00000000..036a6b49 --- /dev/null +++ b/test/utils/Message.test.tsx @@ -0,0 +1,64 @@ +import { shallow, ShallowWrapper } from 'enzyme'; +import { PropsWithChildren } from 'react'; +import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'; +import { Card } from 'reactstrap'; +import Message, { MessageProps } from '../../src/utils/Message'; + +describe('', () => { + let wrapper: ShallowWrapper; + const createWrapper = (props: PropsWithChildren = {}) => { + wrapper = shallow(); + + return wrapper; + }; + + afterEach(() => wrapper?.unmount()); + + it.each([ + [ true, 1, 0 ], + [ false, 0, 1 ], + [ undefined, 0, 1 ], + ])('renders expected classes based on width', (fullWidth, expectedFull, expectedNonFull) => { + const wrapper = createWrapper({ fullWidth }); + + expect(wrapper.find('.col-md-12')).toHaveLength(expectedFull); + expect(wrapper.find('.col-md-10')).toHaveLength(expectedNonFull); + }); + + it.each([ + [ true, 'These are the children contents' ], + [ false, 'These are the children contents' ], + [ true, undefined ], + [ false, undefined ], + ])('renders expected content', (loading, children) => { + const wrapper = createWrapper({ loading, children }); + + expect(wrapper.find(FontAwesomeIcon)).toHaveLength(loading ? 1 : 0); + + if (loading) { + expect(wrapper.find('span').text()).toContain(children ? children : 'Loading...'); + } else { + expect(wrapper.find('span')).toHaveLength(0); + expect(wrapper.find('h3').text()).toContain(children ? children : ''); + } + }); + + it.each([ + [ 'error', 'border-danger', 'text-danger' ], + [ 'default', '', 'text-muted' ], + [ undefined, '', 'text-muted' ], + ])('renders proper classes based on message type', (type, expectedCardClass, expectedH3Class) => { + const wrapper = createWrapper({ type: type as 'default' | 'error' | undefined }); + const card = wrapper.find(Card); + const h3 = wrapper.find('h3'); + + expect(card.prop('className')).toEqual(expectedCardClass); + expect(h3.prop('className')).toEqual(`text-center mb-0 ${expectedH3Class}`); + }); + + it.each([{ className: 'foo' }, { className: 'bar' }, {}])('renders provided classes', ({ className }) => { + const wrapper = createWrapper({ className }); + + expect(wrapper.prop('className')).toEqual(className); + }); +}); From b211a29fc5f29087c3a68db4e3d6672309b7693c Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 21 Dec 2020 17:54:05 +0100 Subject: [PATCH 3/7] Created new Result component to display operation result messages consistently --- src/servers/CreateServer.tsx | 28 ++++++------- .../helpers/CreateShortUrlResult.tsx | 42 +++++++++---------- .../helpers/DeleteShortUrlModal.tsx | 12 +++--- src/short-urls/helpers/EditMetaModal.tsx | 5 ++- src/short-urls/helpers/EditShortUrlModal.tsx | 5 ++- src/short-urls/helpers/EditTagsModal.tsx | 5 ++- src/tags/TagsList.tsx | 3 +- src/tags/helpers/DeleteTagConfirmModal.tsx | 5 ++- src/tags/helpers/EditTagModal.tsx | 5 ++- src/utils/Result.tsx | 30 +++++++++++++ src/utils/SimpleCard.tsx | 5 ++- src/visits/VisitsStats.tsx | 7 +--- test/servers/CreateServer.test.tsx | 6 +-- .../helpers/CreateShortUrlResult.test.tsx | 3 +- .../helpers/DeleteShortUrlModal.test.tsx | 5 ++- .../short-urls/helpers/EditMetaModal.test.tsx | 5 ++- .../helpers/EditShortUrlModal.test.tsx | 3 +- test/tags/TagsList.test.tsx | 3 +- test/visits/VisitsStats.test.tsx | 5 ++- 19 files changed, 110 insertions(+), 72 deletions(-) create mode 100644 src/utils/Result.tsx diff --git a/src/servers/CreateServer.tsx b/src/servers/CreateServer.tsx index 7181344b..a04ccba8 100644 --- a/src/servers/CreateServer.tsx +++ b/src/servers/CreateServer.tsx @@ -1,7 +1,7 @@ import { FC } from 'react'; import { v4 as uuid } from 'uuid'; import { RouterProps } from 'react-router'; -import classNames from 'classnames'; +import { Result } from '../utils/Result'; import NoMenuLayout from '../common/NoMenuLayout'; import { StateFlagTimeout } from '../utils/helpers/hooks'; import { ServerForm } from './helpers/ServerForm'; @@ -15,19 +15,11 @@ interface CreateServerProps extends RouterProps { createServer: (server: ServerWithId) => void; } -const Result: FC<{ type: 'success' | 'error' }> = ({ children, type }) => ( -
-
-
- {children} -
-
-
+const ImportResult = ({ type }: { type: 'error' | 'success' }) => ( + + {type === 'success' && 'Servers properly imported. You can now select one from the list :)'} + {type === 'error' && 'The servers could not be imported. Make sure the format is correct.'} + ); const CreateServer = (ImportServersBtn: FC, useStateFlagTimeout: StateFlagTimeout) => ( @@ -49,8 +41,12 @@ const CreateServer = (ImportServersBtn: FC, useStateFlagT - {serversImported && Servers properly imported. You can now select one from the list :)} - {errorImporting && The servers could not be imported. Make sure the format is correct.} + {(serversImported || errorImporting) && ( +
+ {serversImported && } + {errorImporting && } +
+ )} ); }; diff --git a/src/short-urls/helpers/CreateShortUrlResult.tsx b/src/short-urls/helpers/CreateShortUrlResult.tsx index a9134e7c..ffe3d3e9 100644 --- a/src/short-urls/helpers/CreateShortUrlResult.tsx +++ b/src/short-urls/helpers/CreateShortUrlResult.tsx @@ -4,10 +4,11 @@ import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'; import { isNil } from 'ramda'; import { useEffect } from 'react'; import CopyToClipboard from 'react-copy-to-clipboard'; -import { Card, CardBody, Tooltip } from 'reactstrap'; +import { Tooltip } from 'reactstrap'; import { ShortUrlCreation } from '../reducers/shortUrlCreation'; import { StateFlagTimeout } from '../../utils/helpers/hooks'; import './CreateShortUrlResult.scss'; +import { Result } from '../../utils/Result'; export interface CreateShortUrlResultProps extends ShortUrlCreation { resetCreateShortUrl: () => void; @@ -25,9 +26,10 @@ const CreateShortUrlResult = (useStateFlagTimeout: StateFlagTimeout) => ( if (error) { return ( - + + {canBeClosed && } An error occurred while creating the URL :( - + ); } @@ -38,26 +40,24 @@ const CreateShortUrlResult = (useStateFlagTimeout: StateFlagTimeout) => ( const { shortUrl } = result; return ( - - - {canBeClosed && } - Great! The short URL is {shortUrl} + + {canBeClosed && } + Great! The short URL is {shortUrl} - - - + + + - - Copied! - - - + + Copied! + + ); }; diff --git a/src/short-urls/helpers/DeleteShortUrlModal.tsx b/src/short-urls/helpers/DeleteShortUrlModal.tsx index 20b16709..8e4c9f21 100644 --- a/src/short-urls/helpers/DeleteShortUrlModal.tsx +++ b/src/short-urls/helpers/DeleteShortUrlModal.tsx @@ -4,6 +4,7 @@ import { identity, pipe } from 'ramda'; import { ShortUrlDeletion } from '../reducers/shortUrlDeletion'; import { ShortUrlModalProps } from '../data'; import { handleEventPreventingDefault, OptionalString } from '../../utils/utils'; +import { Result } from '../../utils/Result'; const THRESHOLD_REACHED = 'INVALID_SHORTCODE_DELETION'; @@ -42,25 +43,26 @@ const DeleteShortUrlModal = (

Caution! You are about to delete a short URL.

This action cannot be undone. Once you have deleted it, all the visits stats will be lost.

+

Write {shortUrl.shortCode} to confirm deletion.

setInputValue(e.target.value)} /> {hasThresholdError && ( -
+ {errorData?.threshold && `This short URL has received more than ${errorData.threshold} visits, and therefore, it cannot be deleted.`} {!errorData?.threshold && 'This short URL has received too many visits, and therefore, it cannot be deleted.'} -
+ )} {hasErrorOtherThanThreshold && ( -
+ Something went wrong while deleting the URL :( -
+ )} diff --git a/src/short-urls/helpers/EditMetaModal.tsx b/src/short-urls/helpers/EditMetaModal.tsx index 109925bb..8f1ac31c 100644 --- a/src/short-urls/helpers/EditMetaModal.tsx +++ b/src/short-urls/helpers/EditMetaModal.tsx @@ -10,6 +10,7 @@ import DateInput from '../../utils/DateInput'; import { formatIsoDate } from '../../utils/helpers/date'; import { ShortUrl, ShortUrlMeta, ShortUrlModalProps } from '../data'; import { handleEventPreventingDefault, Nullable, OptionalString } from '../../utils/utils'; +import { Result } from '../../utils/Result'; interface EditMetaModalConnectProps extends ShortUrlModalProps { shortUrlMeta: ShortUrlMetaEdition; @@ -78,9 +79,9 @@ const EditMetaModal = ( /> {error && ( -
+ Something went wrong while saving the metadata :( -
+ )} diff --git a/src/short-urls/helpers/EditShortUrlModal.tsx b/src/short-urls/helpers/EditShortUrlModal.tsx index 019ad5bc..92428f4c 100644 --- a/src/short-urls/helpers/EditShortUrlModal.tsx +++ b/src/short-urls/helpers/EditShortUrlModal.tsx @@ -4,6 +4,7 @@ import { ExternalLink } from 'react-external-link'; import { ShortUrlEdition } from '../reducers/shortUrlEdition'; import { handleEventPreventingDefault, hasValue, OptionalString } from '../../utils/utils'; import { ShortUrlModalProps } from '../data'; +import { Result } from '../../utils/Result'; interface EditShortUrlModalProps extends ShortUrlModalProps { shortUrlEdition: ShortUrlEdition; @@ -34,9 +35,9 @@ const EditShortUrlModal = ({ isOpen, toggle, shortUrl, shortUrlEdition, editShor /> {error && ( -
+ Something went wrong while saving the long URL :( -
+ )} diff --git a/src/short-urls/helpers/EditTagsModal.tsx b/src/short-urls/helpers/EditTagsModal.tsx index 03bbff81..af1909d6 100644 --- a/src/short-urls/helpers/EditTagsModal.tsx +++ b/src/short-urls/helpers/EditTagsModal.tsx @@ -5,6 +5,7 @@ import { ShortUrlTags } from '../reducers/shortUrlTags'; import { ShortUrlModalProps } from '../data'; import { OptionalString } from '../../utils/utils'; import { TagsSelectorProps } from '../../tags/helpers/TagsSelector'; +import { Result } from '../../utils/Result'; interface EditTagsModalProps extends ShortUrlModalProps { shortUrlTags: ShortUrlTags; @@ -32,9 +33,9 @@ const EditTagsModal = (TagsSelector: FC) => ( {shortUrlTags.error && ( -
+ Something went wrong while saving the tags :( -
+ )} diff --git a/src/tags/TagsList.tsx b/src/tags/TagsList.tsx index 9d1cb38a..b72a74f6 100644 --- a/src/tags/TagsList.tsx +++ b/src/tags/TagsList.tsx @@ -4,6 +4,7 @@ import Message from '../utils/Message'; import SearchField from '../utils/SearchField'; import { SelectedServer } from '../servers/data'; import { boundToMercureHub } from '../mercure/helpers/boundToMercureHub'; +import { Result } from '../utils/Result'; import { TagsList as TagsListState } from './reducers/tagsList'; import { TagCardProps } from './TagCard'; @@ -32,7 +33,7 @@ const TagsList = (TagCard: FC) => boundToMercureHub(( } if (tagsList.error) { - return
Error loading tags :(
; + return Error loading tags :(; } const tagsCount = tagsList.filteredTags.length; diff --git a/src/tags/helpers/DeleteTagConfirmModal.tsx b/src/tags/helpers/DeleteTagConfirmModal.tsx index 5429d704..0afc9fb7 100644 --- a/src/tags/helpers/DeleteTagConfirmModal.tsx +++ b/src/tags/helpers/DeleteTagConfirmModal.tsx @@ -1,6 +1,7 @@ import { Modal, ModalBody, ModalFooter, ModalHeader } from 'reactstrap'; import { TagDeletion } from '../reducers/tagDelete'; import { TagModalProps } from '../data'; +import { Result } from '../../utils/Result'; interface DeleteTagConfirmModalProps extends TagModalProps { deleteTag: (tag: string) => Promise; @@ -25,9 +26,9 @@ const DeleteTagConfirmModal = ( Are you sure you want to delete tag {tag}? {tagDelete.error && ( -
+ Something went wrong while deleting the tag :( -
+ )} diff --git a/src/tags/helpers/EditTagModal.tsx b/src/tags/helpers/EditTagModal.tsx index b1ae9c0d..26acc0c7 100644 --- a/src/tags/helpers/EditTagModal.tsx +++ b/src/tags/helpers/EditTagModal.tsx @@ -9,6 +9,7 @@ import ColorGenerator from '../../utils/services/ColorGenerator'; import { TagModalProps } from '../data'; import { TagEdition } from '../reducers/tagEdit'; import './EditTagModal.scss'; +import { Result } from '../../utils/Result'; interface EditTagModalProps extends TagModalProps { tagEdit: TagEdition; @@ -55,9 +56,9 @@ const EditTagModal = ({ getColorForKey }: ColorGenerator) => (
{tagEdit.error && ( -
+ Something went wrong while editing the tag :( -
+ )} diff --git a/src/utils/Result.tsx b/src/utils/Result.tsx new file mode 100644 index 00000000..da3da6b4 --- /dev/null +++ b/src/utils/Result.tsx @@ -0,0 +1,30 @@ +import { FC } from 'react'; +import { Row } from 'reactstrap'; +import classNames from 'classnames'; +import { SimpleCard } from './SimpleCard'; + +interface ResultProps { + type: 'success' | 'error' | 'warning'; + className?: string; + textCentered?: boolean; + small?: boolean; +} + +export const Result: FC = ({ children, type, className, textCentered = false, small = false }) => ( + +
+ + {children} + +
+ +); diff --git a/src/utils/SimpleCard.tsx b/src/utils/SimpleCard.tsx index e50213e7..5ecbdc4f 100644 --- a/src/utils/SimpleCard.tsx +++ b/src/utils/SimpleCard.tsx @@ -4,11 +4,12 @@ import { ReactNode } from 'react'; interface SimpleCardProps extends Omit { title?: ReactNode; + bodyClassName?: string; } -export const SimpleCard = ({ title, children, ...rest }: SimpleCardProps) => ( +export const SimpleCard = ({ title, children, bodyClassName, ...rest }: SimpleCardProps) => ( {title && {title}} - {children} + {children} ); diff --git a/src/visits/VisitsStats.tsx b/src/visits/VisitsStats.tsx index 55dc9ed4..83e267e0 100644 --- a/src/visits/VisitsStats.tsx +++ b/src/visits/VisitsStats.tsx @@ -11,6 +11,7 @@ import Message from '../utils/Message'; import { formatIsoDate } from '../utils/helpers/date'; import { ShlinkVisitsParams } from '../utils/services/types'; import { DateInterval, DateRange, intervalToDateRange } from '../utils/dates/types'; +import { Result } from '../utils/Result'; import SortableBarGraph from './helpers/SortableBarGraph'; import GraphCard from './helpers/GraphCard'; import LineChartCard from './helpers/LineChartCard'; @@ -130,11 +131,7 @@ const VisitsStats: FC = ({ children, visitsInfo, getVisits, ca } if (error) { - return ( - - An error occurred while loading visits :( - - ); + return An error occurred while loading visits :(; } if (isEmpty(visits)) { diff --git a/test/servers/CreateServer.test.tsx b/test/servers/CreateServer.test.tsx index 1e45ca17..dcc90006 100644 --- a/test/servers/CreateServer.test.tsx +++ b/test/servers/CreateServer.test.tsx @@ -30,12 +30,12 @@ describe('', () => { const wrapper = createWrapper(); expect(wrapper.find(ServerForm)).toHaveLength(1); - expect(wrapper.find('Result')).toHaveLength(0); + expect(wrapper.find('ImportResult')).toHaveLength(0); }); it('shows success message when imported is true', () => { const wrapper = createWrapper(true); - const result = wrapper.find('Result'); + const result = wrapper.find('ImportResult'); expect(result).toHaveLength(1); expect(result.prop('type')).toEqual('success'); @@ -43,7 +43,7 @@ describe('', () => { it('shows error message when import failed', () => { const wrapper = createWrapper(false, true); - const result = wrapper.find('Result'); + const result = wrapper.find('ImportResult'); expect(result).toHaveLength(1); expect(result.prop('type')).toEqual('error'); diff --git a/test/short-urls/helpers/CreateShortUrlResult.test.tsx b/test/short-urls/helpers/CreateShortUrlResult.test.tsx index ba280df5..4430987b 100644 --- a/test/short-urls/helpers/CreateShortUrlResult.test.tsx +++ b/test/short-urls/helpers/CreateShortUrlResult.test.tsx @@ -5,6 +5,7 @@ import { Mock } from 'ts-mockery'; import createCreateShortUrlResult from '../../../src/short-urls/helpers/CreateShortUrlResult'; import { ShortUrl } from '../../../src/short-urls/data'; import { StateFlagTimeout } from '../../../src/utils/helpers/hooks'; +import { Result } from '../../../src/utils/Result'; describe('', () => { let wrapper: ShallowWrapper; @@ -24,7 +25,7 @@ describe('', () => { it('renders an error when error is true', () => { const wrapper = createWrapper(Mock.all(), true); - const errorCard = wrapper.find('.bg-danger'); + const errorCard = wrapper.find(Result).filterWhere((result) => result.prop('type') === 'error'); expect(errorCard).toHaveLength(1); expect(errorCard.html()).toContain('An error occurred while creating the URL :('); diff --git a/test/short-urls/helpers/DeleteShortUrlModal.test.tsx b/test/short-urls/helpers/DeleteShortUrlModal.test.tsx index 11498d80..06ce4864 100644 --- a/test/short-urls/helpers/DeleteShortUrlModal.test.tsx +++ b/test/short-urls/helpers/DeleteShortUrlModal.test.tsx @@ -5,6 +5,7 @@ import DeleteShortUrlModal from '../../../src/short-urls/helpers/DeleteShortUrlM import { ShortUrl } from '../../../src/short-urls/data'; import { ShortUrlDeletion } from '../../../src/short-urls/reducers/shortUrlDeletion'; import { ProblemDetailsError } from '../../../src/utils/services/types'; +import { Result } from '../../../src/utils/Result'; describe('', () => { let wrapper: ShallowWrapper; @@ -48,7 +49,7 @@ describe('', () => { shortCode: 'abc123', errorData: Mock.of(errorData), }); - const warning = wrapper.find('.bg-warning'); + const warning = wrapper.find(Result).filterWhere((result) => result.prop('type') === 'warning'); expect(warning).toHaveLength(1); expect(warning.html()).toContain(expectedMessage); @@ -61,7 +62,7 @@ describe('', () => { shortCode: 'abc123', errorData: Mock.of({ type: 'OTHER_ERROR' }), }); - const error = wrapper.find('.bg-danger'); + const error = wrapper.find(Result).filterWhere((result) => result.prop('type') === 'error'); expect(error).toHaveLength(1); expect(error.html()).toContain('Something went wrong while deleting the URL :('); diff --git a/test/short-urls/helpers/EditMetaModal.test.tsx b/test/short-urls/helpers/EditMetaModal.test.tsx index 730ceaa5..b5553125 100644 --- a/test/short-urls/helpers/EditMetaModal.test.tsx +++ b/test/short-urls/helpers/EditMetaModal.test.tsx @@ -4,6 +4,7 @@ import { Mock } from 'ts-mockery'; import EditMetaModal from '../../../src/short-urls/helpers/EditMetaModal'; import { ShortUrl } from '../../../src/short-urls/data'; import { ShortUrlMetaEdition } from '../../../src/short-urls/reducers/shortUrlMeta'; +import { Result } from '../../../src/utils/Result'; describe('', () => { let wrapper: ShallowWrapper; @@ -30,7 +31,7 @@ describe('', () => { it('properly renders form with components', () => { const wrapper = createWrapper({ saving: false, error: false }); - const error = wrapper.find('.bg-danger'); + const error = wrapper.find(Result).filterWhere((result) => result.prop('type') === 'error'); const form = wrapper.find('form'); const formGroup = form.find(FormGroup); @@ -52,7 +53,7 @@ describe('', () => { it('renders error message on error', () => { const wrapper = createWrapper({ saving: false, error: true }); - const error = wrapper.find('.bg-danger'); + const error = wrapper.find(Result).filterWhere((result) => result.prop('type') === 'error'); expect(error).toHaveLength(1); }); diff --git a/test/short-urls/helpers/EditShortUrlModal.test.tsx b/test/short-urls/helpers/EditShortUrlModal.test.tsx index 95e2f494..2ab14c4b 100644 --- a/test/short-urls/helpers/EditShortUrlModal.test.tsx +++ b/test/short-urls/helpers/EditShortUrlModal.test.tsx @@ -4,6 +4,7 @@ import { Mock } from 'ts-mockery'; import EditShortUrlModal from '../../../src/short-urls/helpers/EditShortUrlModal'; import { ShortUrl } from '../../../src/short-urls/data'; import { ShortUrlEdition } from '../../../src/short-urls/reducers/shortUrlEdition'; +import { Result } from '../../../src/utils/Result'; describe('', () => { let wrapper: ShallowWrapper; @@ -31,7 +32,7 @@ describe('', () => { [ true, 1 ], ])('properly renders form with expected components', (error, expectedErrorLength) => { const wrapper = createWrapper({}, { saving: false, error }); - const errorElement = wrapper.find('.bg-danger'); + const errorElement = wrapper.find(Result).filterWhere((result) => result.prop('type') === 'error'); const form = wrapper.find('form'); const formGroup = form.find(FormGroup); diff --git a/test/tags/TagsList.test.tsx b/test/tags/TagsList.test.tsx index 4369cc94..072f3b6f 100644 --- a/test/tags/TagsList.test.tsx +++ b/test/tags/TagsList.test.tsx @@ -7,6 +7,7 @@ import SearchField from '../../src/utils/SearchField'; import { rangeOf } from '../../src/utils/utils'; import { TagsList } from '../../src/tags/reducers/tagsList'; import { MercureBoundProps } from '../../src/mercure/helpers/boundToMercureHub'; +import { Result } from '../../src/utils/Result'; describe('', () => { let wrapper: ShallowWrapper; @@ -41,7 +42,7 @@ describe('', () => { it('shows an error when tags failed to be loaded', () => { const wrapper = createWrapper({ error: true }); - const errorMsg = wrapper.find('.bg-danger'); + const errorMsg = wrapper.find(Result).filterWhere((result) => result.prop('type') === 'error'); expect(errorMsg).toHaveLength(1); expect(errorMsg.html()).toContain('Error loading tags :('); diff --git a/test/visits/VisitsStats.test.tsx b/test/visits/VisitsStats.test.tsx index ba79f7bf..d878d2e8 100644 --- a/test/visits/VisitsStats.test.tsx +++ b/test/visits/VisitsStats.test.tsx @@ -1,5 +1,5 @@ import { shallow, ShallowWrapper } from 'enzyme'; -import { Card, Progress } from 'reactstrap'; +import { Progress } from 'reactstrap'; import { Mock } from 'ts-mockery'; import VisitStats from '../../src/visits/VisitsStats'; import Message from '../../src/utils/Message'; @@ -8,6 +8,7 @@ import SortableBarGraph from '../../src/visits/helpers/SortableBarGraph'; import { Visit, VisitsInfo } from '../../src/visits/types'; import LineChartCard from '../../src/visits/helpers/LineChartCard'; import VisitsTable from '../../src/visits/VisitsTable'; +import { Result } from '../../src/utils/Result'; describe('', () => { const visits = [ Mock.all(), Mock.all(), Mock.all() ]; @@ -53,7 +54,7 @@ describe('', () => { it('renders an error message when visits could not be loaded', () => { const wrapper = createComponent({ loading: false, error: true, visits: [] }); - const errorMessage = wrapper.find(Card); + const errorMessage = wrapper.find(Result).filterWhere((result) => result.prop('type') === 'error'); expect(errorMessage).toHaveLength(1); expect(errorMessage.html()).toContain('An error occurred while loading visits :('); From 344f5e9b0d06f565bce872602904d88ad149cde5 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 21 Dec 2020 17:58:46 +0100 Subject: [PATCH 4/7] Updated Result component so that it has the text centered by default --- src/servers/CreateServer.tsx | 2 +- src/short-urls/helpers/DeleteShortUrlModal.tsx | 4 ++-- src/short-urls/helpers/EditMetaModal.tsx | 2 +- src/short-urls/helpers/EditShortUrlModal.tsx | 2 +- src/short-urls/helpers/EditTagsModal.tsx | 2 +- src/tags/helpers/DeleteTagConfirmModal.tsx | 2 +- src/tags/helpers/EditTagModal.tsx | 2 +- src/utils/Result.tsx | 6 ++---- 8 files changed, 10 insertions(+), 12 deletions(-) diff --git a/src/servers/CreateServer.tsx b/src/servers/CreateServer.tsx index a04ccba8..d98d3619 100644 --- a/src/servers/CreateServer.tsx +++ b/src/servers/CreateServer.tsx @@ -16,7 +16,7 @@ interface CreateServerProps extends RouterProps { } const ImportResult = ({ type }: { type: 'error' | 'success' }) => ( - + {type === 'success' && 'Servers properly imported. You can now select one from the list :)'} {type === 'error' && 'The servers could not be imported. Make sure the format is correct.'} diff --git a/src/short-urls/helpers/DeleteShortUrlModal.tsx b/src/short-urls/helpers/DeleteShortUrlModal.tsx index 8e4c9f21..35f99cf4 100644 --- a/src/short-urls/helpers/DeleteShortUrlModal.tsx +++ b/src/short-urls/helpers/DeleteShortUrlModal.tsx @@ -54,13 +54,13 @@ const DeleteShortUrlModal = ( /> {hasThresholdError && ( - + {errorData?.threshold && `This short URL has received more than ${errorData.threshold} visits, and therefore, it cannot be deleted.`} {!errorData?.threshold && 'This short URL has received too many visits, and therefore, it cannot be deleted.'} )} {hasErrorOtherThanThreshold && ( - + Something went wrong while deleting the URL :( )} diff --git a/src/short-urls/helpers/EditMetaModal.tsx b/src/short-urls/helpers/EditMetaModal.tsx index 8f1ac31c..ca22c2a1 100644 --- a/src/short-urls/helpers/EditMetaModal.tsx +++ b/src/short-urls/helpers/EditMetaModal.tsx @@ -79,7 +79,7 @@ const EditMetaModal = ( /> {error && ( - + Something went wrong while saving the metadata :( )} diff --git a/src/short-urls/helpers/EditShortUrlModal.tsx b/src/short-urls/helpers/EditShortUrlModal.tsx index 92428f4c..1a24d640 100644 --- a/src/short-urls/helpers/EditShortUrlModal.tsx +++ b/src/short-urls/helpers/EditShortUrlModal.tsx @@ -35,7 +35,7 @@ const EditShortUrlModal = ({ isOpen, toggle, shortUrl, shortUrlEdition, editShor /> {error && ( - + Something went wrong while saving the long URL :( )} diff --git a/src/short-urls/helpers/EditTagsModal.tsx b/src/short-urls/helpers/EditTagsModal.tsx index af1909d6..a41a4b98 100644 --- a/src/short-urls/helpers/EditTagsModal.tsx +++ b/src/short-urls/helpers/EditTagsModal.tsx @@ -33,7 +33,7 @@ const EditTagsModal = (TagsSelector: FC) => ( {shortUrlTags.error && ( - + Something went wrong while saving the tags :( )} diff --git a/src/tags/helpers/DeleteTagConfirmModal.tsx b/src/tags/helpers/DeleteTagConfirmModal.tsx index 0afc9fb7..eb95cd55 100644 --- a/src/tags/helpers/DeleteTagConfirmModal.tsx +++ b/src/tags/helpers/DeleteTagConfirmModal.tsx @@ -26,7 +26,7 @@ const DeleteTagConfirmModal = ( Are you sure you want to delete tag {tag}? {tagDelete.error && ( - + Something went wrong while deleting the tag :( )} diff --git a/src/tags/helpers/EditTagModal.tsx b/src/tags/helpers/EditTagModal.tsx index 26acc0c7..5779520b 100644 --- a/src/tags/helpers/EditTagModal.tsx +++ b/src/tags/helpers/EditTagModal.tsx @@ -56,7 +56,7 @@ const EditTagModal = ({ getColorForKey }: ColorGenerator) => ( {tagEdit.error && ( - + Something went wrong while editing the tag :( )} diff --git a/src/utils/Result.tsx b/src/utils/Result.tsx index da3da6b4..a381fd21 100644 --- a/src/utils/Result.tsx +++ b/src/utils/Result.tsx @@ -6,20 +6,18 @@ import { SimpleCard } from './SimpleCard'; interface ResultProps { type: 'success' | 'error' | 'warning'; className?: string; - textCentered?: boolean; small?: boolean; } -export const Result: FC = ({ children, type, className, textCentered = false, small = false }) => ( +export const Result: FC = ({ children, type, className, small = false }) => (
From 94d2f3167bf0b38e8f67959ef21d14fa9024cf2f Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 21 Dec 2020 18:14:11 +0100 Subject: [PATCH 5/7] Created Result test --- src/utils/Result.tsx | 6 +++-- test/utils/Result.test.tsx | 46 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 2 deletions(-) create mode 100644 test/utils/Result.test.tsx diff --git a/src/utils/Result.tsx b/src/utils/Result.tsx index a381fd21..5f49e703 100644 --- a/src/utils/Result.tsx +++ b/src/utils/Result.tsx @@ -3,8 +3,10 @@ import { Row } from 'reactstrap'; import classNames from 'classnames'; import { SimpleCard } from './SimpleCard'; -interface ResultProps { - type: 'success' | 'error' | 'warning'; +export type ResultType = 'success' | 'error' | 'warning'; + +export interface ResultProps { + type: ResultType; className?: string; small?: boolean; } diff --git a/test/utils/Result.test.tsx b/test/utils/Result.test.tsx new file mode 100644 index 00000000..ab0062d2 --- /dev/null +++ b/test/utils/Result.test.tsx @@ -0,0 +1,46 @@ +import { shallow, ShallowWrapper } from 'enzyme'; +import { Result, ResultProps, ResultType } from '../../src/utils/Result'; +import { SimpleCard } from '../../src/utils/SimpleCard'; + +describe('', () => { + let wrapper: ShallowWrapper; + const createWrapper = (props: ResultProps) => { + wrapper = shallow(); + + return wrapper; + }; + + afterEach(() => wrapper?.unmount()); + + it.each([ + [ 'success' as ResultType, 'bg-main text-white' ], + [ 'error' as ResultType, 'bg-danger text-white' ], + [ 'warning' as ResultType, 'bg-warning' ], + ])('renders expected classes based on type', (type, expectedClasses) => { + const wrapper = createWrapper({ type }); + const innerCard = wrapper.find(SimpleCard); + + expect(innerCard.prop('className')).toEqual(`text-center ${expectedClasses}`); + }); + + it.each([ + [ undefined ], + [ 'foo' ], + [ 'bar' ], + ])('renders provided classes in root element', (className) => { + const wrapper = createWrapper({ type: 'success', className }); + + expect(wrapper.prop('className')).toEqual(className); + }); + + it.each([{ small: true }, { small: false }])('renders small results properly', ({ small }) => { + const wrapper = createWrapper({ type: 'success', small }); + const bigElement = wrapper.find('.col-md-10'); + const smallElement = wrapper.find('.col-12'); + const innerCard = wrapper.find(SimpleCard); + + expect(bigElement).toHaveLength(small ? 0 : 1); + expect(smallElement).toHaveLength(small ? 1 : 0); + expect(innerCard.prop('bodyClassName')).toEqual(small ? 'p-2' : ''); + }); +}); From 195aaa8be64daa3b5e641d2aa3f8eae9bdb2e52d Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 21 Dec 2020 18:15:09 +0100 Subject: [PATCH 6/7] Updated changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index faf98cfa..c0ff9857 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ### Changed * [#267](https://github.com/shlinkio/shlink-web-client/issues/267) Added some subtle but important improvements on UI/UX. * [#352](https://github.com/shlinkio/shlink-web-client/issues/352) Moved from Scrutinizer to Codecov as the code coverage backend. +* [#217](https://github.com/shlinkio/shlink-web-client/issues/217) Improved how messages are displayed, by centralizing it in the `Message` and `Result` components.. ### Deprecated * *Nothing* From 7a48a06442fd48c6a11f57f9719cefc5712d8c90 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 21 Dec 2020 18:20:59 +0100 Subject: [PATCH 7/7] Normalized import --- src/short-urls/helpers/CreateShortUrlResult.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/short-urls/helpers/CreateShortUrlResult.tsx b/src/short-urls/helpers/CreateShortUrlResult.tsx index ffe3d3e9..916f622e 100644 --- a/src/short-urls/helpers/CreateShortUrlResult.tsx +++ b/src/short-urls/helpers/CreateShortUrlResult.tsx @@ -7,8 +7,8 @@ import CopyToClipboard from 'react-copy-to-clipboard'; import { Tooltip } from 'reactstrap'; import { ShortUrlCreation } from '../reducers/shortUrlCreation'; import { StateFlagTimeout } from '../../utils/helpers/hooks'; -import './CreateShortUrlResult.scss'; import { Result } from '../../utils/Result'; +import './CreateShortUrlResult.scss'; export interface CreateShortUrlResultProps extends ShortUrlCreation { resetCreateShortUrl: () => void;