Improved the UX on Stations Page

2025-12-14 12:51:29 -06:00
parent 3852ec9f71
commit a9fc7741a6
9 changed files with 377 additions and 55 deletions
--- a/STATION-CHANGES.md
+++ b/STATION-CHANGES.md
@@ -67,21 +67,6 @@ Tradeoffs:
 - Slightly more frontend code, but minimal security risk.
 - Must ensure caching behavior is acceptable (browser cache won’t cache `blob:` URLs; rely on backend caching headers + client-side memoization).

-#### Option A2 (Simplest Code, Higher Risk): Make Photo Endpoint Public
-
-Why: Restores `<img>` behavior with minimal frontend work.
-
-Implementation outline:
- Backend: remove `preHandler: [fastify.authenticate]` from `/stations/photo/:reference`.
- Add lightweight protections to reduce abuse (choose as many as feasible without adding heavy deps):
-  - strict input validation (length/charset) for `reference`
-  - low maxWidth clamp and no arbitrary URL fetching
-  - maintain `Cache-Control` header (already present)
-  - optionally add server-side rate limit (only if repo already uses a rate-limit plugin; avoid introducing new infra unless necessary)
-
-Tradeoffs:
- Anyone can hit `/api/stations/photo/:reference` and spend your Google quota.
-
 ### Option B (Remove Images): Simplify Cards

 Why: If image delivery adds too much complexity or risk, remove images from station cards.
@@ -124,7 +109,7 @@ Important: some saved stations may have `latitude/longitude = 0` if cache miss;

 - Desktop saved list: add a “Navigate” icon button that opens a small menu with the 3 links (cleaner than inline links inside `ListItemText`).
  - File: `frontend/src/features/stations/components/SavedStationsList.tsx`
- Mobile bottom sheet (station details): add a “Navigate” section with the same 3 links (buttons or list items).
+- Mobile bottom sheet (station details): add a “Navigate” section with the same 3 links as buttons.
  - File: `frontend/src/features/stations/mobile/StationsMobileScreen.tsx`

 ## Work Breakdown for Multiple Agents
--- a/frontend/src/features/stations/tests/components/StationCard.test.tsx
+++ b/frontend/src/features/stations/tests/components/StationCard.test.tsx
@@ -8,6 +8,12 @@ import '@testing-library/jest-dom';
 import { StationCard } from '../../components/StationCard';
 import { Station } from '../../types/stations.types';

+jest.mock('@/core/api/client', () => ({
+  apiClient: {
+    get: jest.fn(() => Promise.resolve({ data: new Blob(['photo'], { type: 'image/jpeg' }) }))
+  }
+}));
+
 const mockStation: Station = {
  placeId: 'test-place-id',
  name: 'Shell Gas Station',
@@ -23,6 +29,14 @@ describe('StationCard', () => {
  beforeEach(() => {
    jest.clearAllMocks();
    window.open = jest.fn();
+
+    // JSDOM may not implement these; mock for blob URL handling
+    if (!global.URL.createObjectURL) {
+      global.URL.createObjectURL = jest.fn(() => 'blob:mock-url');
+    }
+    if (!global.URL.revokeObjectURL) {
+      global.URL.revokeObjectURL = jest.fn();
+    }
  });

  describe('Rendering', () => {
@@ -33,12 +47,12 @@ describe('StationCard', () => {
      expect(screen.getByText('123 Main St, San Francisco, CA 94105')).toBeInTheDocument();
    });

-    it('should render station photo if available', () => {
+    it('should render station photo if available', async () => {
      render(<StationCard station={mockStation} isSaved={false} />);

-      const photo = screen.getByAltText('Shell Gas Station');
+      const photo = await screen.findByAltText('Shell Gas Station');
      expect(photo).toBeInTheDocument();
-      expect(photo).toHaveAttribute('src', '/api/stations/photo/mock-photo-reference');
+      expect(photo).toHaveAttribute('src', expect.stringContaining('blob:'));
    });

    it('should render rating when available', () => {
--- a/frontend/src/features/stations/tests/utils/navigation-links.test.ts
+++ b/frontend/src/features/stations/tests/utils/navigation-links.test.ts
@@ -0,0 +1,34 @@
+import { buildNavigationLinks } from '../../utils/navigation-links';
+import { Station } from '../../types/stations.types';
+
+const baseStation: Station = {
+  placeId: 'place-123',
+  name: 'Test Station',
+  address: '123 Main St, City',
+  latitude: 37.7749,
+  longitude: -122.4194,
+  rating: 4.2
+};
+
+describe('buildNavigationLinks', () => {
+  it('uses coordinates when valid', () => {
+    const links = buildNavigationLinks(baseStation);
+
+    expect(links.google).toContain('destination=37.7749,-122.4194');
+    expect(links.google).toContain('destination_place_id=place-123');
+    expect(links.apple).toContain('daddr=37.7749,-122.4194');
+    expect(links.waze).toContain('ll=37.7749,-122.4194');
+  });
+
+  it('falls back to query when coordinates are missing', () => {
+    const links = buildNavigationLinks({
+      ...baseStation,
+      latitude: 0,
+      longitude: 0
+    });
+
+    expect(links.google).toContain('query=');
+    expect(links.apple).toContain('q=');
+    expect(links.waze).toContain('q=');
+  });
+});
--- a/frontend/src/features/stations/components/SavedStationsList.tsx
+++ b/frontend/src/features/stations/components/SavedStationsList.tsx
@@ -8,16 +8,18 @@ import {
  ListItem,
  ListItemButton,
  ListItemText,
-  ListItemSecondaryAction,
-  IconButton,
  Box,
  Typography,
  Chip,
  Divider,
  Alert,
-  Skeleton
+  Skeleton,
+  Menu,
+  MenuItem,
+  Button
 } from '@mui/material';
 import DeleteIcon from '@mui/icons-material/Delete';
+import NavigationIcon from '@mui/icons-material/Navigation';
 import { OctanePreference, SavedStation } from '../types/stations.types';
 import { formatDistance } from '../utils/distance';
 import {
@@ -27,6 +29,7 @@ import {
  resolveSavedStationPlaceId
 } from '../utils/savedStations';
 import { OctanePreferenceSelector } from './OctanePreferenceSelector';
+import { buildNavigationLinks } from '../utils/navigation-links';

 interface SavedStationsListProps {
  stations: SavedStation[];
@@ -47,6 +50,66 @@ export const SavedStationsList: React.FC<SavedStationsListProps> = ({
  onOctanePreferenceChange,
  octaneUpdatingId
 }) => {
+  const [navAnchorEl, setNavAnchorEl] = React.useState<null | HTMLElement>(null);
+  const [navStation, setNavStation] = React.useState<SavedStation | null>(null);
+
+  const handleOpenNavMenu = (event: React.MouseEvent<HTMLElement>, station: SavedStation) => {
+    event.stopPropagation();
+    setNavAnchorEl(event.currentTarget);
+    setNavStation(station);
+  };
+
+  const handleCloseNavMenu = () => {
+    setNavAnchorEl(null);
+    setNavStation(null);
+  };
+
+  const renderNavMenu = () => {
+    if (!navStation) {
+      return null;
+    }
+
+    const links = buildNavigationLinks(navStation);
+
+    return (
+      <Menu
+        anchorEl={navAnchorEl}
+        open={Boolean(navAnchorEl)}
+        onClose={handleCloseNavMenu}
+        anchorOrigin={{ vertical: 'bottom', horizontal: 'right' }}
+        transformOrigin={{ vertical: 'top', horizontal: 'right' }}
+      >
+        <MenuItem
+          component="a"
+          href={links.google}
+          target="_blank"
+          rel="noopener"
+          onClick={handleCloseNavMenu}
+        >
+          Navigate in Google
+        </MenuItem>
+        <MenuItem
+          component="a"
+          href={links.apple}
+          target="_blank"
+          rel="noopener"
+          onClick={handleCloseNavMenu}
+        >
+          Navigate in Apple Maps
+        </MenuItem>
+        <MenuItem
+          component="a"
+          href={links.waze}
+          target="_blank"
+          rel="noopener"
+          onClick={handleCloseNavMenu}
+        >
+          Navigate in Waze
+        </MenuItem>
+      </Menu>
+    );
+  };
+
  if (loading) {
    return (
      <List sx={{ width: '100%', bgcolor: 'background.paper' }}>
@@ -168,39 +231,53 @@ export const SavedStationsList: React.FC<SavedStationsListProps> = ({
                            value={octanePreference}
                            onChange={(value) => onOctanePreferenceChange?.(placeId, value)}
                            disabled={!onOctanePreferenceChange || octaneUpdatingId === placeId}
-                            helperText="Show on search cards"
                          />
                        </Box>
                      )}
-                    </Box>
-                  }
-                />
-              </ListItemButton>
-              <ListItemSecondaryAction>
-                <IconButton
-                  edge="end"
-                  aria-label="delete"
+                      <Box
+                        sx={{
+                          display: 'flex',
+                          gap: 1,
+                          mt: 1,
+                          flexWrap: 'wrap'
+                        }}
+                      >
+                        <Button
+                          variant="text"
+                          size="small"
+                          startIcon={<NavigationIcon />}
+                          onClick={(e) => handleOpenNavMenu(e, station)}
+                          sx={{ minHeight: 36 }}
+                        >
+                          Navigate
+                        </Button>
+                        <Button
+                          variant="text"
+                          size="small"
+                          color="error"
+                          startIcon={<DeleteIcon />}
                          onClick={(e) => {
                            e.stopPropagation();
                            if (placeId) {
                              onDeleteStation?.(placeId);
                            }
                          }}
-                  title="Delete saved station"
-                  sx={{
-                    minWidth: '44px',
-                    minHeight: '44px'
-                  }}
                          disabled={!placeId}
+                          sx={{ minHeight: 36 }}
                        >
-                  <DeleteIcon />
-                </IconButton>
-              </ListItemSecondaryAction>
+                          Delete
+                        </Button>
+                      </Box>
+                    </Box>
+                  }
+                />
+              </ListItemButton>
            </ListItem>
            {index < stations.length - 1 && <Divider />}
          </React.Fragment>
        );
      })}
+      {renderNavMenu()}
    </List>
  );
 };
--- a/frontend/src/features/stations/components/StationCard.tsx
+++ b/frontend/src/features/stations/components/StationCard.tsx
@@ -6,7 +6,6 @@ import React from 'react';
 import {
  Card,
  CardContent,
-  CardMedia,
  Typography,
  Chip,
  IconButton,
@@ -18,7 +17,7 @@ import BookmarkBorderIcon from '@mui/icons-material/BookmarkBorder';
 import DirectionsIcon from '@mui/icons-material/Directions';
 import { Station, SavedStation } from '../types/stations.types';
 import { formatDistance } from '../utils/distance';
-import { getStationPhotoUrl } from '../utils/photo-utils';
+import { StationPhoto } from './StationPhoto';

 interface StationCardProps {
  station: Station;
@@ -84,15 +83,7 @@ export const StationCard: React.FC<StationCardProps> = ({
        flexDirection: 'column'
      }}
    >
-      {station.photoReference && (
-        <CardMedia
-          component="img"
-          height="200"
-          image={getStationPhotoUrl(station.photoReference)}
-          alt={station.name}
-          sx={{ objectFit: 'cover' }}
-        />
-      )}
+      <StationPhoto photoReference={station.photoReference} alt={station.name} height={200} />

      <CardContent sx={{ flexGrow: 1 }}>
        {/* Station Name */}
--- a/frontend/src/features/stations/components/StationPhoto.tsx
+++ b/frontend/src/features/stations/components/StationPhoto.tsx
@@ -0,0 +1,105 @@
+/**
+ * @ai-summary Authenticated station photo loader using backend proxy
+ */
+
+import React, { useEffect, useState } from 'react';
+import { CardMedia, CircularProgress, Box } from '@mui/material';
+import { apiClient } from '@/core/api/client';
+import { getStationPhotoUrl } from '../utils/photo-utils';
+
+interface StationPhotoProps {
+  photoReference?: string;
+  alt: string;
+  height?: number | string;
+}
+
+export const StationPhoto: React.FC<StationPhotoProps> = ({ photoReference, alt, height = 200 }) => {
+  const [photoUrl, setPhotoUrl] = useState<string | null>(null);
+  const [isLoading, setIsLoading] = useState(false);
+
+  useEffect(() => {
+    let isMounted = true;
+    let objectUrl: string | null = null;
+
+    const loadPhoto = async () => {
+      if (!photoReference) {
+        setPhotoUrl(null);
+        return;
+      }
+
+      setIsLoading(true);
+
+      try {
+        const url = getStationPhotoUrl(photoReference);
+        if (!url) {
+          setPhotoUrl(null);
+          return;
+        }
+
+        const response = await apiClient.get(url, {
+          responseType: 'blob'
+        });
+
+        objectUrl = URL.createObjectURL(response.data);
+
+        if (isMounted) {
+          setPhotoUrl(objectUrl);
+        }
+      } catch (error) {
+        // Silent fail – just hide the image if the request is unauthorized or fails
+        if (isMounted) {
+          setPhotoUrl(null);
+        }
+      } finally {
+        if (isMounted) {
+          setIsLoading(false);
+        }
+      }
+    };
+
+    loadPhoto();
+
+    return () => {
+      isMounted = false;
+      if (objectUrl) {
+        URL.revokeObjectURL(objectUrl);
+      }
+    };
+  }, [photoReference]);
+
+  if (!photoReference) {
+    return null;
+  }
+
+  if (isLoading && !photoUrl) {
+    return (
+      <Box
+        sx={{
+          display: 'flex',
+          alignItems: 'center',
+          justifyContent: 'center',
+          height
+        }}
+        data-testid="station-photo-loading"
+      >
+        <CircularProgress size={24} />
+      </Box>
+    );
+  }
+
+  if (!photoUrl) {
+    return null;
+  }
+
+  return (
+    <CardMedia
+      component="img"
+      height={height}
+      image={photoUrl}
+      alt={alt}
+      sx={{ objectFit: 'cover' }}
+    />
+  );
+};
+
+export default StationPhoto;
--- a/frontend/src/features/stations/components/index.ts
+++ b/frontend/src/features/stations/components/index.ts
@@ -3,6 +3,7 @@
 */

 export { StationCard } from './StationCard';
+export { StationPhoto } from './StationPhoto';
 export { StationsList } from './StationsList';
 export { SavedStationsList } from './SavedStationsList';
 export { StationsSearchForm } from './StationsSearchForm';
--- a/frontend/src/features/stations/mobile/StationsMobileScreen.tsx
+++ b/frontend/src/features/stations/mobile/StationsMobileScreen.tsx
@@ -13,7 +13,8 @@ import {
  IconButton,
  Typography,
  Divider,
-  useTheme
+  useTheme,
+  Button
 } from '@mui/material';
 import SearchIcon from '@mui/icons-material/Search';
 import BookmarkIcon from '@mui/icons-material/Bookmark';
@@ -41,6 +42,7 @@ import {
  OctanePreference
 } from '../types/stations.types';
 import { octanePreferenceToFlags, resolveSavedStationPlaceId } from '../utils/savedStations';
+import { buildNavigationLinks } from '../utils/navigation-links';

 // Tab indices
 const TAB_SEARCH = 0;
@@ -178,6 +180,11 @@ export const StationsMobileScreen: React.FC = () => {
    // TODO: Implement pull-to-refresh
  }, []);

+  const navigationLinks = useMemo(
+    () => (selectedStation ? buildNavigationLinks(selectedStation) : null),
+    [selectedStation]
+  );
+
  return (
    <Box
      sx={{
@@ -414,6 +421,53 @@ export const StationsMobileScreen: React.FC = () => {
                  </Typography>
                </Box>
              )}
+
+              {navigationLinks && (
+                <Box>
+                  <Typography variant="caption" color="text.secondary">
+                    Navigate
+                  </Typography>
+                  <Box
+                    sx={{
+                      display: 'flex',
+                      flexWrap: 'wrap',
+                      gap: 1,
+                      mt: 1
+                    }}
+                  >
+                    <Button
+                      variant="outlined"
+                      size="small"
+                      component="a"
+                      href={navigationLinks.google}
+                      target="_blank"
+                      rel="noopener"
+                    >
+                      Navigate in Google
+                    </Button>
+                    <Button
+                      variant="outlined"
+                      size="small"
+                      component="a"
+                      href={navigationLinks.apple}
+                      target="_blank"
+                      rel="noopener"
+                    >
+                      Navigate in Apple Maps
+                    </Button>
+                    <Button
+                      variant="outlined"
+                      size="small"
+                      component="a"
+                      href={navigationLinks.waze}
+                      target="_blank"
+                      rel="noopener"
+                    >
+                      Navigate in Waze
+                    </Button>
+                  </Box>
+                </Box>
+              )}
            </Box>
          </Box>
        )}
--- a/frontend/src/features/stations/utils/navigation-links.ts
+++ b/frontend/src/features/stations/utils/navigation-links.ts
@@ -0,0 +1,61 @@
+/**
+ * @ai-summary Helpers to build navigation URLs for stations
+ */
+
+import { SavedStation, Station } from '../types/stations.types';
+
+type StationLike = Pick<Station, 'placeId' | 'name' | 'address' | 'latitude' | 'longitude'> & Partial<SavedStation>;
+
+const hasValidCoordinates = (station: StationLike): boolean => {
+  const { latitude, longitude } = station;
+  if (typeof latitude !== 'number' || typeof longitude !== 'number') {
+    return false;
+  }
+
+  if (Number.isNaN(latitude) || Number.isNaN(longitude)) {
+    return false;
+  }
+
+  if (latitude === 0 && longitude === 0) {
+    return false;
+  }
+
+  return true;
+};
+
+const getQuery = (station: StationLike): string => {
+  return encodeURIComponent(station.address || station.name);
+};
+
+export interface NavigationLinks {
+  google: string;
+  apple: string;
+  waze: string;
+}
+
+/**
+ * Build navigation URLs for Google Maps, Apple Maps, and Waze.
+ * Uses coordinates when available; falls back to address/name query.
+ */
+export const buildNavigationLinks = (station: StationLike): NavigationLinks => {
+  if (hasValidCoordinates(station)) {
+    const { latitude, longitude, placeId } = station;
+    const latLng = `${latitude},${longitude}`;
+
+    return {
+      google: `https://www.google.com/maps/dir/?api=1&destination=${latLng}${
+        placeId ? `&destination_place_id=${placeId}` : ''
+      }`,
+      apple: `https://maps.apple.com/?daddr=${latLng}`,
+      waze: `https://waze.com/ul?ll=${latLng}&navigate=yes`
+    };
+  }
+
+  const query = getQuery(station);
+
+  return {
+    google: `https://www.google.com/maps/search/?api=1&query=${query}`,
+    apple: `https://maps.apple.com/?q=${query}`,
+    waze: `https://waze.com/ul?q=${query}&navigate=yes`
+  };
+};