Convert popup buttons to labels and remove locate points (#359)

Author: thisisharsh7

e2e/map.spec.ts

@@ -2,7 +2,6 @@ import { test, expect } from '@playwright/test';
 import {
   BERLIN_COORDINATES,
   setupHeightMock,
-  setupLocateMock,
   setupNominatimMock,
   setupRouteMock,
   setupSearchMock,
@@ -481,6 +480,9 @@ test.describe('Map interactions with left context menu', () => {
       button: 'left',
     });
 
+    // Wait for popup to appear (has 200ms delay)
+    await expect(page.getByTestId('map-info-popup')).toBeVisible();
+
     await expect(page.getByTestId('dd-button')).toContainText(
       '13.393707, 52.518310'
     );
@@ -496,14 +498,9 @@ test.describe('Map interactions with left context menu', () => {
     );
     await expect(page.getByTestId('dms-copy-button')).toBeVisible();
 
-    await expect(
-      page.getByRole('button', { name: 'Locate Point' })
-    ).toBeVisible();
-    await expect(page.getByTestId('locate-point-copy-button')).toBeVisible();
-
-    await expect(
-      page.getByRole('button', { name: 'Valhalla Location JSON' })
-    ).toBeVisible();
+    await expect(page.getByTestId('location-json-button')).toContainText(
+      'Valhalla Location JSON'
+    );
     await expect(page.getByTestId('location-json-copy-button')).toBeVisible();
 
     await expect(page.getByTestId('elevation-button')).toContainText('34 m');
@@ -516,35 +513,10 @@ test.describe('Map interactions with left context menu', () => {
       button: 'left',
     });
 
-    await expect(page.getByTestId('elevation-button')).toContainText('34 m');
-  });
-
-  test('should call locate', async ({ page }) => {
-    await setupHeightMock(page);
-    const locateRequests = await setupLocateMock(page);
+    // Wait for popup to appear (has 200ms delay)
+    await expect(page.getByTestId('map-info-popup')).toBeVisible();
 
-    await page.getByRole('region', { name: 'Map' }).click({
-      button: 'left',
-    });
-
-    await expect(
-      page.getByRole('button', { name: 'Locate Point' })
-    ).toBeVisible();
-
-    await page.getByRole('button', { name: 'Locate Point' }).click();
-
-    expect(locateRequests.length).toBeGreaterThan(0);
-
-    const locateRequest = locateRequests[0] as RouteApiRequest;
-    expect(locateRequest.method).toBe('POST');
-    expect(locateRequest.url).toMatch(
-      /https:\/\/valhalla1\.openstreetmap\.de\/locate/
-    );
-    expect(locateRequest.body).toBeDefined();
-    expect(locateRequest.body?.costing).toBe('bicycle');
-    expect(locateRequest.body?.locations).toStrictEqual([
-      { lat: 52.51830999999976, lon: 13.393706999999239 },
-    ]);
+    await expect(page.getByTestId('elevation-button')).toContainText('34 m');
   });
 
   test('should copy text to clipboard', async ({ page }) => {
@@ -554,6 +526,9 @@ test.describe('Map interactions with left context menu', () => {
       button: 'left',
     });
 
+    // Wait for popup to appear (has 200ms delay)
+    await expect(page.getByTestId('map-info-popup')).toBeVisible();
+
     await page.getByTestId('dd-copy-button').click();
 
     const clipboardContent = await page.evaluate(() =>

src/components/map/index.tsx

@@ -16,11 +16,7 @@ import 'maplibre-gl/dist/maplibre-gl.css';
 
 import axios from 'axios';
 import { throttle } from 'throttle-debounce';
-import {
-  getValhallaUrl,
-  buildHeightRequest,
-  buildLocateRequest,
-} from '@/utils/valhalla';
+import { getValhallaUrl, buildHeightRequest } from '@/utils/valhalla';
 import { buildHeightgraphData } from '@/utils/heightgraph';
 import HeightGraph from '@/components/heightgraph';
 import { DrawControl } from './draw-control';
@@ -89,12 +85,10 @@ export const MapComponent = () => {
   );
   const updateSettings = useCommonStore((state) => state.updateSettings);
   const setMapReady = useCommonStore((state) => state.setMapReady);
-  const { profile, style } = useSearch({ from: '/$activeTab' });
+  const { style } = useSearch({ from: '/$activeTab' });
   const [showInfoPopup, setShowInfoPopup] = useState(false);
   const [showContextPopup, setShowContextPopup] = useState(false);
-  const [isLocateLoading, setIsLocateLoading] = useState(false);
   const [isHeightLoading, setIsHeightLoading] = useState(false);
-  const [locate, setLocate] = useState([]);
   const [popupLngLat, setPopupLngLat] = useState<{
     lng: number;
     lat: number;
@@ -292,32 +286,6 @@ export const MapComponent = () => {
       });
   }, []);
 
-  const getLocate = useCallback(
-    (lng: number, lat: number) => {
-      setIsLocateLoading(true);
-      axios
-        .post(
-          getValhallaUrl() + '/locate',
-          buildLocateRequest({ lng, lat }, profile || 'bicycle'),
-          {
-            headers: {
-              'Content-Type': 'application/json',
-            },
-          }
-        )
-        .then(({ data }) => {
-          setLocate(data);
-        })
-        .catch(({ response }) => {
-          console.log(response);
-        })
-        .finally(() => {
-          setIsLocateLoading(false);
-        });
-    },
-    [profile]
-  );
-
   const handleAddWaypoint = useCallback(
     (index: number) => {
       if (!popupLngLat) return;
@@ -876,12 +844,8 @@ export const MapComponent = () => {
             popupLngLat={popupLngLat}
             elevation={elevation}
             isHeightLoading={isHeightLoading}
-            isLocateLoading={isLocateLoading}
-            locate={locate}
-            onLocate={getLocate}
             onClose={() => {
               setShowInfoPopup(false);
-              setLocate([]);
             }}
           />
         </Popup>

src/components/map/parts/map-info-popup.spec.tsx

@@ -11,9 +11,6 @@ const defaultProps = {
   popupLngLat: { lng: 10.123456, lat: 50.654321 },
   elevation: '250 m',
   isHeightLoading: false,
-  isLocateLoading: false,
-  locate: [],
-  onLocate: vi.fn(),
   onClose: vi.fn(),
 };
 
@@ -68,40 +65,14 @@ describe('MapInfoPopup', () => {
     expect(screen.getByTestId('elevation-button')).toHaveTextContent('500 m');
   });
 
-  it('should display Locate Point button', () => {
-    render(<MapInfoPopup {...defaultProps} />);
-
-    expect(screen.getByTestId('locate-point-button')).toHaveTextContent(
-      'Locate Point'
-    );
-  });
-
-  it('should display Valhalla Location JSON button', () => {
+  it('should display Valhalla Location JSON label', () => {
     render(<MapInfoPopup {...defaultProps} />);
 
     expect(screen.getByTestId('location-json-button')).toHaveTextContent(
       'Valhalla Location JSON'
     );
   });
 
-  it('should call onLocate when Locate Point is clicked', async () => {
-    const user = userEvent.setup();
-    const onLocate = vi.fn();
-    const popupLngLat = { lng: 10.5, lat: 50.5 };
-
-    render(
-      <MapInfoPopup
-        {...defaultProps}
-        onLocate={onLocate}
-        popupLngLat={popupLngLat}
-      />
-    );
-
-    await user.click(screen.getByTestId('locate-point-button'));
-
-    expect(onLocate).toHaveBeenCalledWith(10.5, 50.5);
-  });
-
   it('should format coordinates to 6 decimal places', () => {
     render(
       <MapInfoPopup
@@ -115,12 +86,20 @@ describe('MapInfoPopup', () => {
     );
   });
 
-  it('should have copy buttons for coordinate rows', () => {
+  it('should have copy buttons for coordinate rows with copyable values', () => {
     render(<MapInfoPopup {...defaultProps} />);
 
     expect(screen.getByTestId('dd-copy-button')).toBeInTheDocument();
     expect(screen.getByTestId('latlng-copy-button')).toBeInTheDocument();
     expect(screen.getByTestId('dms-copy-button')).toBeInTheDocument();
     expect(screen.getByTestId('location-json-copy-button')).toBeInTheDocument();
   });
+
+  it('should not have copy button for elevation (non-copyable value)', () => {
+    render(<MapInfoPopup {...defaultProps} />);
+
+    expect(
+      screen.queryByTestId('elevation-copy-button')
+    ).not.toBeInTheDocument();
+  });
 });

src/components/map/parts/map-info-popup.tsx

@@ -1,33 +1,19 @@
 import { Button } from '@/components/ui/button';
 import { CoordinateRow } from '@/components/ui/coordinate-row';
-import {
-  X,
-  Locate,
-  Globe,
-  Compass,
-  Cog,
-  MapPin,
-  ArrowUpDown,
-} from 'lucide-react';
+import { X, Locate, Globe, Compass, MapPin, ArrowUpDown } from 'lucide-react';
 import { convertDDToDMS } from '../utils';
 
 interface MapInfoPopupProps {
   popupLngLat: { lng: number; lat: number };
   elevation: string;
   isHeightLoading: boolean;
-  isLocateLoading: boolean;
-  locate: unknown[];
-  onLocate: (lng: number, lat: number) => void;
   onClose: () => void;
 }
 
 export function MapInfoPopup({
   popupLngLat,
   elevation,
   isHeightLoading,
-  isLocateLoading,
-  locate,
-  onLocate,
   onClose,
 }: MapInfoPopupProps) {
   const lngLatStr = `${popupLngLat.lng.toFixed(6)}, ${popupLngLat.lat.toFixed(6)}`;
@@ -43,12 +29,13 @@ export function MapInfoPopup({
   );
 
   return (
-    <div className="flex flex-col gap-2 p-2">
+    <div className="flex flex-col gap-2 px-4 py-6" data-testid="map-info-popup">
       <Button
         variant="ghost"
         size="icon-xs"
         onClick={onClose}
         className="absolute right-1 top-1"
+        aria-label="Close"
       >
         <X className="size-4" />
       </Button>
@@ -78,18 +65,7 @@ export function MapInfoPopup({
       />
 
       <CoordinateRow
-        label="Calls Valhalla's Locate API"
-        value="Locate Point"
-        copyText={JSON.stringify(locate)}
-        icon={<Cog className="size-3.5" />}
-        onClick={() => onLocate(popupLngLat.lng, popupLngLat.lat)}
-        isLoading={isLocateLoading}
-        copyDisabled={locate.length === 0}
-        testId="locate-point"
-      />
-
-      <CoordinateRow
-        label="Copies a Valhalla location object to clipboard for API requests"
+        label="Valhalla location object for API requests"
         value="Valhalla Location JSON"
         copyText={valhallaJson}
         icon={<MapPin className="size-3.5" />}

src/components/ui/coordinate-row.tsx

@@ -1,26 +1,23 @@
 import { type ReactNode } from 'react';
 import { Loader2 } from 'lucide-react';
-import { Button } from '@/components/ui/button';
-import { ButtonGroup } from '@/components/ui/button-group';
 import { CopyButton } from '@/components/ui/copy-button';
 import {
   Tooltip,
   TooltipContent,
   TooltipTrigger,
 } from '@/components/ui/tooltip';
+import { cn } from '@/lib/utils';
 
 interface CoordinateRowProps {
-  /** Tooltip content describing the button */
+  /** Tooltip content describing the label */
   label: string;
-  /** Display value/label shown in the button */
+  /** Display value/label shown */
   value: string;
   /** Text to copy to clipboard. If omitted, no copy button is shown */
   copyText?: string;
   /** Optional icon to show before the value */
   icon?: ReactNode;
-  /** Click handler for the main button */
-  onClick?: () => void;
-  /** Shows loading spinner and disables button */
+  /** Shows loading spinner */
   isLoading?: boolean;
   /** Disables the copy button */
   copyDisabled?: boolean;
@@ -33,37 +30,38 @@ function CoordinateRow({
   value,
   copyText,
   icon,
-  onClick,
   isLoading,
   copyDisabled,
   testId,
 }: CoordinateRowProps) {
   return (
-    <ButtonGroup>
+    <div
+      className={cn(
+        'flex items-center border rounded-lg gap-1.5',
+        copyText ? 'justify-between' : 'self-start'
+      )}
+    >
       <Tooltip>
         <TooltipTrigger asChild>
-          <Button
-            variant="outline"
-            size="sm"
-            onClick={onClick}
-            disabled={isLoading}
-            className="gap-1.5"
+          <div
+            className="flex items-center gap-1.5 px-2 py-1"
             data-testid={testId ? `${testId}-button` : undefined}
           >
             {isLoading ? <Loader2 className="size-3.5 animate-spin" /> : icon}
             {value}
-          </Button>
+          </div>
         </TooltipTrigger>
         <TooltipContent side="top">{label}</TooltipContent>
       </Tooltip>
       {copyText && (
         <CopyButton
+          variant="ghost"
           value={copyText}
           disabled={copyDisabled}
           data-testid={testId ? `${testId}-copy-button` : undefined}
         />
       )}
-    </ButtonGroup>
+    </div>
   );
 }