Add Geolocation#39
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull Request Overview
This PR adds a geolocation-based notification feature that reminds users to update their presence status when they are near the office. Users must opt-in to enable location tracking and notifications.
Key Changes:
- Database migration adds
geolocation_notifications_enabledpreference column (default: true) - New
GeolocationNotifiercomponent monitors user location and sends proximity-based notifications - UI toggle in menu bar allows users to enable/disable location reminders
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
supabase/migrations/20251022120000_add_geolocation_notifications_to_profiles.sql |
Adds boolean column for geolocation notification preference to profiles table |
docs/GEOLOCATION_FEATURE.md |
Comprehensive documentation covering feature overview, setup, testing, and troubleshooting |
components/icons.tsx |
Adds LocationOnIcon and LocationOffIcon components for the toggle UI |
components/Menu.tsx |
Implements geolocation toggle button with permission handling and state management |
app/page.tsx |
Integrates GeolocationNotifier component into main page |
app/GeolocationNotifier.tsx |
Core component implementing location tracking, distance calculation, and notification logic |
.env |
Adds office coordinate environment variables |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| @@ -0,0 +1,5 @@ | |||
| -- Add geolocation_notifications_enabled preference to profiles | |||
| alter table public.profiles | |||
| add column if not exists geolocation_notifications_enabled boolean not null default true; | |||
There was a problem hiding this comment.
The column defaults to true, but the documentation states 'Default State: Disabled (users must opt-in)' and the comment in line 5 says 'Users must opt-in to geolocation-based notifications'. This contradicts the opt-in requirement. Change default true to default false to ensure users explicitly consent before location tracking is enabled.
| add column if not exists geolocation_notifications_enabled boolean not null default true; | |
| add column if not exists geolocation_notifications_enabled boolean not null default false; |
| return R * c; | ||
| } | ||
|
|
||
| const OFFICE_RADIUS_METERS = 5000; // Notify when within 500m of office |
There was a problem hiding this comment.
The constant value is 5000 meters but the comment says 500m. This inconsistency appears throughout the documentation which states 500 meters. Update the value to 500 to match the intended behavior, or update all documentation to reflect 5000 meters if that is the correct radius.
| const OFFICE_RADIUS_METERS = 5000; // Notify when within 500m of office | |
| const OFFICE_RADIUS_METERS = 500; // Notify when within 500m of office |
No description provided.