Skip to content

Commit

Permalink
[fix]: object select bugs (#2785)
Browse files Browse the repository at this point in the history
* Fix submit botton to bottom

* Fix scroll height without submit button

* Fix mobile styling issue

* Fix flaky max asset functionality

* Fix tests

* Remove useCurrentAddress

* fix ESLinting warning

* PR feedback
  • Loading branch information
vdegraaf authored Dec 13, 2023
1 parent fcbf149 commit 68c3ef2
Show file tree
Hide file tree
Showing 17 changed files with 104 additions and 193 deletions.
28 changes: 20 additions & 8 deletions src/components/IconList/IconList.tsx
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
// SPDX-License-Identifier: MPL-2.0
// Copyright (C) 2021-2023 Gemeente Amsterdam
// Copyright (C) 2021 - 2023 Gemeente Amsterdam
import type { ReactNode } from 'react'
import { useCallback, useState } from 'react'
import { useCallback, useEffect, useMemo, useState } from 'react'

import { List } from '@amsterdam/asc-ui'
import { useSelector } from 'react-redux'
import styled from 'styled-components'

import type {
FeatureStatusType,
Item,
} from 'signals/incident/components/form/MapSelectors/types'
import { makeSelectMaxAssetWarning } from 'signals/incident/containers/IncidentContainer/selectors'

import { StyledListItem, StyledImg, StatusIcon } from './styled'
import Checkbox from '../Checkbox'
Expand Down Expand Up @@ -42,21 +44,31 @@ export const IconListItem = ({
const [checkedState, setCheckedState] = useState<boolean | undefined>(
undefined
)
const [disabled, setDisabled] = useState<boolean>(false)
const { maxAssetWarning } = useSelector(makeSelectMaxAssetWarning)

useEffect(() => {
setCheckedState(checked)
}, [checked, maxAssetWarning])

const disableOnClick = useMemo(() => {
if (maxAssetWarning) {
return checked
}

return true
}, [checked, maxAssetWarning])

const onClickWithDelay = useCallback(
(item) => {
if (onClick && !disabled) {
setDisabled(true)
if (onClick && disableOnClick) {
setCheckedState(!checked)
const timeout = setTimeout(() => {
onClick(item)
setDisabled(false)
}, 600)
}, 400)
return () => clearTimeout(timeout)
}
},
[checked, disabled, onClick]
[checked, disableOnClick, onClick]
)

return (
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: MPL-2.0
// Copyright (C) 2023 Gemeente Amsterdam
import { act, renderHook } from '@testing-library/react-hooks'
import * as reactRedux from 'react-redux'

import reverseGeocoderService from 'shared/services/reverse-geocoder'

Expand Down Expand Up @@ -90,10 +91,18 @@ describe('useSelectionProps', () => {
],
},
}

beforeEach(() => {
jest
.spyOn(reactRedux, 'useSelector')
.mockReturnValue({ makeSelectMaxAssetWarning: false })
})

it('should give a result', async () => {
jest.mocked(reverseGeocoderService).mockImplementation(async () => {
return geocodedResponse
})

const result = renderHook(() =>
useSelectionProps({
featureTypes: props.featureTypes,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@
// Copyright (C) 2023 Gemeente Amsterdam
import { useContext } from 'react'

import { useSelector } from 'react-redux'

import { makeSelectMaxAssetWarning } from 'signals/incident/containers/IncidentContainer/selectors'

import { featureToCoordinates } from '../../../../../../../../shared/services/map-location'
import reverseGeocoderService from '../../../../../../../../shared/services/reverse-geocoder'
import type {
Expand All @@ -16,7 +20,6 @@ import type { Feature, FeatureType, Item } from '../../../types'
import { FeatureStatus } from '../../../types'
import AssetSelectContext from '../../context'
import type { Props } from '../AssetListItemSelectable'

const getFeatureType = (feature: Feature, featureTypes: FeatureType[]) => {
return featureTypes.find(
({ typeField, typeValue }) => feature.properties[typeField] === typeValue
Expand All @@ -29,6 +32,7 @@ export const useSelectionProps = ({
selection,
}: Props) => {
const { setItem } = useContext(AssetSelectContext)
const { maxAssetWarning } = useSelector(makeSelectMaxAssetWarning)

const coordinates = featureToCoordinates(feature.geometry as Geometrie)

Expand Down Expand Up @@ -60,7 +64,7 @@ export const useSelectionProps = ({
featureTypes?.find(({ typeValue }) => typeValue === item.type) ?? {}

const onClick = async () => {
if (typeValue !== FeatureStatus.REPORTED) {
if (typeValue !== FeatureStatus.REPORTED && !maxAssetWarning) {
const location: Location = { coordinates }

const item: Item = {
Expand All @@ -72,8 +76,6 @@ export const useSelectionProps = ({
coordinates,
}

setItem(item, location)

const response = await reverseGeocoderService(coordinates)

if (response) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,11 @@ import { useCallback, useState } from 'react'

import type { FeatureCollection } from 'geojson'
import type { LatLngLiteral } from 'leaflet'
import { useSelector } from 'react-redux'
import { useDispatch, useSelector } from 'react-redux'

import Summary from 'components/Summary'
import reverseGeocoderService from 'shared/services/reverse-geocoder'
import { updateIncident as updateReduxIncident } from 'signals/incident/containers/IncidentContainer/actions'
import { makeSelectIncidentContainer } from 'signals/incident/containers/IncidentContainer/selectors'
import type { Incident, Location } from 'types/incident'

Expand Down Expand Up @@ -61,9 +62,9 @@ export interface AssetSelectProps {
}

const AssetSelect: FC<AssetSelectProps> = ({ value, layer, meta, parent }) => {
const dispatch = useDispatch()
const { selection, location } = value || {}
const [message, setMessage] = useState<string>()
const [addressLoading, setAddressLoading] = useState(false)
const [selectableFeatures, setSelectableFeatures] = useState<
FeatureCollection | undefined
>(undefined)
Expand Down Expand Up @@ -107,6 +108,7 @@ const AssetSelect: FC<AssetSelectProps> = ({ value, layer, meta, parent }) => {
selection: selectedItem ? [selectedItem] : undefined,
}

dispatch(updateReduxIncident({ maxAssetWarning: false }))
parent.meta.removeFromSelection({
[meta.name as string]: payload,
meta_name: meta.name,
Expand Down Expand Up @@ -149,11 +151,9 @@ const AssetSelect: FC<AssetSelectProps> = ({ value, layer, meta, parent }) => {
updateIncident(payload)

if (payload.location) {
setAddressLoading(true)
const response = await reverseGeocoderService(latLng)
payload.location.address = response?.data?.address
updateIncident(payload)
setAddressLoading(false)
}
},
[address, getUpdatePayload, updateIncident]
Expand Down Expand Up @@ -204,7 +204,6 @@ const AssetSelect: FC<AssetSelectProps> = ({ value, layer, meta, parent }) => {
layer,
message,
selectableFeatures,
addressLoading,
meta: {
...meta,
featureTypes,
Expand All @@ -216,7 +215,6 @@ const AssetSelect: FC<AssetSelectProps> = ({ value, layer, meta, parent }) => {
fetchLocation,
setMessage,
setSelectableFeatures,
setAddressLoading,
}}
>
{!mapActive && !hasSelection && <Intro />}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -292,9 +292,10 @@ describe('DetailPanel', () => {

userEvent.click(screen.getByTestId('legend-toggle-button'))

await waitFor(() => {
expect(screen.getByTestId('close-button')).toHaveFocus()
})
await waitFor(() => {})
const closeButton = screen.getByTestId('close-button')

expect(closeButton).toHaveFocus()
})

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import {
StyledLabelPDOkAutoSuggest,
StyledParagraphPDOkAutoSuggest,
} from './styled'
import { useCurrentAddress } from './useCurrentAddress'
import { useResetDrawerState } from './useResetDrawerState'
import {
DrawerOverlay,
Expand Down Expand Up @@ -53,7 +52,6 @@ const DetailPanel: FC<DetailPanelProps> = ({ language, zoomLevel }) => {
setLocation,
meta,
selectableFeatures,
addressLoading,
} = useContext(AssetSelectContext)
const { featureTypes } = meta
const featureStatusTypes = meta.featureStatusTypes || []
Expand Down Expand Up @@ -88,18 +86,13 @@ const DetailPanel: FC<DetailPanelProps> = ({ language, zoomLevel }) => {
: 100
}, [selection, zoomLevel, featureTypes, legendOpen])

const currentAddress = useCurrentAddress({
address,
addressLoading,
})

return (
<DrawerOverlay
state={drawerState}
onStateChange={setDrawerState}
disableDrawerHandleDesktop
topPositionDrawerMobile={topPositionDrawerMobile}
address={currentAddress}
address={address}
>
<PanelContent data-testid="detail-panel">
{!shouldRenderMobileVersion && (
Expand All @@ -114,7 +107,10 @@ const DetailPanel: FC<DetailPanelProps> = ({ language, zoomLevel }) => {
variant="blank"
/>
)}
<ScrollWrapper>
<ScrollWrapper
$hasSubmitButton={!!address}
$isMobile={shouldRenderMobileVersion}
>
{!shouldRenderMobileVersion && (
<>
<StyledParagraphPDOkAutoSuggest>
Expand Down Expand Up @@ -147,31 +143,33 @@ const DetailPanel: FC<DetailPanelProps> = ({ language, zoomLevel }) => {
zoomLevel={zoomLevel}
/>
)}
{currentAddress && !shouldRenderMobileVersion && (
<StyledButton
onClick={() => dispatch(closeMap())}
variant="primary"
data-testid="asset-select-submit-button"
tabIndex={0}
>
{submitButtonText}
</StyledButton>
)}
</ScrollWrapper>
{address && !shouldRenderMobileVersion && (
<StyledButton
onClick={() => dispatch(closeMap())}
variant="primary"
data-testid="asset-select-submit-button"
tabIndex={0}
>
{submitButtonText}
</StyledButton>
)}
</PanelContent>
<Legend
onLegendToggle={() => {
setDrawerState(DrawerState.Open)
setLegendOpen(!legendOpen)
}}
/>
{shouldRenderMobileVersion && currentAddress && (
{shouldRenderMobileVersion && address && (
<StyledButtonWrapper>
<StyledButton
onClick={() => dispatch(closeMap())}
variant="primary"
data-testid="asset-select-submit-button"
tabIndex={0}
$isMobile={shouldRenderMobileVersion}
$hasSubmitButton={!!address}
>
{submitButtonText}
</StyledButton>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
// SPDX-License-Identifier: MPL-2.0
// Copyright (C) 2022-2023 Gemeente Amsterdam
import { Button, themeSpacing, themeColor, breakpoint } from '@amsterdam/asc-ui'
import styled from 'styled-components'
import styled, { css } from 'styled-components'

import { DETAIL_PANEL_WIDTH } from '../../../constants'
import AssetList from '../../AssetList'
import LegendPanel from '../LegendPanel'
import LegendToggle from '../LegendToggleButton'
import { ScrollWrapper } from '../styled'

export const StyledBackButton = styled(Button)`
@media only screen and ${breakpoint('min-width', 'tabletM')} {
Expand All @@ -27,9 +26,29 @@ export const StyledAssetList = styled(AssetList)`
}
`

export const StyledButton = styled(Button)`
margin-top: ${themeSpacing(4)};
export const StyledButton = styled(Button)<{
$isMobile?: boolean
$hasSubmitButton?: boolean
}>`
position: sticky;
margin: ${themeSpacing(4)};
bottom: 0;
z-index: 1;
font-family: inherit;
${({ $isMobile }) =>
$isMobile &&
css`
margin: 0;
position: relative;
`}
${({ $hasSubmitButton, $isMobile }) =>
$hasSubmitButton &&
$isMobile &&
css`
height: calc(100% - 44px);
`}
`

export const StyledButtonWrapper = styled.div`
Expand All @@ -41,10 +60,6 @@ export const StyledButtonWrapper = styled.div`
padding: ${themeSpacing(5)};
background: white;
box-shadow: rgba(0, 0, 0, 0.1) 0px -4px 4px 0px;
${StyledButton} {
margin-top: 0;
}
`

export const LegendToggleButton = styled(LegendToggle)`
Expand All @@ -67,12 +82,6 @@ export const PanelContent = styled.div`
z-index: 1;
position: relative;
height: 100%;
@media only screen and ${breakpoint('max-width', 'tabletM')} {
${ScrollWrapper} {
padding-top: 0;
}
}
`

export const StyledLegendPanel = styled(LegendPanel)`
Expand Down
Loading

0 comments on commit 68c3ef2

Please sign in to comment.