feat: dark mode, matched chart/map sizes, responsive layout, privacy-friendly location prompt - #35
Conversation
…friendly location prompt - Add dark mode via CSS custom properties and prefers-color-scheme - Match chart and map box heights using align-items: stretch + flex layout - Chart uses maintainAspectRatio: false to fill container - Add responsive breakpoints for 768px and 480px viewports - Replace auto location request with explicit opt-in buttons - Add inline manual address input with geocoding support - Add privacy disclaimer explaining location permission usage - Show helpful error messages when location is denied Co-Authored-By: LCS <lcs.recovery693@passinbox.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
Review changes with Changed Files
|
|
Seems you are using me but didn't get OPENAI_API_KEY seted in Variables/Secrets for this repo. you could follow readme for more information |
❌ Deploy Preview for larme failed.
|
|
Skipping PR review because a bot author is detected. If you want to trigger CodeAnt AI, comment |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Reviewer's GuideImplements dark-mode theming via CSS variables, synchronizes chart/map sizing with flexible containers, adds responsive layout breakpoints, and replaces automatic geolocation with an explicit, privacy-friendly location selection flow (auto-detect or manual input with geocoding). Sequence diagram for the new privacy-friendly location selection flowsequenceDiagram
actor User
participant Dashboard
participant Geolocation as navigator.geolocation
participant Nominatim as NominatimAPI
User->>Dashboard: click handleGeolocation
Dashboard->>Dashboard: setLocationLoading(true)
Dashboard->>Dashboard: setError(null)
Dashboard->>Geolocation: getCurrentPosition(success, error)
alt geolocation success
Geolocation-->>Dashboard: success({ coords })
Dashboard->>Dashboard: setCoords({ lat, lon })
Dashboard->>Dashboard: setLocationLoading(false)
else geolocation error
Geolocation-->>Dashboard: error(err)
Dashboard->>Dashboard: setError(msgs[err.code])
Dashboard->>Dashboard: setShowManualInput(true)
Dashboard->>Dashboard: setLocationLoading(false)
end
User->>Dashboard: click Enter Location Manually
Dashboard->>Dashboard: setShowManualInput(true)
User->>Dashboard: click handleManualSubmit / press Enter
Dashboard->>Dashboard: setError(null)
Dashboard->>Dashboard: setLocationLoading(true)
alt input is lat, lon
Dashboard->>Dashboard: setCoords({ lat, lon })
Dashboard->>Dashboard: setLocationLoading(false)
else input is place name
Dashboard->>Nominatim: fetch(search?q=input)
Nominatim-->>Dashboard: results
alt results found
Dashboard->>Dashboard: setCoords({ lat, lon })
else no results
Dashboard->>Dashboard: setError("Location not found...")
end
Dashboard->>Dashboard: setLocationLoading(false)
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
View changes in DiffLens |
|
View changes in DiffLens |
PR Summary
|
|
View changes in DiffLens |
|
Seems you are using me but didn't get OPENAI_API_KEY seted in Variables/Secrets for this repo. you could follow readme for more information |
|
View changes in DiffLens |
![codereviewbot-ai[bot]](https://avatars.githubusercontent.com/in/309638?s=60&v=4){kind=small}
| border-radius: 0.75rem; | ||
| overflow: hidden; | ||
| } | ||
|
|
||
| @media (max-width: 768px) { | ||
| .leaflet-container { | ||
| height: 280px; | ||
| } | ||
| } | ||
|
|
||
| @media (max-width: 480px) { | ||
| .leaflet-container { | ||
| height: 220px; | ||
| } | ||
| } |
There was a problem hiding this comment.
Maintainability Concern:
The .leaflet-container heights are hardcoded for each breakpoint (350px, 280px, 220px). If the design requirements change, these values must be updated in multiple places, increasing the risk of inconsistency and maintenance overhead.
Recommendation:
Consider using CSS custom properties for the container height, or use relative units (e.g., vh, em, or %) to improve flexibility and maintainability. For example:
:root {
--leaflet-height-desktop: 350px;
--leaflet-height-tablet: 280px;
--leaflet-height-mobile: 220px;
}
.leaflet-container {
height: var(--leaflet-height-desktop);
}
@media (max-width: 768px) {
.leaflet-container {
height: var(--leaflet-height-tablet);
}
}
@media (max-width: 480px) {
.leaflet-container {
height: var(--leaflet-height-mobile);
}
}This approach centralizes the height values and makes future changes easier.
| { label: 'Forecast AQI', data: fore.map((d) => d.aqi), borderColor: 'green', tension: 0.4 }, | ||
| ], |
There was a problem hiding this comment.
The code assumes that all elements in the 'hist' and 'fore' arrays have valid 'dt' and 'aqi' properties. If any element is missing these properties or is malformed, this will result in runtime errors or invalid chart data.
Recommended solution:
Add validation or filtering to ensure that only objects with valid 'dt' (number) and 'aqi' (number) properties are included in the mapping operations. For example:
const safeHist = hist.filter(d => d && typeof d.dt === 'number' && typeof d.aqi === 'number');
const safeFore = fore.filter(d => d && typeof d.dt === 'number' && typeof d.aqi === 'number');Then use 'safeHist' and 'safeFore' in place of 'hist' and 'fore' in the data object.
| const handleManualSubmit = async () => { | ||
| const input = manualInput.trim(); | ||
| if (!input) return; | ||
| setError(null); | ||
| setLocationLoading(true); | ||
|
|
||
| const parts = input.split(",").map((s) => s.trim()); | ||
| if (parts.length === 2 && !isNaN(+parts[0]) && !isNaN(+parts[1])) { | ||
| setCoords({ lat: +parts[0], lon: +parts[1] }); | ||
| setLocationLoading(false); | ||
| return; | ||
| } | ||
|
|
||
| try { | ||
| const resp = await fetch( | ||
| `https://nominatim.openstreetmap.org/search?q=${encodeURIComponent(input)}&format=json&limit=1`, | ||
| ); | ||
| const results = await resp.json(); | ||
| if (!results?.length) { | ||
| setError("Location not found. Try another name or enter coordinates (lat, lon)."); | ||
| } else { | ||
| setCoords({ | ||
| lat: parseFloat(results[0].lat), | ||
| lon: parseFloat(results[0].lon), | ||
| }); | ||
| } | ||
| } catch { | ||
| setError("Geocoding failed. Please try again."); | ||
| } | ||
| setLocationLoading(false); | ||
| }; |
There was a problem hiding this comment.
Potential for Excessive Geocoding Requests
The manual input handler does not throttle or debounce geocoding requests. If the user presses Enter multiple times or rapidly, multiple fetch requests will be sent to the geocoding API, potentially leading to rate limiting or degraded performance.
Recommendation:
- Disable the input/button while
locationLoadingis true to prevent multiple submissions. - Alternatively, implement a debounce mechanism to limit request frequency.
Example:
if (locationLoading) return;Add this check at the start of handleManualSubmit.
|
View changes in DiffLens |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ❌ Deployment failed View logs |
airmerge | f267730 | Jun 21 2026, 06:13 PM |
Vulnerable Libraries (1)
More info on how to fix Vulnerable Libraries in JavaScript. 👉 Go to the dashboard for detailed results. 📥 Happy? Share your feedback with us. |
| const data = { | ||
| labels: [...hist, ...fore].map((d) => new Date(d.dt * 1000).toLocaleString()), | ||
| labels: [...hist, ...fore].map((d) => | ||
| new Date(d.dt * 1000).toLocaleString(), | ||
| ), | ||
| datasets: [ | ||
| { label: 'Historical AQI', data: hist.map((d) => d.aqi), borderColor: 'blue', tension: 0.4 }, | ||
| { label: 'Forecast AQI', data: fore.map((d) => d.aqi), borderColor: 'green', tension: 0.4 }, | ||
| { | ||
| label: "Historical AQI", | ||
| data: hist.map((d) => d.aqi), | ||
| borderColor: "blue", | ||
| tension: 0.4, | ||
| }, | ||
| { | ||
| label: "Forecast AQI", | ||
| data: fore.map((d) => d.aqi), | ||
| borderColor: "green", | ||
| tension: 0.4, | ||
| }, | ||
| ], | ||
| }; |
There was a problem hiding this comment.
There is a potential for a mismatch between the chart labels and the dataset values. The labels array is constructed by concatenating 'hist' and 'fore', but the datasets are kept separate (one for 'hist', one for 'fore'). If 'hist' and 'fore' are not contiguous in time or have different lengths, this could result in a chart where the data points do not align correctly with their labels, leading to misleading visualizations or rendering issues.
Recommended solution:
Ensure that the labels and datasets are aligned in length and order. If you intend to display two separate lines, consider using only the respective time ranges for each dataset's labels, or pad the datasets with nulls to align with the combined labels array.
| if (parts.length === 2 && !isNaN(+parts[0]) && !isNaN(+parts[1])) { | ||
| setCoords({ lat: +parts[0], lon: +parts[1] }); |
There was a problem hiding this comment.
Lack of Latitude/Longitude Range Validation
When the user enters coordinates manually, the code checks if both values are numbers but does not validate their ranges. Latitude should be between -90 and 90, and longitude between -180 and 180. Setting invalid coordinates may cause downstream errors in map rendering or API requests.
Recommendation:
Add range validation before setting coordinates:
const lat = +parts[0];
const lon = +parts[1];
if (lat >= -90 && lat <= 90 && lon >= -180 && lon <= 180) {
setCoords({ lat, lon });
setLocationLoading(false);
return;
} else {
setError("Coordinates out of range. Latitude must be -90 to 90, longitude -180 to 180.");
setLocationLoading(false);
return;
}
|
View changes in DiffLens |
 |
|
Overall Grade |
Security Reliability  Complexity Hygiene |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| Ruby |  |
Jun 21, 2026 6:12p.m. | Review ↗ |
| Rust | |
Jun 21, 2026 6:12p.m. | Review ↗ |
| JavaScript |  |
Jun 21, 2026 6:12p.m. | Review ↗ |
| Scala | |
Jun 21, 2026 6:12p.m. | Review ↗ |
| Shell | |
Jun 21, 2026 6:12p.m. | Review ↗ |
| Secrets | |
Jun 21, 2026 6:12p.m. | Review ↗ |
| Terraform | |
Jun 21, 2026 6:12p.m. | Review ↗ |
| Swift | |
Jun 21, 2026 6:12p.m. | Review ↗ |
| SQL | |
Jun 21, 2026 6:12p.m. | Review ↗ |
| Code coverage |  |
Jun 21, 2026 6:12p.m. | Review ↗ |
| C & C++ | |
Jun 21, 2026 6:12p.m. | Review ↗ |
| C# | |
Jun 21, 2026 6:12p.m. | Review ↗ |
| Ansible | |
Jun 21, 2026 6:12p.m. | Review ↗ |
Important
AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.
|
2 new issues
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The Nominatim geocoding request in
handleManualSubmitshould include a descriptiveUser-Agentheader (and ideallyReferer) to comply with their usage policy and avoid unexpected request blocking. - For consistency with the new theming system, consider moving the inline styles in the location prompt (e.g., the
<p>underh2) and remaining hardcoded colors in.location-errorinto CSS classes using the same CSS custom properties.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The Nominatim geocoding request in `handleManualSubmit` should include a descriptive `User-Agent` header (and ideally `Referer`) to comply with their usage policy and avoid unexpected request blocking.
- For consistency with the new theming system, consider moving the inline styles in the location prompt (e.g., the `<p>` under `h2`) and remaining hardcoded colors in `.location-error` into CSS classes using the same CSS custom properties.
## Individual Comments
### Comment 1
<location path="components/Dashboard.css" line_range="418-425" />
<code_context>
+ color: var(--text-primary);
+}
+
+.location-error {
+ margin-top: 0.5rem;
+ padding: 0.5rem 0.75rem;
+ background: rgba(204, 0, 51, 0.08);
+ border: 1px solid rgba(204, 0, 51, 0.2);
+ border-radius: 0.5rem;
+ font-size: 0.9rem;
+ color: #cc0033;
+}
+
</code_context>
<issue_to_address>
**suggestion:** Align the location error styling with your CSS variable theming to avoid hardcoded light-theme colors.
These styles still hardcode light-theme reds instead of using your CSS variables, so they may look out of place in dark mode. Please introduce error color variables (e.g. `--error-bg`, `--error-border`, `--error-text`) in `:root` and the dark-mode block, and reference those here instead of the fixed values.
Suggested implementation:
```
.location-disclaimer strong {
color: var(--text-primary);
}
.location-error {
margin-top: 0.5rem;
padding: 0.5rem 0.75rem;
background: var(--error-bg);
border: 1px solid var(--error-border);
border-radius: 0.5rem;
font-size: 0.9rem;
color: var(--error-text);
}
.dashboard button:hover {
```
To fully implement your suggestion, you also need to:
1. Define error color variables in your global theme, e.g. in `:root`:
- `--error-bg`
- `--error-border`
- `--error-text`
2. Add corresponding overrides in your dark-mode block (e.g. `[data-theme="dark"]` or `.dark`), ensuring the error colors are adjusted for dark backgrounds.
3. Optionally align the font-size (`0.9rem`) with any existing alert/error text sizing variables if your design system already defines them.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| .location-error { | ||
| margin-top: 0.5rem; | ||
| padding: 0.5rem 0.75rem; | ||
| background: rgba(204, 0, 51, 0.08); | ||
| border: 1px solid rgba(204, 0, 51, 0.2); | ||
| border-radius: 0.5rem; | ||
| font-size: 0.9rem; | ||
| color: #cc0033; |
There was a problem hiding this comment.
suggestion: Align the location error styling with your CSS variable theming to avoid hardcoded light-theme colors.
These styles still hardcode light-theme reds instead of using your CSS variables, so they may look out of place in dark mode. Please introduce error color variables (e.g. --error-bg, --error-border, --error-text) in :root and the dark-mode block, and reference those here instead of the fixed values.
Suggested implementation:
.location-disclaimer strong {
color: var(--text-primary);
}
.location-error {
margin-top: 0.5rem;
padding: 0.5rem 0.75rem;
background: var(--error-bg);
border: 1px solid var(--error-border);
border-radius: 0.5rem;
font-size: 0.9rem;
color: var(--error-text);
}
.dashboard button:hover {
To fully implement your suggestion, you also need to:
- Define error color variables in your global theme, e.g. in
:root:--error-bg--error-border--error-text
- Add corresponding overrides in your dark-mode block (e.g.
[data-theme="dark"]or.dark), ensuring the error colors are adjusted for dark backgrounds. - Optionally align the font-size (
0.9rem) with any existing alert/error text sizing variables if your design system already defines them.
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| Compatibility | 2 medium 1 high |
| BestPractice | 26 medium 6 minor 2 high |
| ErrorProne | 2 medium 4 high |
| CodeStyle | 47 minor |
| Complexity | 1 minor |
🟢 Metrics 6 complexity · 0 duplication
Metric Results Complexity 6 Duplication 0
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
🚩 AQI fetch error is invisible on the loading screen
When coords is set and the AQI fetch fails (.catch at line 130-133), setError("Failed to fetch AQI data") and setAq(null) are called. Since coords is non-null and aq is null, the component renders the loading/skeleton state (lines 192-210), which does not display the error message. The user sees an infinite loading spinner with no feedback. This is a pre-existing issue not introduced by this PR, but the new location prompt flow makes it more noticeable since users now actively choose their location before seeing the loading screen.
(Refers to lines 192-210)
Was this helpful? React with 👍 or 👎 to provide feedback.
| </div> | ||
|
|
||
| {showManualInput && ( |
There was a problem hiding this comment.
🟡 Enter key on manual input bypasses locationLoading disabled guard, enabling race condition with pending geolocation
The "Go" button correctly uses disabled={locationLoading} at components/Dashboard.tsx:172 to prevent manual submission while geolocation is in progress. However, the onKeyDown handler on the input at components/Dashboard.tsx:168-170 calls handleManualSubmit() unconditionally when Enter is pressed, without checking locationLoading. Additionally, the "Enter Location Manually" toggle button at line 156 is never disabled.
This means a user can: (1) click "Use My Location" (starts geolocation, sets locationLoading=true), (2) click the toggle to show manual input, (3) type a city name and press Enter — bypassing the disabled button guard. If the manual submit sets coords, and the browser's geolocation callback resolves later, it will call setCoords again with different coordinates, silently switching the dashboard to a different location and re-triggering the AQI data fetch via the useEffect at components/Dashboard.tsx:86.
| </div> | |
| {showManualInput && ( | |
| onKeyDown={(e) => { | |
| if (e.key === "Enter" && !locationLoading) handleManualSubmit(); | |
| }} |
Was this helpful? React with 👍 or 👎 to provide feedback.
| const handleGeolocation = () => { | ||
| if (!navigator.geolocation) { | ||
| setError("Geolocation is not supported by this browser."); | ||
| setShowManualInput(true); | ||
| return; | ||
| } | ||
| setLocationLoading(true); | ||
| setError(null); | ||
| navigator.geolocation.getCurrentPosition( | ||
| ({ coords: { latitude, longitude } }) => { | ||
| setCoords({ lat: latitude, lon: longitude }); | ||
| setLocationLoading(false); | ||
| }, | ||
| (err) => { | ||
| setLocationLoading(false); | ||
| const msgs: Record<number, string> = { | ||
| 1: "Permission Denied. Please allow location access in your browser settings.", | ||
| 2: "Location Information is Unavailable.", | ||
| 3: "The request to get your location timed out.", | ||
| 1: "Location permission was denied. You can enter your location manually below.", | ||
| 2: "Location information is unavailable. Please enter your location manually.", | ||
| 3: "Location request timed out. Please try again or enter manually.", | ||
| }; | ||
| setError(msgs[err.code] ?? "An Unknown Error Occurred."); | ||
| setError(msgs[err.code] ?? "An unknown error occurred."); | ||
| setShowManualInput(true); | ||
| }, | ||
| ); | ||
| }, []); | ||
| }; |
There was a problem hiding this comment.
🚩 Behavioral change: geolocation no longer auto-triggers on mount
The old code ran navigator.geolocation.getCurrentPosition inside a useEffect([], []) on mount, so location detection started automatically. The new code requires the user to explicitly click "Use My Location". This is a significant UX change — users who previously saw data immediately after granting location permission will now always land on the location prompt screen first. This appears intentional based on the privacy-focused redesign, but reviewers should confirm this is the desired first-visit experience.
Was this helpful? React with 👍 or 👎 to provide feedback.
| labels: [...hist, ...fore].map((d) => | ||
| new Date(d.dt * 1000).toLocaleString(), | ||
| ), | ||
| datasets: [ |
There was a problem hiding this comment.
📝 Info: Chart.js maintainAspectRatio: false relies on grid stretch for sizing
Setting maintainAspectRatio: false in ChartComp.tsx:17 means Chart.js depends entirely on the parent container's dimensions for canvas sizing. The .chart-wrapper uses flex: 1; min-height: 0; on desktop, getting its height from the grid's align-items: stretch which matches the map container's height (driven by .map-wrapper's min-height: 250px). On mobile (max-width: 768px), .chart-wrapper gets an explicit min-height: 250px. This should work correctly in practice, but if the map container were ever removed or hidden, the chart would collapse to zero height on desktop.
Was this helpful? React with 👍 or 👎 to provide feedback.
| const parts = input.split(",").map((s) => s.trim()); | ||
| if (parts.length === 2 && !isNaN(+parts[0]) && !isNaN(+parts[1])) { | ||
| setCoords({ lat: +parts[0], lon: +parts[1] }); | ||
| setLocationLoading(false); | ||
| return; |
There was a problem hiding this comment.
📝 Info: Coordinate parsing treats empty-string parts as valid zero values
At line 61, the check !isNaN(+parts[0]) && !isNaN(+parts[1]) passes for empty strings because +"" evaluates to 0 and isNaN(0) is false. An input like "," would set coordinates to {lat: 0, lon: 0} (Gulf of Guinea). This is a pre-existing issue — the same logic existed in the old window.prompt handler — but the inline input makes it marginally easier to trigger accidentally. A fix would be to additionally check that the trimmed parts are non-empty: parts[0] !== '' && parts[1] !== ''.
Was this helpful? React with 👍 or 👎 to provide feedback.
![llamapreview[bot]](https://avatars.githubusercontent.com/in/1023541?s=60&v=4){kind=small}
There was a problem hiding this comment.
AI Code Review by LlamaPReview
🎯 TL;DR & Recommendation
Recommendation: Approve with suggestions
This PR modernizes the dashboard with dark mode, responsive layout, and a privacy-friendly location flow. The changes are well-structured but three maintainability issues should be addressed before merging.
📄 Documentation Diagram
This diagram documents the refactored location selection flow, highlighting the explicit opt-in mechanism.
sequenceDiagram
participant User
participant Dashboard
participant Browser
participant Nominatim
User->>Dashboard: Click "Use My Location"
Dashboard->>Browser: navigator.geolocation.getCurrentPosition()
alt Success
Browser-->>Dashboard: {coords}
Dashboard->>Dashboard: Set coords
else Error
Browser-->>Dashboard: Permission denied/timeout
Dashboard->>Dashboard: Show error & manual input
end
User->>Dashboard: Enter location manually
Dashboard->>Nominatim: GET /search?q=...
Nominatim-->>Dashboard: Geocoding result
Dashboard->>Dashboard: Set coords
Dashboard->>API: Fetch AQI data
note over Dashboard: PR #35;35: Explicit opt-in flow replaces auto-request
🌟 Strengths
- Dark mode implementation using CSS custom properties for consistent theming across light/dark modes.
- Privacy-friendly location flow replaces automatic geolocation with an explicit opt-in, improving user trust.
| Priority | File | Category | Impact Summary (≤12 words) | Anchors |
|---|---|---|---|---|
| P2 | components/Dashboard.tsx | Maintainability | Nominatim API call missing User-Agent, risks rate-limiting. | method:handleManualSubmit |
| P2 | components/Dashboard.css | Maintainability | Hardcoded error color breaks dark mode theming. | |
| P2 | app/globals.css | Maintainability | Dead CSS rules for leaflet-container, never applied. |
💡 Have feedback? We'd love to hear it in our GitHub Discussions.
✨ This review was generated by LlamaPReview Advanced, which is free for all open-source projects. Learn more.
| const resp = await fetch( | ||
| `https://nominatim.openstreetmap.org/search?q=${encodeURIComponent(input)}&format=json&limit=1`, | ||
| ); | ||
| const results = await resp.json(); |
There was a problem hiding this comment.
P2 | Confidence: High
The handleManualSubmit function calls the Nominatim API without a User-Agent header or an optional email parameter as required by the Nominatim usage policy (see https://operations.osmfoundation.org/policies/nominatim/). Without a custom header, the request may be rate-limited, blocked, or fail in production. Additionally, the response resp.json() is called without checking resp.ok first; if the API returns an HTTP error (e.g., 429, 502), the code will attempt to parse an error body as JSON, which may throw and be caught by the generic catch block, leading to a misleading “Geocoding failed” error message. Both issues degrade reliability and transparency for the user.
| const resp = await fetch( | |
| `https://nominatim.openstreetmap.org/search?q=${encodeURIComponent(input)}&format=json&limit=1`, | |
| ); | |
| const results = await resp.json(); | |
| const resp = await fetch( | |
| `https://nominatim.openstreetmap.org/search?q=${encodeURIComponent(input)}&format=json&limit=1`, | |
| { | |
| headers: { | |
| 'User-Agent': 'AirMerge/1.0 (air-quality-app)', | |
| }, | |
| }, | |
| ); | |
| if (!resp.ok) { | |
| setError(`Geocoding API error (${resp.status}). Please try again later.`); | |
| return; | |
| } | |
| const results = await resp.json(); |
Evidence: method:handleManualSubmit
| color: var(--text-primary); | ||
| } | ||
|
|
||
| .location-error { |
There was a problem hiding this comment.
P2 | Confidence: High
The .location-error style uses a hardcoded hex color #cc0033 instead of a CSS custom property. This breaks the overall theming system introduced in this PR: in dark mode, the error text will appear as bright red on a dark background, which may be harsh or inconsistent with other themed elements (e.g., --text-muted, --text-primary). All error/focus states should use the same theme variables to ensure visual cohesion and easy future maintenance.
Code Suggestion:
.location-error {
color: var(--error-color, #cc0033);
}| color: var(--text-primary); | ||
| -webkit-font-smoothing: antialiased; | ||
| -moz-osx-font-smoothing: grayscale; | ||
| } |
There was a problem hiding this comment.
[Contextual Comment]
This comment refers to code near real line 66. Anchored to nearest_changed(64) line 64.
P2 | Confidence: High
The globals.css file sets fixed heights for .leaflet-container at breakpoints. However, MapComp.tsx passes an inline style={{ height: "100%", width: "100%" }} to its MapContainer, which takes precedence via CSS specificity. As a result, these CSS-based height rules never take effect – the map always fills its flex‑based wrapper (flex: 1; min-height: 250px in .map-wrapper). This is dead code that adds confusion and sets an incorrect contract for future developers expecting the map height to be controlled here. It should either be removed or refactored to set a default via the wrapper instead.
Code Suggestion:
/* Remove the .leaflet-container height rules entirely.
The map dimensions are now controlled by the flex layout in Dashboard.css:
.map-container .map-wrapper { flex: 1; min-height: 250px; }
MapComp uses inline style height:100% which will fill the wrapper. */
1 participant
Summary
Four UI enhancements to the AirMerge dashboard:
1. Matched chart & map box sizes
.viz-rownow usesalign-items: stretchso both columns share the same heightdisplay: flex; flex-direction: columnwith the content area (chart-wrapper/map-wrapper) set toflex: 1ChartCompsetsmaintainAspectRatio: falseso Chart.js fills its container instead of computing height from width2. Dark mode
All hardcoded colors replaced with CSS custom properties (
--card-bg,--text-primary,--border-color, etc.) defined in:rootand overridden in@media (prefers-color-scheme: dark). Dark palette uses deep navy/indigo tones (#1a1a2e,#1e1e2f,#16213e).3. Dynamic screen resize
Added responsive breakpoints:
768px: viz-row collapses to single column, map shrinks to 280px, card grid to 1-col480px: further padding/font reduction, map to 220px4. Privacy-friendly location prompt
Replaced the auto-requesting
navigator.geolocation.getCurrentPosition()on mount with an explicit opt-in flow:lat, loncoordinatesLink to Devin session: https://app.devin.ai/sessions/f0bde6cb162848ee8b283d4b6426d9cf
Requested by: @LCSOGthb
Summary by Sourcery
Introduce a privacy-friendly location selection flow, responsive layout improvements, and themeable styling with dark mode support for the AirMerge dashboard.
New Features:
Enhancements:
Summary by cubic
Adds dark mode, matches chart/map heights, improves responsive layout, and replaces auto geolocation with an explicit, privacy-friendly location prompt. Also applies automated formatting for consistency with no functional changes.
New Features
prefers-color-scheme.maintainAspectRatio: false.Refactors
Written for commit f267730. Summary will update on new commits.