Skip to content

Commit

Permalink
2990: Use navigate directly over passing it down
Browse files Browse the repository at this point in the history
  • Loading branch information
steffenkleinle committed Jan 22, 2025
1 parent 68f1241 commit 82d1abd
Show file tree
Hide file tree
Showing 12 changed files with 36 additions and 79 deletions.
4 changes: 0 additions & 4 deletions web/src/components/CategoriesContent.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import React, { ReactElement } from 'react'
import { useTranslation } from 'react-i18next'
import { useNavigate } from 'react-router-dom'

import { getCategoryTiles } from 'shared'
import { CategoriesMapModel, CategoryModel } from 'shared/api'
Expand All @@ -26,7 +25,6 @@ const CategoriesContent = ({
languageCode,
}: CategoriesContentProps): ReactElement => {
const children = categories.getChildren(categoryModel)
const navigate = useNavigate()
const { t } = useTranslation('layout')

if (categories.isLeaf(categoryModel)) {
Expand All @@ -35,7 +33,6 @@ const CategoriesContent = ({
title={categoryModel.title}
content={categoryModel.content}
lastUpdate={categoryModel.lastUpdate}
onInternalLinkClick={navigate}
AfterContent={
categoryModel.organization && <OrganizationContentInfo organization={categoryModel.organization} />
}
Expand All @@ -60,7 +57,6 @@ const CategoriesContent = ({
<CategoryList
items={children.map(it => ({ category: it, subCategories: categories.getChildren(it) }))}
category={categoryModel}
onInternalLinkClick={navigate}
/>
)
}
Expand Down
5 changes: 2 additions & 3 deletions web/src/components/CategoryList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,12 @@ const List = styled.ul`
type CategoryListProps = {
items: { category: CategoryModel; subCategories: CategoryModel[]; contentWithoutHtml?: string }[]
category?: CategoryModel
onInternalLinkClick: (link: string) => void
}

const CategoryList = ({ items, category, onInternalLinkClick }: CategoryListProps): ReactElement => (
const CategoryList = ({ items, category }: CategoryListProps): ReactElement => (
<div>
{!!category?.title && <Caption title={category.title} />}
{!!category?.content && <RemoteContent html={category.content} onInternalLinkClick={onInternalLinkClick} />}
{!!category?.content && <RemoteContent html={category.content} />}
{!!category?.content && <LastUpdateInfo lastUpdate={category.lastUpdate} withText />}
<List>
{items.map(({ category, subCategories }) => (
Expand Down
4 changes: 1 addition & 3 deletions web/src/components/ChatMessage.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { TFunction } from 'i18next'
import React, { ReactElement } from 'react'
import { useTranslation } from 'react-i18next'
import { useNavigate } from 'react-router-dom'
import styled from 'styled-components'

import ChatMessageModel from 'shared/api/models/ChatMessageModel'
Expand Down Expand Up @@ -59,15 +58,14 @@ const getIcon = (userIsAuthor: boolean, isAutomaticAnswer: boolean, t: TFunction
}

const ChatMessage = ({ message, showIcon }: ChatMessageProps): ReactElement => {
const navigate = useNavigate()
const { t } = useTranslation('chat')
const { body, userIsAuthor, isAutomaticAnswer } = message

return (
<Container $isAuthor={userIsAuthor}>
<IconContainer $visible={showIcon}>{getIcon(userIsAuthor, isAutomaticAnswer, t)}</IconContainer>
<Message data-testid={message.id}>
<RemoteContent html={body} onInternalLinkClick={navigate} smallText />
<RemoteContent html={body} smallText />
</Message>
</Container>
)
Expand Down
4 changes: 1 addition & 3 deletions web/src/components/Page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ type PageProps = {
content: string
lastUpdate?: DateTime
showLastUpdateText?: boolean
onInternalLinkClick: (url: string) => void
BeforeContent?: ReactNode
AfterContent?: ReactNode
Footer?: ReactNode
Expand All @@ -35,7 +34,6 @@ const Page = ({
content,
lastUpdate,
showLastUpdateText = true,
onInternalLinkClick,
BeforeContent,
AfterContent,
Footer,
Expand All @@ -44,7 +42,7 @@ const Page = ({
{!!thumbnailSrcSet && <Thumbnail alt='' srcSet={thumbnailSrcSet} />}
<Caption title={title} />
{BeforeContent}
<RemoteContent html={content} onInternalLinkClick={onInternalLinkClick} />
<RemoteContent html={content} />
{AfterContent}
{lastUpdate && !!content && content.length > 0 && (
<LastUpdateInfo lastUpdate={lastUpdate} withText={showLastUpdateText} />
Expand Down
3 changes: 1 addition & 2 deletions web/src/components/PoiDetails.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,6 @@ type PoiDetailsProps = {
}

const PoiDetails = ({ poi, distance, toolbar }: PoiDetailsProps): ReactElement => {
const navigate = useNavigate()
const { viewportSmall } = useWindowDimensions()
const theme = useTheme()
const { t } = useTranslation('pois')
Expand Down Expand Up @@ -220,7 +219,7 @@ const PoiDetails = ({ poi, distance, toolbar }: PoiDetailsProps): ReactElement =
<>
<Spacer $borderColor={theme.colors.borderColor} />
<Collapsible title={t('detailsInformation')}>
<RemoteContent html={content} onInternalLinkClick={navigate} smallText />
<RemoteContent html={content} smallText />
</Collapsible>
</>
)}
Expand Down
14 changes: 5 additions & 9 deletions web/src/components/RemoteContent.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import Dompurify from 'dompurify'
import React, { ReactElement, useCallback, useEffect, useState } from 'react'
import { useTranslation } from 'react-i18next'
import { useNavigate } from 'react-router-dom'
import styled, { css } from 'styled-components'

import { ExternalSourcePermissions } from 'shared'
Expand Down Expand Up @@ -168,7 +169,6 @@ const SandBox = styled.div<{ $centered: boolean; $smallText: boolean }>`

type RemoteContentProps = {
html: string
onInternalLinkClick: (url: string) => void
centered?: boolean
smallText?: boolean
}
Expand All @@ -181,12 +181,8 @@ const DOMPURIFY_ATTRIBUTE_TARGET = 'target'
export type IframeSources = Record<number, string>
export const IFRAME_BLANK_SOURCE = 'about:blank'

const RemoteContent = ({
html,
onInternalLinkClick,
centered = false,
smallText = false,
}: RemoteContentProps): ReactElement => {
const RemoteContent = ({ html, centered = false, smallText = false }: RemoteContentProps): ReactElement => {
const navigate = useNavigate()
const sandBoxRef = React.createRef<HTMLDivElement>()
const { value: externalSourcePermissions, updateLocalStorageItem } = useLocalStorage<ExternalSourcePermissions>({
key: LOCAL_STORAGE_ITEM_EXTERNAL_SOURCES,
Expand All @@ -205,10 +201,10 @@ const RemoteContent = ({
if (target instanceof HTMLAnchorElement) {
const href = target.href
const url = new URL(decodeURIComponent(href))
onInternalLinkClick(decodeURIComponent(`${url.pathname}${url.search}${url.hash}`))
navigate(decodeURIComponent(`${url.pathname}${url.search}${url.hash}`))
}
},
[onInternalLinkClick],
[navigate],
)

const onUpdateLocalStorage = useCallback(
Expand Down
13 changes: 2 additions & 11 deletions web/src/components/__tests__/CategoryList.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -88,24 +88,15 @@ const items = [
]

describe('CategoryList', () => {
beforeEach(() => {
jest.clearAllMocks()
})
const onInternalLinkClick = jest.fn()

it('should render category list', () => {
const { getByText } = renderWithRouterAndTheme(
<CategoryList onInternalLinkClick={onInternalLinkClick} items={items} />,
)
const { getByText } = renderWithRouterAndTheme(<CategoryList items={items} />)
categoryModels.forEach(() => {
expect(getByText(categoryModels[0].title)).toBeTruthy()
})
})

it('should render title, content and thumbnail of category', () => {
const { getByText } = renderWithRouterAndTheme(
<CategoryList onInternalLinkClick={onInternalLinkClick} items={[]} category={modelWithTitle} />,
)
const { getByText } = renderWithRouterAndTheme(<CategoryList items={[]} category={modelWithTitle} />)
expect(getByText('Asylantrag')).toBeTruthy()
expect(getByText('This is some special test content')).toBeTruthy()
})
Expand Down
47 changes: 20 additions & 27 deletions web/src/components/__tests__/RemoteContent.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,56 +4,55 @@ import React from 'react'
import { renderWithTheme } from '../../testing/render'
import RemoteContent from '../RemoteContent'

const navigate = jest.fn()

jest.mock('react-router-dom', () => ({
...jest.requireActual('react-router-dom'),
useNavigate: () => navigate,
}))
jest.mock('react-i18next')

describe('RemoteContent', () => {
beforeEach(jest.resetAllMocks)

it('should render the html content', () => {
const content = 'Test html'
const { getByText } = renderWithTheme(
<RemoteContent html={`<div>${content}</div>`} onInternalLinkClick={() => undefined} />,
)
const { getByText } = renderWithTheme(<RemoteContent html={`<div>${content}</div>`} />)
expect(getByText(content)).toBeTruthy()
})

it('should trigger on internalLinkClick for internal links', () => {
it('should navigate for internal links', () => {
const path = '/augsburg/de'
const href = `https://integreat.app${path}`
const html = `<a href=${href}>Test Anchor</a>`
const onInternalLinkClick = jest.fn()

const { getByRole, getAllByRole } = renderWithTheme(
<RemoteContent html={html} onInternalLinkClick={onInternalLinkClick} />,
)
const { getByRole, getAllByRole } = renderWithTheme(<RemoteContent html={html} />)

expect(getAllByRole('link')).toHaveLength(1)
fireEvent.click(getByRole('link'))

expect(onInternalLinkClick).toHaveBeenCalledTimes(1)
expect(onInternalLinkClick).toHaveBeenLastCalledWith(path)
expect(navigate).toHaveBeenCalledTimes(1)
expect(navigate).toHaveBeenLastCalledWith(path)
})

it('should not trigger on internalLinkClick for external links', () => {
it('should not navigate for external links', () => {
const href = `https://some.external/link`
const html = `<a href=${href} class="link-external">Test Anchor</a>`
const onInternalLinkClick = jest.fn()

const { getByRole, getAllByRole } = renderWithTheme(
<RemoteContent html={html} onInternalLinkClick={onInternalLinkClick} />,
)
const { getByRole, getAllByRole } = renderWithTheme(<RemoteContent html={html} />)

expect(getAllByRole('link')).toHaveLength(1)
fireEvent.click(getByRole('link'))

expect(onInternalLinkClick).toHaveBeenCalledTimes(0)
expect(navigate).toHaveBeenCalledTimes(0)
})

it('should block an iframe with unsupported source', () => {
const src = `https://unknownvideo.com`
const iframeTitle = 'unknown'
const html = `<iframe title=${iframeTitle} src=${src} />`
const onInternalLinkClick = jest.fn()

const { getByTitle } = renderWithTheme(<RemoteContent html={html} onInternalLinkClick={onInternalLinkClick} />)
const { getByTitle } = renderWithTheme(<RemoteContent html={html} />)

expect(getByTitle(iframeTitle)).toHaveAttribute('src', 'about:blank')
})
Expand All @@ -62,9 +61,8 @@ describe('RemoteContent', () => {
const src = `https://vimeo.com`
const iframeTitle = 'vimeo'
const html = `<iframe title=${iframeTitle} src=${src} />`
const onInternalLinkClick = jest.fn()

const { getAllByRole } = renderWithTheme(<RemoteContent html={html} onInternalLinkClick={onInternalLinkClick} />)
const { getAllByRole } = renderWithTheme(<RemoteContent html={html} />)

expect(getAllByRole('checkbox')).toHaveLength(1)
})
Expand All @@ -74,11 +72,8 @@ describe('RemoteContent', () => {
const doNotTrackParameter = `/?dnt=1`
const iframeTitle = 'vimeo'
const html = `<iframe title=${iframeTitle} src=${src} />`
const onInternalLinkClick = jest.fn()

const { getByRole, getAllByRole, getByTitle } = renderWithTheme(
<RemoteContent html={html} onInternalLinkClick={onInternalLinkClick} />,
)
const { getByRole, getAllByRole, getByTitle } = renderWithTheme(<RemoteContent html={html} />)

expect(getAllByRole('checkbox')).toHaveLength(1)
fireEvent.click(getByRole('checkbox'))
Expand All @@ -91,9 +86,7 @@ describe('RemoteContent', () => {

const content =
'<div><p>Ich bleib aber da.<iframe//src=jAva&Tab;script:alert(3)>def</p><math><mi//xlink:href="data:x,<script>alert(4)</script>">'
const { getByText } = renderWithTheme(
<RemoteContent html={`<div>${content}</div>`} onInternalLinkClick={() => undefined} />,
)
const { getByText } = renderWithTheme(<RemoteContent html={`<div>${content}</div>`} />)

expect(alertSpy).not.toHaveBeenCalled()
// window.alert is not implemented in jsdom and upon calling it an error message is logged to the console.
Expand Down
9 changes: 1 addition & 8 deletions web/src/routes/DisclaimerPage.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import React, { ReactElement } from 'react'
import { useTranslation } from 'react-i18next'
import { useNavigate } from 'react-router-dom'

import { DISCLAIMER_ROUTE, pathnameFromRouteInformation } from 'shared'
import { createDisclaimerEndpoint, useLoadFromEndpoint } from 'shared/api'
Expand All @@ -15,7 +14,6 @@ import Page from '../components/Page'
import { cmsApiBaseUrl } from '../constants/urls'

const DisclaimerPage = ({ cityCode, languageCode, city }: CityRouteProps): ReactElement | null => {
const navigate = useNavigate()
const { t } = useTranslation('disclaimer')

const {
Expand Down Expand Up @@ -69,12 +67,7 @@ const DisclaimerPage = ({ cityCode, languageCode, city }: CityRouteProps): React
return (
<CityContentLayout isLoading={false} {...locationLayoutParams}>
<Helmet pageTitle={pageTitle} languageChangePaths={languageChangePaths} cityModel={city} />
<Page
lastUpdate={disclaimer.lastUpdate}
title={disclaimer.title}
content={disclaimer.content}
onInternalLinkClick={navigate}
/>
<Page lastUpdate={disclaimer.lastUpdate} title={disclaimer.title} content={disclaimer.content} />
</CityContentLayout>
)
}
Expand Down
4 changes: 1 addition & 3 deletions web/src/routes/EventsPage.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { DateTime } from 'luxon'
import React, { ReactElement } from 'react'
import { useTranslation } from 'react-i18next'
import { useNavigate, useParams } from 'react-router-dom'
import { useParams } from 'react-router-dom'
import styled from 'styled-components'

import { EVENTS_ROUTE, pathnameFromRouteInformation, useDateFilter } from 'shared'
Expand Down Expand Up @@ -38,7 +38,6 @@ const EventsPage = ({ city, pathname, languageCode, cityCode }: CityRouteProps):
const previousPathname = usePreviousProp({ prop: pathname })
const { eventId } = useParams()
const { t } = useTranslation('events')
const navigate = useNavigate()

const {
data: events,
Expand Down Expand Up @@ -124,7 +123,6 @@ const EventsPage = ({ city, pathname, languageCode, cityCode }: CityRouteProps):
lastUpdate={lastUpdate}
content={content}
title={title}
onInternalLinkClick={navigate}
BeforeContent={
<Spacing $content={content} $lastUpdate={lastUpdate}>
<DatesPageDetail date={date} languageCode={languageCode} />
Expand Down
4 changes: 1 addition & 3 deletions web/src/routes/LocalNewsPage.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React, { ReactElement } from 'react'
import { useTranslation } from 'react-i18next'
import { useNavigate, useParams } from 'react-router-dom'
import { useParams } from 'react-router-dom'

import {
LOCAL_NEWS_TYPE,
Expand Down Expand Up @@ -29,7 +29,6 @@ const LocalNewsPage = ({ city, pathname, languageCode, cityCode }: CityRouteProp
const previousPathname = usePreviousProp({ prop: pathname })
const { newsId } = useParams()
const { t } = useTranslation('news')
const navigate = useNavigate()

const {
data: localNews,
Expand Down Expand Up @@ -134,7 +133,6 @@ const LocalNewsPage = ({ city, pathname, languageCode, cityCode }: CityRouteProp
content={linkedContent}
lastUpdate={newsModel.timestamp}
showLastUpdateText={false}
onInternalLinkClick={navigate}
/>
</CityContentLayout>
)
Expand Down
4 changes: 1 addition & 3 deletions web/src/routes/TuNewsDetailPage.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React, { ReactElement } from 'react'
import { useTranslation } from 'react-i18next'
import { useNavigate, useParams } from 'react-router-dom'
import { useParams } from 'react-router-dom'
import styled from 'styled-components'

import { TU_NEWS_TYPE } from 'shared'
Expand Down Expand Up @@ -56,7 +56,6 @@ const TuNewsDetailPage = ({ city, pathname, cityCode, languageCode }: CityRouteP
// This component is only opened when there is a news ID in the route
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const newsId = useParams().newsId!
const navigate = useNavigate()
const { t } = useTranslation('news')

const {
Expand Down Expand Up @@ -122,7 +121,6 @@ const TuNewsDetailPage = ({ city, pathname, cityCode, languageCode }: CityRouteP
content={newsModel.content}
lastUpdate={newsModel.date}
showLastUpdateText={false}
onInternalLinkClick={navigate}
/>
</>
</StyledContainer>
Expand Down

0 comments on commit 82d1abd

Please sign in to comment.