Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions src/App.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { ThemeProvider } from './context';
import { ToastProvider, BottomNavigation } from './components/ui';
import Header from './components/Header.jsx';
import Footer from './components/Footer.jsx';
import ScrollToTop from './components/ScrollToTop.jsx';
import AppRoutes from './router/index.jsx';
import './App.css';

Expand Down Expand Up @@ -39,6 +40,7 @@ function App() {
<LanguageProvider>
<ToastProvider>
<Router>
<ScrollToTop />
<div className="App">
<Header />
<main className="main-content">
Expand Down
5 changes: 5 additions & 0 deletions src/components/Hero.css
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,11 @@
}

@media (max-width: 768px) {
/* Hide globe on mobile to prevent cutoff issues */
.hero__globe-container {
display: none !important;
Comment on lines +211 to +212
Copy link

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

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

The !important flag should be avoided unless absolutely necessary as it makes styles harder to override and maintain. If the globe container is appearing on mobile despite this rule, consider investigating the specificity issue in the original CSS rather than using !important.

Suggested change
.hero__globe-container {
display: none !important;
.hero .hero__globe-container {
display: none;

Copilot uses AI. Check for mistakes.
}

.hero--small {
min-height: 250px;
padding: var(--space-10) 0;
Expand Down
18 changes: 18 additions & 0 deletions src/components/ScrollToTop.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import { useEffect } from 'react';
import { useLocation } from 'react-router-dom';

/**
* ScrollToTop Component
* Scrolls to top of page on route changes
*/
const ScrollToTop = () => {
const { pathname } = useLocation();

useEffect(() => {
window.scrollTo(0, 0);
Copy link

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

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

The scroll behavior in this component doesn't respect user preferences for reduced motion. Consider checking prefers-reduced-motion media query and using window.scrollTo({top: 0, left: 0, behavior: 'auto'}) when users prefer reduced motion.

Suggested change
window.scrollTo(0, 0);
const prefersReducedMotion = window.matchMedia('(prefers-reduced-motion: reduce)').matches;
window.scrollTo({
top: 0,
left: 0,
behavior: prefersReducedMotion ? 'auto' : 'smooth'
});

Copilot uses AI. Check for mistakes.
}, [pathname]);

return null;
};

export default ScrollToTop;
2 changes: 1 addition & 1 deletion src/components/ui/BottomNavigation.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ const BottomNavigation = ({ isAuthenticated = false }) => {
{ path: '/recommendations', icon: 'star', label: 'Recs', requiresAuth: true },
{ path: '/favourites', icon: 'heart', label: 'Saved', requiresAuth: true },
{ path: '/navigation', icon: 'navigation', label: 'Navigate' },
{ path: isAuthenticated ? '/profile' : '/login', icon: 'user', label: isAuthenticated ? 'Profile' : 'Login' },
// Profile button removed - accessible via header user dropdown
];

const filteredItems = navItems.filter(item => !item.requiresAuth || isAuthenticated);
Expand Down
14 changes: 12 additions & 2 deletions src/pages/NavigationPage.jsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useState, useEffect, useCallback } from 'react';
import React, { useState, useEffect, useCallback, useRef } from 'react';
import { useSearchParams } from 'react-router-dom';
import { navigationAPI } from '../api';
import { useTranslation } from '../i18n';
Expand Down Expand Up @@ -42,6 +42,9 @@ const NavigationPage = () => {
const [routeGeometry, setRouteGeometry] = useState(null);
const [routeError, setRouteError] = useState(null); // For impossible routes

// Ref for auto-scrolling to results
const resultsRef = useRef(null);

// Reverse geocode when route data changes
const geocodeLocations = useCallback(async () => {
if (!routeData) return;
Expand Down Expand Up @@ -153,6 +156,13 @@ const NavigationPage = () => {
}

setRouteData(routeInfo);

// Auto-scroll to map on mobile after route calculation
setTimeout(() => {
if (resultsRef.current) {
resultsRef.current.scrollIntoView({ behavior: 'smooth', block: 'start' });
Copy link

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

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

The auto-scroll behavior may be disorienting for users with vestibular disorders or motion sensitivity. Consider respecting the prefers-reduced-motion media query. You can check window.matchMedia('(prefers-reduced-motion: reduce)').matches and conditionally set behavior: 'auto' instead of 'smooth', or skip the scroll entirely for users who prefer reduced motion.

Suggested change
resultsRef.current.scrollIntoView({ behavior: 'smooth', block: 'start' });
const prefersReducedMotion = window.matchMedia && window.matchMedia('(prefers-reduced-motion: reduce)').matches;
resultsRef.current.scrollIntoView({ behavior: prefersReducedMotion ? 'auto' : 'smooth', block: 'start' });

Copilot uses AI. Check for mistakes.
}
}, 100);
Comment on lines +161 to +165
Copy link

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

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

[nitpick] The 100ms timeout appears arbitrary. Consider using requestAnimationFrame or a longer delay to ensure the DOM has fully updated and rendered before attempting to scroll. Alternatively, document why 100ms was chosen to help future maintainers.

Suggested change
setTimeout(() => {
if (resultsRef.current) {
resultsRef.current.scrollIntoView({ behavior: 'smooth', block: 'start' });
}
}, 100);
requestAnimationFrame(() => {
if (resultsRef.current) {
resultsRef.current.scrollIntoView({ behavior: 'smooth', block: 'start' });
}
});

Copilot uses AI. Check for mistakes.
} catch (err) {
setError(err.message || t('errorCalculatingRoute'));
console.error('Error calculating route:', err);
Expand Down Expand Up @@ -320,7 +330,7 @@ const NavigationPage = () => {
</div>

{/* Results */}
<div className="results-column">
<div className="results-column" ref={resultsRef}>
{routeData ? (
<div className="route-results-card animate-fadeInUp">
<div className="card-title">
Expand Down
45 changes: 41 additions & 4 deletions src/pages/PlaceDetailsPage.css
Original file line number Diff line number Diff line change
Expand Up @@ -646,7 +646,8 @@

@media (max-width: 768px) {
.place-hero {
padding: var(--space-12) 0 var(--space-16);
padding: var(--space-12) 0 var(--space-24);
/* Increased bottom padding for action buttons */
}

.place-title {
Expand All @@ -659,11 +660,19 @@
}

.place-actions-bar {
flex-direction: column;
flex-direction: row;
/* Keep horizontal on mobile */
flex-wrap: wrap;
justify-content: center;
margin-top: calc(-1 * var(--space-24));
/* Adjust overlap */
}

.place-actions-bar button {
width: 100%;
.place-actions-bar button,
.place-actions-bar a {
flex: 1 1 auto;
min-width: 100px;
max-width: 150px;
}

.place-description-card,
Expand All @@ -689,4 +698,32 @@
.star-btn {
font-size: 1.5rem;
}
}

/* Extra small screens (iPhone SE, small phones) */
@media (max-width: 480px) {
.place-hero {
padding: var(--space-10) 0 calc(var(--space-32) + var(--space-20));
Copy link

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

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

[nitpick] The complex calc expression calc(var(--space-32) + var(--space-20)) makes it difficult to understand the intended spacing. Consider using a CSS variable (e.g., --place-hero-bottom-padding-small: calc(var(--space-32) + var(--space-20))) to make the intent clearer and improve maintainability.

Suggested change
padding: var(--space-10) 0 calc(var(--space-32) + var(--space-20));
--place-hero-bottom-padding-small: calc(var(--space-32) + var(--space-20));
padding: var(--space-10) 0 var(--place-hero-bottom-padding-small);

Copilot uses AI. Check for mistakes.
/* Increased bottom padding for 3 stacked buttons */
}

.place-title {
font-size: var(--text-2xl);
}

.place-actions-bar {
flex-direction: column;
/* Stack vertically on very small screens */
align-items: stretch;
gap: var(--space-2);
margin-top: calc(-1 * (var(--space-32) + var(--space-16)));
Copy link

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

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

[nitpick] The complex negative calc expression calc(-1 * (var(--space-32) + var(--space-16))) is difficult to understand and maintain. Consider using a CSS variable for this value to improve code clarity.

Copilot uses AI. Check for mistakes.
/* Move buttons higher up to stay inside hero */
}

.place-actions-bar button,
.place-actions-bar a {
min-width: auto;
max-width: none;
width: 100%;
}
}
3 changes: 2 additions & 1 deletion src/pages/RecommendationsPage.css
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@
}

.filters-bar--open {
Copy link

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider adding a comment explaining why 500px was chosen as the max-height value. This helps future maintainers understand if this value needs adjustment based on different filter configurations.

Suggested change
.filters-bar--open {
.filters-bar--open {
/*
* max-height set to 500px to ensure all filter options and the Apply button are visible
* when the filter bar is expanded, especially on mobile devices. Adjust if filter content changes.
*/

Copilot uses AI. Check for mistakes.
max-height: 200px;
max-height: 500px;
/* Increased for mobile to show Apply button */
opacity: 1;
margin-top: var(--space-4);
}
Expand Down
Loading