Skip to content

Fix duplicate Leaflet map controls on Overview and Deployment pages #508

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ksalamy merged 1 commit into from
Feb 13, 2026
Merged

Fix duplicate Leaflet map controls on Overview and Deployment pages #508

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 60 additions & 45 deletions apps/lrauv-dash2/pages/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -725,6 +725,19 @@ const OverViewMap: React.FC<{

type MobileView = 'map' | 'list'

// xl breakpoint = 1280px (matches Tailwind xl:)
const useIsDesktop = () => {
const [isDesktop, setIsDesktop] = useState(true)
useEffect(() => {
const mq = window.matchMedia('(min-width: 1280px)')
setIsDesktop(mq.matches)
const handler = (e: MediaQueryListEvent) => setIsDesktop(e.matches)
mq.addEventListener('change', handler)
return () => mq.removeEventListener('change', handler)
}, [])
Comment on lines +730 to +737
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

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

The useIsDesktop hook defaults to true in useState, then updates to the actual media query value in useEffect. This can cause a hydration mismatch in Next.js SSR: the server renders thinking it's desktop, but on the client the actual media query might indicate mobile. This can lead to React hydration warnings. Consider adding a typeof window check or using a null initial state to avoid SSR issues, or set the initial state to undefined and handle that case in the component logic.

Suggested change
const [isDesktop, setIsDesktop] = useState(true)
useEffect(() => {
const mq = window.matchMedia('(min-width: 1280px)')
setIsDesktop(mq.matches)
const handler = (e: MediaQueryListEvent) => setIsDesktop(e.matches)
mq.addEventListener('change', handler)
return () => mq.removeEventListener('change', handler)
}, [])
const [isDesktop, setIsDesktop] = useState(() => {
if (typeof window === 'undefined') {
// On the server, default to desktop to match existing behavior.
return true
}
return window.matchMedia('(min-width: 1280px)').matches
})
useEffect(() => {
if (typeof window === 'undefined') {
return
}
const mq = window.matchMedia('(min-width: 1280px)')
setIsDesktop(mq.matches)
const handler = (e: MediaQueryListEvent) => setIsDesktop(e.matches)
mq.addEventListener('change', handler)
return () => mq.removeEventListener('change', handler)
}, [])

Copilot uses AI. Check for mistakes.
Copy link
Collaborator Author

@ksalamy ksalamy Feb 11, 2026

Choose a reason for hiding this comment

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

Thanks for the review. We’re keeping the current approach: useState(true) to avoid hydration mismatches, and we’ll hold off on extracting the hook for now since dash5 is desktop-only. We may revisit when we add mobile support.

return isDesktop
}
Comment on lines +729 to +739
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

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

The useIsDesktop hook is duplicated between index.tsx and [...deployment].tsx. This violates the DRY principle and creates a maintenance burden - any fixes or improvements need to be applied in two places. Consider extracting this hook to a shared location like apps/lrauv-dash2/lib/useIsDesktop.ts.

Copilot uses AI. Check for mistakes.
Copy link
Collaborator Author

@ksalamy ksalamy Feb 11, 2026

Choose a reason for hiding this comment

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

Good idea long term. For now we’re leaving the hook in place. Dash5 is desktop-only for the moment, so we’ll consider extracting to a shared util when we add mobile support.


// OverviewPage: NextPage
const OverviewPage: NextPage = () => {
const { mapsLoaded } = useGoogleMaps()
Expand All @@ -733,6 +746,7 @@ const OverviewPage: NextPage = () => {
const mounted = useRef(false)
const { setGlobalModalId } = useGlobalModalId()
const [mobileView, setMobileView] = useState<MobileView>('map')
const isDesktop = useIsDesktop()

useEffect(() => {
if (!mounted.current) {
Expand Down Expand Up @@ -781,54 +795,55 @@ const OverviewPage: NextPage = () => {
className={styles.content}
data-testid="vehicle-dashboard"
>
{/* Desktop view (keep existing Allotment layout) */}
<div className="hidden h-full w-full xl:block">
<Allotment
separator
snap
defaultSizes={[75, 25]}
proportionalLayout
>
<Allotment.Pane>{primarySection}</Allotment.Pane>
<Allotment.Pane priority={LayoutPriority.High}>
{secondarySection}
</Allotment.Pane>
</Allotment>
</div>

{/* Mobile view (toggle between map and list) */}
<div className="flex h-full w-full flex-col xl:hidden">
<div className="flex shrink-0 items-center gap-2 border-b border-slate-200 bg-white px-4 py-2">
<button
type="button"
onClick={() => setMobileView('map')}
className={
mobileView === 'map'
? 'rounded bg-secondary-300/60 px-3 py-1 text-sm font-bold text-black'
: 'rounded px-3 py-1 text-sm font-bold text-slate-600'
}
>
Map
</button>
<button
type="button"
onClick={() => setMobileView('list')}
className={
mobileView === 'list'
? 'rounded bg-secondary-300/60 px-3 py-1 text-sm font-bold text-black'
: 'rounded px-3 py-1 text-sm font-bold text-slate-600'
}
{/* Single map instance: render one layout to avoid duplicate controls */}
{isDesktop ? (
<div className="h-full w-full">
<Allotment
separator
snap
defaultSizes={[75, 25]}
proportionalLayout
>
Vehicles
</button>
<Allotment.Pane>{primarySection}</Allotment.Pane>
<Allotment.Pane priority={LayoutPriority.High}>
{secondarySection}
</Allotment.Pane>
</Allotment>
</div>

<div className="min-h-0 flex-1 overflow-hidden">
{mobileView === 'map'
? primarySection
: secondarySection}
) : (
<div className="flex h-full w-full flex-col">
<div className="flex shrink-0 items-center gap-2 border-b border-slate-200 bg-white px-4 py-2">
<button
type="button"
onClick={() => setMobileView('map')}
className={
mobileView === 'map'
? 'rounded bg-secondary-300/60 px-3 py-1 text-sm font-bold text-black'
: 'rounded px-3 py-1 text-sm font-bold text-slate-600'
}
>
Map
</button>
<button
type="button"
onClick={() => setMobileView('list')}
className={
mobileView === 'list'
? 'rounded bg-secondary-300/60 px-3 py-1 text-sm font-bold text-black'
: 'rounded px-3 py-1 text-sm font-bold text-slate-600'
}
>
Vehicles
</button>
</div>

<div className="min-h-0 flex-1 overflow-hidden">
{mobileView === 'map'
? primarySection
: secondarySection}
</div>
</div>
</div>
)}
</div>
</>
) : (
Expand Down
54 changes: 33 additions & 21 deletions apps/lrauv-dash2/pages/vehicle/[...deployment].tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,18 @@ const DeploymentMap = dynamic(() => import('../../components/DeploymentMap'), {
type AvailableTab = 'vehicle' | 'depth' | null
type MobileView = 'main' | 'sidebar'

const useIsDesktop = () => {
const [isDesktop, setIsDesktop] = useState(true)
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

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

The useIsDesktop hook defaults to true in useState, then updates to the actual media query value in useEffect. This can cause a hydration mismatch in Next.js SSR: the server renders thinking it's desktop, but on the client the actual media query might indicate mobile. This can lead to React hydration warnings. Consider adding a typeof window check or using a null initial state to avoid SSR issues, or set the initial state to undefined and handle that case in the component logic.

Copilot uses AI. Check for mistakes.
Copy link
Collaborator Author

@ksalamy ksalamy Feb 11, 2026

Choose a reason for hiding this comment

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

Thanks for the review. We’re keeping the current approach: useState(true) to avoid hydration mismatches, and we’ll hold off on extracting the hook for now since dash5 is desktop-only. We may revisit when we add mobile support.

useEffect(() => {
const mq = window.matchMedia('(min-width: 1280px)')
setIsDesktop(mq.matches)
const handler = (e: MediaQueryListEvent) => setIsDesktop(e.matches)
mq.addEventListener('change', handler)
return () => mq.removeEventListener('change', handler)
}, [])
return isDesktop
}

const Vehicle: NextPage = () => {
const { authenticated } = useTethysApiContext()
const { mapsLoaded } = useGoogleMaps()
Expand All @@ -94,6 +106,7 @@ const Vehicle: NextPage = () => {
}

const [mobileView, setMobileView] = useState<MobileView>('main')
const isDesktop = useIsDesktop()

const params = (router.query?.deployment ?? []) as string[]
const vehicleName = params[0]
Expand Down Expand Up @@ -452,27 +465,26 @@ const Vehicle: NextPage = () => {
authenticated={authenticated}
/>

{/* Desktop view */}
<div className="hidden min-h-0 flex-1 xl:flex">
<div className={styles.content}>
<Allotment
separator
defaultSizes={[75, 25]}
className="min-h-0"
>
<Allotment.Pane minSize={720}>
{primarySection}
</Allotment.Pane>
<Allotment.Pane minSize={512}>
{secondarySection}
</Allotment.Pane>
</Allotment>
{/* Single map instance: render one layout to avoid duplicate controls */}
{isDesktop ? (
<div className="min-h-0 flex-1 flex">
<div className={styles.content}>
<Allotment
separator
defaultSizes={[75, 25]}
className="min-h-0"
>
<Allotment.Pane minSize={720}>
{primarySection}
</Allotment.Pane>
<Allotment.Pane minSize={512}>
{secondarySection}
</Allotment.Pane>
</Allotment>
</div>
</div>
</div>

{/* Mobile view (toggle between main and sidebar) */}
<div className="flex min-h-0 flex-1 xl:hidden">
<div className={clsx(styles.content, 'flex-col')}>
) : (
<div className="flex min-h-0 flex-1 flex-col">
<div className="flex shrink-0 items-center gap-2 border-b border-slate-200 bg-white px-4 py-2">
<button
type="button"
Expand Down Expand Up @@ -504,7 +516,7 @@ const Vehicle: NextPage = () => {
{mobileView === 'main' ? primarySection : secondarySection}
</div>
</div>
</div>
)}
</Layout>
</div>
</SelectedStationsProvider>
Expand Down
Loading
Loading