mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
fix(atlas): keep long location-note tooltips on-screen and scrollable
Closes #2153 Bucket-list marker tooltips rendered as a single nowrap line, so long notes overflowed off both edges of the viewport instead of wrapping. Wrap and cap the tooltip's size, add a scrollbar for notes that still exceed it (only shown when content actually overflows, and scoped to this tooltip so short region/country tooltips are unaffected), and make the tooltip interactive with a delayed close so the pointer can reach it to scroll instead of Leaflet closing it the instant it leaves the marker's tiny hit area. Also compute the tooltip's direction and offset from the marker's on-screen position before it opens, so it flips below the marker and stays clear of the left/right edges when there's no room to render it centered above. The positioning/scroll math is pure and covered by unit tests in atlasModel.test.ts. Updated the L.marker mocks in AtlasPage.test.tsx and useAtlas.test.tsx with on/off/getTooltip/closeTooltip so the new marker event wiring doesn't crash under test.
This commit is contained in:
committed by
Maurice
parent
394201cd32
commit
92bb834f2d
@@ -122,8 +122,29 @@ body { height: 100%; overflow: auto; overscroll-behavior: none; -webkit-overflow
|
||||
font-family: inherit !important;
|
||||
box-shadow: 0 8px 32px rgba(0,0,0,0.25) !important;
|
||||
transition: none !important;
|
||||
max-width: min(280px, calc(100vw - 32px)) !important;
|
||||
white-space: normal !important;
|
||||
word-wrap: break-word !important;
|
||||
}
|
||||
.atlas-tooltip::before { border-top-color: rgba(10, 10, 20, 0.6) !important; }
|
||||
/* Bucket-list marker tooltip only — arbitrary-length user notes need a scroll
|
||||
cap. The scroll happens on .atlas-tooltip-scroll-inner, not this element,
|
||||
so the outer border-radius clips the scrollbar instead of it poking
|
||||
through the rounded corner. overflow-y on the inner div is toggled
|
||||
per-instance in useAtlas.ts, not forced here. */
|
||||
.atlas-tooltip-scrollable {
|
||||
width: min(480px, calc(100vw - 32px)) !important;
|
||||
max-width: min(480px, calc(100vw - 32px)) !important;
|
||||
overflow: hidden !important;
|
||||
pointer-events: auto !important;
|
||||
}
|
||||
.atlas-tooltip-scroll-inner {
|
||||
max-height: min(200px, 40vh);
|
||||
overflow-y: hidden;
|
||||
}
|
||||
.atlas-tooltip-scroll-inner::-webkit-scrollbar { width: 6px; }
|
||||
.atlas-tooltip-scroll-inner::-webkit-scrollbar-thumb { background: rgba(255,255,255,0.25); border-radius: 3px; }
|
||||
html:not(.dark) .atlas-tooltip-scroll-inner::-webkit-scrollbar-thumb { background: rgba(0,0,0,0.2); }
|
||||
html:not(.dark) .atlas-tooltip {
|
||||
background: rgba(255, 255, 255, 0.75) !important;
|
||||
color: #0f172a !important;
|
||||
|
||||
@@ -113,12 +113,23 @@ vi.mock('leaflet', () => {
|
||||
};
|
||||
}),
|
||||
divIcon: vi.fn(() => ({})),
|
||||
marker: vi.fn(() => ({
|
||||
addTo: vi.fn().mockReturnThis(),
|
||||
on: vi.fn(),
|
||||
remove: vi.fn(),
|
||||
bindTooltip: vi.fn().mockReturnThis(),
|
||||
})),
|
||||
marker: vi.fn(() => {
|
||||
let tooltip: { options: Record<string, unknown>; getElement: () => null } | undefined;
|
||||
const m = {
|
||||
addTo: vi.fn().mockReturnThis(),
|
||||
on: vi.fn(() => m),
|
||||
off: vi.fn(() => m),
|
||||
remove: vi.fn(),
|
||||
getLatLng: vi.fn(() => ({ lat: 0, lng: 0 })),
|
||||
bindTooltip: vi.fn(() => {
|
||||
tooltip = { options: { direction: 'top', offset: [0, -14] }, getElement: () => null };
|
||||
return m;
|
||||
}),
|
||||
getTooltip: vi.fn(() => tooltip),
|
||||
closeTooltip: vi.fn(),
|
||||
};
|
||||
return m;
|
||||
}),
|
||||
latLngBounds: vi.fn(() => ({ extend: vi.fn(), isValid: vi.fn(() => true) })),
|
||||
layerGroup: vi.fn(() => ({ addTo: vi.fn().mockReturnThis(), clearLayers: vi.fn() })),
|
||||
canvas: vi.fn(() => ({})),
|
||||
|
||||
@@ -1,6 +1,9 @@
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import {
|
||||
A2_TO_A3,
|
||||
bucketTooltipNeedsScroll,
|
||||
bucketTooltipPlacement,
|
||||
bucketTooltipWidth,
|
||||
countryStatus,
|
||||
findBucketDuplicate,
|
||||
isBucketDuplicateError,
|
||||
@@ -296,3 +299,62 @@ describe('isBucketDuplicateError (#1898)', () => {
|
||||
expect(isBucketDuplicateError(null)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('bucketTooltipWidth (#2153)', () => {
|
||||
it('caps at 480px on a wide viewport', () => {
|
||||
expect(bucketTooltipWidth(1400)).toBe(480);
|
||||
});
|
||||
|
||||
it('shrinks to fit a narrow viewport, minus 32px of margin', () => {
|
||||
expect(bucketTooltipWidth(390)).toBe(358);
|
||||
});
|
||||
});
|
||||
|
||||
describe('bucketTooltipPlacement (#2153)', () => {
|
||||
it('opens above a marker with room on all sides, centred with no horizontal nudge', () => {
|
||||
const placement = bucketTooltipPlacement({ x: 500, y: 400 }, 1000, 480);
|
||||
expect(placement.direction).toBe('top');
|
||||
expect(placement.offset).toEqual([0, -14]);
|
||||
});
|
||||
|
||||
it('flips to open below the marker when there is no room above it', () => {
|
||||
const placement = bucketTooltipPlacement({ x: 500, y: 100 }, 1000, 480);
|
||||
expect(placement.direction).toBe('bottom');
|
||||
expect(placement.offset[1]).toBe(14);
|
||||
});
|
||||
|
||||
it('nudges right when centring the tooltip would clip the left edge', () => {
|
||||
// Marker at x=50 with a 480px-wide tooltip centred on it would start at
|
||||
// x=-190; the offset needs to push the tooltip's left edge to the margin (8px).
|
||||
const placement = bucketTooltipPlacement({ x: 50, y: 400 }, 1000, 480);
|
||||
const defaultLeft = 50 - 480 / 2;
|
||||
expect(placement.offset[0]).toBe(8 - defaultLeft);
|
||||
});
|
||||
|
||||
it('nudges left when centring the tooltip would clip the right edge', () => {
|
||||
const placement = bucketTooltipPlacement({ x: 950, y: 400 }, 1000, 480);
|
||||
const defaultLeft = 950 - 480 / 2;
|
||||
const clampedLeft = 1000 - 8 - 480;
|
||||
expect(placement.offset[0]).toBe(clampedLeft - defaultLeft);
|
||||
});
|
||||
|
||||
it('does not nudge horizontally when the tooltip already fits', () => {
|
||||
const placement = bucketTooltipPlacement({ x: 500, y: 400 }, 1000, 480);
|
||||
expect(placement.offset[0]).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe('bucketTooltipNeedsScroll (#2153)', () => {
|
||||
it('is false when content fits within the height cap', () => {
|
||||
expect(bucketTooltipNeedsScroll(180, 200)).toBe(false);
|
||||
expect(bucketTooltipNeedsScroll(200, 200)).toBe(false);
|
||||
});
|
||||
|
||||
it('is true once content actually overflows the cap', () => {
|
||||
expect(bucketTooltipNeedsScroll(260, 200)).toBe(true);
|
||||
});
|
||||
|
||||
it('tolerates a 1px measurement rounding difference without enabling scroll', () => {
|
||||
expect(bucketTooltipNeedsScroll(201, 200)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -213,6 +213,40 @@ export function regionCacheEvictions(order: string[], keep: Set<string>, max: nu
|
||||
return drop
|
||||
}
|
||||
|
||||
/** Width (CSS px) the bucket-list marker tooltip renders at — mirrors .atlas-tooltip-scrollable in index.css. */
|
||||
export function bucketTooltipWidth(viewportWidth: number): number {
|
||||
return Math.min(480, viewportWidth - 32)
|
||||
}
|
||||
|
||||
export interface TooltipPlacement {
|
||||
direction: 'top' | 'bottom'
|
||||
offset: [number, number]
|
||||
}
|
||||
|
||||
/** Keeps the bucket-list marker tooltip on-screen (#2153): Leaflet has no viewport
|
||||
* awareness, so flip below the marker when there's no room above, and nudge the
|
||||
* horizontal offset to keep it within [margin, viewportWidth - margin]. */
|
||||
export function bucketTooltipPlacement(
|
||||
markerScreen: { x: number; y: number },
|
||||
viewportWidth: number,
|
||||
tooltipWidth: number,
|
||||
opts: { margin?: number; topThreshold?: number } = {},
|
||||
): TooltipPlacement {
|
||||
const margin = opts.margin ?? 8
|
||||
const topThreshold = opts.topThreshold ?? 220
|
||||
const flip = markerScreen.y < topThreshold
|
||||
const defaultLeft = markerScreen.x - tooltipWidth / 2
|
||||
const clampedLeft = Math.min(Math.max(defaultLeft, margin), viewportWidth - margin - tooltipWidth)
|
||||
const dx = clampedLeft - defaultLeft
|
||||
return { direction: flip ? 'bottom' : 'top', offset: [dx, flip ? 14 : -14] }
|
||||
}
|
||||
|
||||
/** scrollHeight ignores the CSS max-height clip, so this is the one place that decides
|
||||
* whether the bucket-list tooltip actually needs its scrollbar (#2153). */
|
||||
export function bucketTooltipNeedsScroll(scrollHeight: number, clientHeight: number): boolean {
|
||||
return scrollHeight > clientHeight + 1
|
||||
}
|
||||
|
||||
// Convert country code to flag emoji
|
||||
export function countryCodeToFlag(code: string): string {
|
||||
if (!code || code.length !== 2) return ''
|
||||
|
||||
@@ -131,7 +131,19 @@ vi.mock('leaflet', () => {
|
||||
divIcon: vi.fn(() => ({})),
|
||||
marker: vi.fn(() => {
|
||||
lf.markers += 1;
|
||||
return { bindTooltip: vi.fn(() => ({})) };
|
||||
let tooltip: { options: Record<string, unknown>; getElement: () => null } | undefined;
|
||||
const m = {
|
||||
on: vi.fn(() => m),
|
||||
off: vi.fn(() => m),
|
||||
getLatLng: vi.fn(() => ({ lat: 0, lng: 0 })),
|
||||
bindTooltip: vi.fn(() => {
|
||||
tooltip = { options: { direction: 'top', offset: [0, -14] }, getElement: () => null };
|
||||
return m;
|
||||
}),
|
||||
getTooltip: vi.fn(() => tooltip),
|
||||
closeTooltip: vi.fn(),
|
||||
};
|
||||
return m;
|
||||
}),
|
||||
layerGroup: vi.fn(() => ({ addTo: vi.fn(() => ({})) })),
|
||||
geoJSON: vi.fn((data: { features?: Record<string, unknown>[] }, options: Record<string, unknown>) => {
|
||||
|
||||
@@ -9,7 +9,7 @@ import { isVectorStyle } from '../../utils/tileUrl'
|
||||
import apiClient, { mapsApi, pluginsApi, type PluginAtlasLayer } from '../../api/client'
|
||||
import L from 'leaflet'
|
||||
import type { GeoJsonFeatureCollection } from '../../types'
|
||||
import { A2_TO_A3, countryStatus, findBucketDuplicate, isBucketDuplicateError, isCountryVisible, normalizeRegionName, regionCacheEvictions, withCountryMarkedVisited, wishlistA3Codes, countryColor, REGION_CACHE_MAX, type AtlasData, type AtlasPlaceHit, type CountryDetail, type BucketItem } from './atlasModel'
|
||||
import { A2_TO_A3, countryStatus, findBucketDuplicate, isBucketDuplicateError, isCountryVisible, normalizeRegionName, regionCacheEvictions, withCountryMarkedVisited, wishlistA3Codes, countryColor, REGION_CACHE_MAX, bucketTooltipWidth, bucketTooltipPlacement, bucketTooltipNeedsScroll, type AtlasData, type AtlasPlaceHit, type CountryDetail, type BucketItem } from './atlasModel'
|
||||
import { continentForCountry, escapeHtml, type VisitStatus } from '@trek/shared'
|
||||
import { useToast } from '../../components/shared/Toast'
|
||||
import { getApiErrorMessage } from '../../types'
|
||||
@@ -1091,10 +1091,42 @@ export function useAtlas() {
|
||||
iconSize: [28, 28],
|
||||
iconAnchor: [14, 14],
|
||||
})
|
||||
return L.marker([b.lat!, b.lng!], { icon }).bindTooltip(
|
||||
`<div style="font-size:12px;font-weight:600">${escapeHtml(b.name)}</div>${b.notes ? `<div style="font-size:10px;opacity:0.7;margin-top:2px">${escapeHtml(b.notes)}</div>` : ''}`,
|
||||
{ className: 'atlas-tooltip', direction: 'top', offset: [0, -14] }
|
||||
const marker = L.marker([b.lat!, b.lng!], { icon })
|
||||
// Registered before bindTooltip, so this runs ahead of Leaflet's own
|
||||
// mouseover handler and lets it read the corrected direction/offset on open.
|
||||
marker.on('mouseover', () => {
|
||||
const map = mapInstance.current
|
||||
const tooltip = marker.getTooltip()
|
||||
if (!map || !tooltip) return
|
||||
const containerRect = map.getContainer().getBoundingClientRect()
|
||||
const point = map.latLngToContainerPoint(marker.getLatLng())
|
||||
const screenX = containerRect.left + point.x
|
||||
const screenY = containerRect.top + point.y
|
||||
const placement = bucketTooltipPlacement({ x: screenX, y: screenY }, window.innerWidth, bucketTooltipWidth(window.innerWidth))
|
||||
tooltip.options.direction = placement.direction
|
||||
tooltip.options.offset = placement.offset
|
||||
})
|
||||
marker.bindTooltip(
|
||||
`<div class="atlas-tooltip-scroll-inner"><div style="font-size:12px;font-weight:600">${escapeHtml(b.name)}</div>${b.notes ? `<div style="font-size:10px;opacity:0.7;margin-top:2px">${escapeHtml(b.notes)}</div>` : ''}</div>`,
|
||||
{ className: 'atlas-tooltip atlas-tooltip-scrollable', direction: 'top', offset: [0, -14], interactive: true }
|
||||
)
|
||||
// Leaflet closes the tooltip the instant the pointer leaves the marker's tiny
|
||||
// hit area — replace that with a delayed close, cancelled while hovering the
|
||||
// tooltip itself, so the pointer has a path from marker to tooltip to scroll it.
|
||||
let closeTimer: ReturnType<typeof setTimeout> | null = null
|
||||
const cancelClose = () => { if (closeTimer) { clearTimeout(closeTimer); closeTimer = null } }
|
||||
const scheduleClose = () => { closeTimer = setTimeout(() => marker.closeTooltip(), 200) }
|
||||
marker.off('mouseout', marker.closeTooltip, marker)
|
||||
marker.on('mouseout', scheduleClose)
|
||||
marker.on('tooltipopen', () => {
|
||||
const el = marker.getTooltip()?.getElement()
|
||||
if (!el) return
|
||||
el.addEventListener('mouseenter', cancelClose)
|
||||
el.addEventListener('mouseleave', scheduleClose)
|
||||
const inner = el.querySelector<HTMLElement>('.atlas-tooltip-scroll-inner')
|
||||
if (inner) inner.style.overflowY = bucketTooltipNeedsScroll(inner.scrollHeight, inner.clientHeight) ? 'auto' : 'hidden'
|
||||
})
|
||||
return marker
|
||||
})
|
||||
bucketMarkersRef.current = L.layerGroup(markers).addTo(mapInstance.current)
|
||||
}, [bucketList])
|
||||
|
||||
Reference in New Issue
Block a user