feat: add high-contrast mode toggle for outdoor visibility#59
Open
onkar0127 wants to merge 3 commits into
Open
feat: add high-contrast mode toggle for outdoor visibility#59onkar0127 wants to merge 3 commits into
onkar0127 wants to merge 3 commits into
Conversation
|
@onkar0127 is attempting to deploy a commit to the karan3431's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
🎉 Thanks for your contribution, @onkar0127! Please make sure CI passes and the checklist in the PR template is complete. A maintainer will review this soon. — The AgroNavis team |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThis PR implements a high-contrast mode (pure black/white, thick borders) for outdoor visibility. The feature adds a reusable state hook with localStorage persistence, global CSS theme overrides, app-level initialization, a user toggle in ProfileTab settings, and annotations across UI components. ChangesHigh-Contrast Mode Implementation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes The PR spans multiple files with moderate logic density: state hook with localStorage synchronization and SSR guards, comprehensive CSS selector scope covering multiple UI patterns (default elements, active/selected states, SVG stroke/fill, nested text), and app-level initialization. Requires careful verification of localStorage/DOM sync timing, CSS selector specificity and coverage across component types, and consistency of markup annotations across the app without breaking existing component layouts or styling. Poem
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (4)
frontend/src/styles/globals.css (2)
196-197: ⚡ Quick winRemove redundant
background-colorafterbackgroundshorthand.Line 196 sets
background-colorwhich is then immediately overridden by thebackgroundshorthand on line 197. Remove the redundant declaration.🧹 Proposed fix
border: 3px solid `#000000` !important; - background-color: `#ffffff` !important; background: `#ffffff` !important; color: `#000000` !important;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/styles/globals.css` around lines 196 - 197, Remove the redundant background-color declaration that is immediately overridden by the background shorthand: delete the `background-color: `#ffffff` !important;` line so only `background: `#ffffff` !important;` remains (this applies to the CSS block containing those two declarations).
146-156: ⚖️ Poor tradeoffConsider removing
!importantfrom CSS custom properties.Using
!importanton custom properties (lines 146-156) prevents normal CSS cascade and makes it harder to override these values if needed. Custom properties naturally cascade through the DOM, so the specificity of[data-theme='high-contrast']should be sufficient.However, if you've tested and found that some components require the
!importantto enforce the theme, this can remain as-is.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/styles/globals.css` around lines 146 - 156, The CSS custom properties (--color-primary, --color-primary-light, --color-surface, --color-bg, --color-border, --color-text-primary, --color-text-secondary, --color-text-muted, --color-border-secondary, --color-border-tertiary) currently have !important which breaks normal cascade and makes overrides difficult; remove the trailing !important from these custom-property declarations in globals.css (the block that defines these variables, e.g., inside the [data-theme='high-contrast'] rule) so the variables can cascade normally, and only reintroduce !important in a specific declaration if a concrete component proves impossible to override without it.frontend/src/components/ProfileTab.tsx (1)
49-57: ⚖️ Poor tradeoffConsider extracting theme toggle logic to a shared hook.
The logic for setting/removing
data-themeattributes (lines 52-56) is duplicated between_app.tsx(lines 19-23) and this component. This duplication makes it harder to maintain consistent behavior.Consider extracting to a shared hook like
useHighContrastMode()that encapsulates the localStorage access, state management, and DOM manipulation.♻️ Example hook implementation
// hooks/useHighContrastMode.ts import { useState, useEffect, useCallback } from 'react'; export function useHighContrastMode() { const [highContrast, setHighContrast] = useState(false); useEffect(() => { try { const saved = localStorage.getItem('high-contrast') === 'true'; setHighContrast(saved); applyTheme(saved); } catch (err) { console.warn('Could not load theme preference:', err); } }, []); const toggleHighContrast = useCallback((enabled: boolean) => { setHighContrast(enabled); try { localStorage.setItem('high-contrast', enabled ? 'true' : 'false'); } catch (err) { console.warn('Could not save theme preference:', err); } applyTheme(enabled); }, []); return { highContrast, toggleHighContrast }; } function applyTheme(enabled: boolean) { if (enabled) { document.documentElement.setAttribute('data-theme', 'high-contrast'); } else { document.documentElement.removeAttribute('data-theme'); } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/ProfileTab.tsx` around lines 49 - 57, Duplicate theme-toggle logic in handleToggleHighContrast (ProfileTab.tsx) and _app.tsx should be extracted into a shared hook; create useHighContrastMode() that encapsulates reading/writing localStorage, maintaining highContrast state, and updating document.documentElement (applyTheme), then refactor ProfileTab's handleToggleHighContrast to call the hook's toggleHighContrast (or directly use returned toggleHighContrast/highContrast) and update _app.tsx to use the same hook instead of duplicating the DOM/localStorage logic so both components share the single source of truth.frontend/src/styles/ProfileTab.module.css (1)
389-390: ⚡ Quick winRemove redundant
background-colorafterbackgroundshorthand.Line 389 sets
background-colorwhich is immediately overridden bybackgroundon line 390. The same pattern appears on lines 399-400.🧹 Proposed fix
:global([data-theme='high-contrast']) .slider { border: 3px solid `#000000` !important; - background-color: `#ffffff` !important; background: `#ffffff` !important; }:global([data-theme='high-contrast']) .switch input:checked + .slider { - background-color: `#000000` !important; background: `#000000` !important; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/styles/ProfileTab.module.css` around lines 389 - 390, Remove the redundant background-color declarations that are immediately overridden by the background shorthand: delete the "background-color: `#ffffff` !important;" lines that appear directly before "background: `#ffffff` !important;" (and the same redundant pair later in the file) so only the background shorthand remains; this removes duplicate rules while preserving the intended styling.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/components/ProfileTab.tsx`:
- Around line 28-31: The useEffect block that calls localStorage.getItem should
be made resilient to storage access errors: wrap the access in a try/catch
inside the existing useEffect (around the call to
localStorage.getItem('high-contrast')), defaulting hc to false (or keeping
existing state) if an error is thrown, and then call setHighContrast(hc) only
with the safe value; ensure you reference the same useEffect and setHighContrast
symbols and do not allow the exception to escape the effect so the component can
mount in environments where localStorage is unavailable or throws.
- Around line 49-57: handleToggleHighContrast currently calls
localStorage.setItem without protection; wrap the
localStorage.setItem('high-contrast', ...) call in a try/catch inside
handleToggleHighContrast, and on error log the error with context (e.g.,
console.error or a logger) so the failure is visible and optionally set a
fallback (like a cookie) or surface a user-facing notice; do not remove the
existing setHighContrast or DOM theme changes—only add error handling around the
persistence call to avoid silent loss of the preference.
In `@frontend/src/pages/_app.tsx`:
- Line 1: Three localStorage accesses lack error handling: wrap the read in
_app.tsx (the useEffect or theme initialization that calls localStorage.getItem)
in a try-catch and fall back to the default theme when an exception occurs; in
ProfileTab.tsx wrap the initial read (the useState initializer or component
mount that calls localStorage.getItem) in try-catch and default the state if
reading fails; and wrap the write in ProfileTab's toggle handler (the function
that calls localStorage.setItem) in try-catch so the state update still proceeds
but persistence failures are swallowed or logged. Ensure each catch does not
rethrow and optionally logs the error via console.warn/error so the app does not
crash when storage is unavailable.
- Around line 17-24: The effect that reads localStorage (the useEffect callback
that calls localStorage.getItem('high-contrast')) lacks error handling; wrap the
localStorage access in a try-catch (and optionally check typeof window !==
'undefined') so any thrown error falls back to treating high-contrast as false,
and only call document.documentElement.setAttribute('data-theme',
'high-contrast') or removeAttribute when the read succeeds or when the fallback
says false; update the useEffect callback to catch errors and safely default
instead of allowing the exception to break rendering.
In `@frontend/src/styles/globals.css`:
- Around line 202-210: The selectors in globals.css use substring matching
([class*="active"], [class*="Active"], [class*="selected"], [class*="Selected"])
which incorrectly matches many unrelated class names; update them to use
whole-token matching ([class~="active"], [class~="Active"], [class~="selected"],
[class~="Selected"]) so only classes that are exactly those tokens are targeted,
leaving the rest of the rule (background-color, color, border-color and
!important) unchanged.
- Line 1: The high-contrast CSS rules use overly broad attribute substring
selectors (e.g. [class*="card"], [class*="tab"], [class*="item"],
[class*="bar"], and state selectors [class*="active"], [class*="selected"])
which match unintended elements; update those rules to target a finite set of
explicit class names (e.g. .card, .card--primary, .tab, .tab--panel, .item,
.bar) or, preferably, switch to a dedicated attribute like data-hc-target="true"
and data-hc-state="active" and replace the selectors with
[data-hc-target="true"] and [data-hc-state="active"] (or explicit
.active/.selected class names) so only intended components and states receive
high-contrast styles; modify the selectors in the existing high-contrast rule
blocks that currently list the [class*="..."] patterns to use the explicit class
list or the data-attribute equivalents and remove the substring selectors.
- Around line 169-199: The broad substring attribute selectors like
[class*="tab"], [class*="item"], [class*="bar"], etc. in the high-contrast block
are matching unintended classes and must be narrowed; replace those
[class*="..."] selectors with either an explicit list of the actual component
classes that should receive high-contrast styling (e.g., .tab, .Tab, .myApp-tab,
etc.) or switch to a dedicated data attribute selector such as
[data-hc-target="true"] and then add data-hc-target="true" to components that
need high-contrast styling (update the CSS rule that currently targets button,
select, input, textarea, aside, header and the class* selectors to use the
explicit classes or the new data attribute).
- Around line 220-247: The high-contrast CSS rules currently force stroke/fill
on every SVG via the selectors [data-theme='high-contrast'] svg, svg * and the
active/selected variants, which clobbers semantic/icon colors; change these
selectors to only target decorative SVGs (e.g. [data-theme='high-contrast']
svg:not([data-semantic]), [data-theme='high-contrast'] svg:not(.semantic) and
their active/selected variants) or add an explicit opt-out (e.g. require
decorative SVGs to have class="hc-decorative" and change selectors to
[data-theme='high-contrast'] svg.hc-decorative, etc.), and remove the global svg
* rules so inline/icon colors (including currentColor) are preserved for
semantic icons; update the active/selected selectors similarly (replace
[class*="active"] svg with [class*="active"] svg.hc-decorative or
[class*="active"] svg:not([data-semantic])) so only decorative icons are
overridden.
---
Nitpick comments:
In `@frontend/src/components/ProfileTab.tsx`:
- Around line 49-57: Duplicate theme-toggle logic in handleToggleHighContrast
(ProfileTab.tsx) and _app.tsx should be extracted into a shared hook; create
useHighContrastMode() that encapsulates reading/writing localStorage,
maintaining highContrast state, and updating document.documentElement
(applyTheme), then refactor ProfileTab's handleToggleHighContrast to call the
hook's toggleHighContrast (or directly use returned
toggleHighContrast/highContrast) and update _app.tsx to use the same hook
instead of duplicating the DOM/localStorage logic so both components share the
single source of truth.
In `@frontend/src/styles/globals.css`:
- Around line 196-197: Remove the redundant background-color declaration that is
immediately overridden by the background shorthand: delete the
`background-color: `#ffffff` !important;` line so only `background: `#ffffff`
!important;` remains (this applies to the CSS block containing those two
declarations).
- Around line 146-156: The CSS custom properties (--color-primary,
--color-primary-light, --color-surface, --color-bg, --color-border,
--color-text-primary, --color-text-secondary, --color-text-muted,
--color-border-secondary, --color-border-tertiary) currently have !important
which breaks normal cascade and makes overrides difficult; remove the trailing
!important from these custom-property declarations in globals.css (the block
that defines these variables, e.g., inside the [data-theme='high-contrast']
rule) so the variables can cascade normally, and only reintroduce !important in
a specific declaration if a concrete component proves impossible to override
without it.
In `@frontend/src/styles/ProfileTab.module.css`:
- Around line 389-390: Remove the redundant background-color declarations that
are immediately overridden by the background shorthand: delete the
"background-color: `#ffffff` !important;" lines that appear directly before
"background: `#ffffff` !important;" (and the same redundant pair later in the
file) so only the background shorthand remains; this removes duplicate rules
while preserving the intended styling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 523c1b05-aace-4117-b506-a0288528d5e4
📒 Files selected for processing (4)
frontend/src/components/ProfileTab.tsxfrontend/src/pages/_app.tsxfrontend/src/styles/ProfileTab.module.cssfrontend/src/styles/globals.css
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/components/ProfileTab.tsx (1)
205-215:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHigh-contrast toggle lacks a reliable accessible name.
At Line 210, the checkbox is associated with a label that contains no text. The visible “High Contrast Mode” text is outside that label, so screen readers may announce an unnamed control.
Suggested fix (associate visible text via
aria-labelledby)-<span className={styles.menuItemLabel}>High Contrast Mode</span> +<span id="high-contrast-label" className={styles.menuItemLabel}>High Contrast Mode</span> <input id="high-contrast-toggle" type="checkbox" checked={highContrast} + aria-labelledby="high-contrast-label" onChange={(e) => toggleHighContrast(e.target.checked)} />🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/ProfileTab.tsx` around lines 205 - 215, The checkbox input with id "high-contrast-toggle" lacks an accessible name because the label element is empty and the visible "High Contrast Mode" text is located outside the label in a sibling span element. To fix this, add a unique id attribute to the span element containing the "High Contrast Mode" text (the one with className={styles.menuItemLabel}), then add an aria-labelledby attribute to the checkbox input that references this id, establishing the proper association between the checkbox control and its visible label text for screen readers.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/hooks/useHighContrastMode.ts`:
- Around line 6-21: The high-contrast mode initialization in the
useHighContrastMode hook runs inside a useEffect, which executes after the
component's first render and causes a visible theme flash. Move the localStorage
reading logic out of useEffect and into a state initializer function passed to
useState so that the high-contrast preference is read synchronously before the
component renders. The initializer function should check if window is defined
and read the localStorage value the same way it currently does in the useEffect,
then remove or simplify the useEffect since the initialization will happen at
the correct time (before paint).
In `@frontend/src/styles/globals.css`:
- Around line 182-186: The active and selected state selectors in the
high-contrast theme are inconsistent across parent and descendant rules, causing
descendants to potentially render with black text on black backgrounds. Review
all active-state selector blocks (at lines 182-186, 192-194, 204-206, 213-215,
and 241-249) and ensure each block includes the same complete set of selectors
covering all variations: the class-based selectors (active, Active, selected,
Selected) AND the data-attribute selectors (data-hc-state="active" and
data-hc-state="selected"). Maintain selector parity across all these locations
so that descendant text color rules are consistently applied to all active and
selected state conditions.
- Around line 146-160: Fix the remaining stylelint violations in the
high-contrast theme color variable block. Address the color-hex-length
violations by adjusting the hex color format to match your stylelint
configuration (either converting six-character hex codes to three-character
format or ensuring consistency across all color definitions). Additionally, fix
the rule-empty-line-before violations by adding proper empty lines before CSS
rules as required by your stylelint configuration. These violations appear in
multiple locations within the theme definition block and must be resolved to
prevent merge blocking.
---
Outside diff comments:
In `@frontend/src/components/ProfileTab.tsx`:
- Around line 205-215: The checkbox input with id "high-contrast-toggle" lacks
an accessible name because the label element is empty and the visible "High
Contrast Mode" text is located outside the label in a sibling span element. To
fix this, add a unique id attribute to the span element containing the "High
Contrast Mode" text (the one with className={styles.menuItemLabel}), then add an
aria-labelledby attribute to the checkbox input that references this id,
establishing the proper association between the checkbox control and its visible
label text for screen readers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 214fe2d6-4b3f-4769-943d-3f791d532218
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (13)
frontend/src/components/CropScanTab.tsxfrontend/src/components/DailyTaskReminders.tsxfrontend/src/components/Dashboard.tsxfrontend/src/components/ProfileTab.tsxfrontend/src/hooks/useHighContrastMode.tsfrontend/src/pages/_app.tsxfrontend/src/pages/auth/login.tsxfrontend/src/pages/onboarding/crops.tsxfrontend/src/pages/onboarding/farm.tsxfrontend/src/pages/onboarding/profile.tsxfrontend/src/pages/weather-forecast.tsxfrontend/src/styles/ProfileTab.module.cssfrontend/src/styles/globals.css
💤 Files with no reviewable changes (1)
- frontend/src/styles/ProfileTab.module.css
✅ Files skipped from review due to trivial changes (8)
- frontend/src/pages/onboarding/profile.tsx
- frontend/src/pages/onboarding/farm.tsx
- frontend/src/components/CropScanTab.tsx
- frontend/src/pages/onboarding/crops.tsx
- frontend/src/pages/auth/login.tsx
- frontend/src/components/DailyTaskReminders.tsx
- frontend/src/components/Dashboard.tsx
- frontend/src/pages/weather-forecast.tsx
Comment on lines
+6
to
+21
| useEffect(() => { | ||
| let hc = false; | ||
| try { | ||
| if (typeof window !== 'undefined') { | ||
| hc = localStorage.getItem('high-contrast') === 'true'; | ||
| } | ||
| } catch (err) { | ||
| console.error('Failed to read high-contrast from localStorage:', err); | ||
| } | ||
| setHighContrast(hc); | ||
| if (hc) { | ||
| document.documentElement.setAttribute('data-theme', 'high-contrast'); | ||
| } else { | ||
| document.documentElement.removeAttribute('data-theme'); | ||
| } | ||
| }, []); |
There was a problem hiding this comment.
High-contrast is restored too late to prevent first-paint flash.
At Line 6, initialization runs in useEffect, so data-theme="high-contrast" is applied after mount. Users with saved preference can briefly see the default theme first.
Suggested fix (pre-paint initialization in the hook)
-import { useState, useEffect } from 'react';
+import { useState, useEffect, useLayoutEffect } from 'react';
export function useHighContrastMode() {
- const [highContrast, setHighContrast] = useState(false);
+ const [highContrast, setHighContrast] = useState<boolean>(() => {
+ if (typeof window === 'undefined') return false;
+ try {
+ return localStorage.getItem('high-contrast') === 'true';
+ } catch {
+ return false;
+ }
+ });
- useEffect(() => {
- let hc = false;
- try {
- if (typeof window !== 'undefined') {
- hc = localStorage.getItem('high-contrast') === 'true';
- }
- } catch (err) {
- console.error('Failed to read high-contrast from localStorage:', err);
- }
- setHighContrast(hc);
- if (hc) {
- document.documentElement.setAttribute('data-theme', 'high-contrast');
- } else {
- document.documentElement.removeAttribute('data-theme');
- }
- }, []);
+ useLayoutEffect(() => {
+ if (highContrast) {
+ document.documentElement.setAttribute('data-theme', 'high-contrast');
+ } else {
+ document.documentElement.removeAttribute('data-theme');
+ }
+ }, [highContrast]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/hooks/useHighContrastMode.ts` around lines 6 - 21, The
high-contrast mode initialization in the useHighContrastMode hook runs inside a
useEffect, which executes after the component's first render and causes a
visible theme flash. Move the localStorage reading logic out of useEffect and
into a state initializer function passed to useState so that the high-contrast
preference is read synchronously before the component renders. The initializer
function should check if window is defined and read the localStorage value the
same way it currently does in the useEffect, then remove or simplify the
useEffect since the initialization will happen at the correct time (before
paint).
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
resolved #13
Title
feat: add High-Contrast Mode toggle for outdoor visibilityDescription
This PR addresses visibility issues for farmers operating the application outdoors in direct sunlight. It adds a global High-Contrast Mode setting that changes the visual theme to pure black/white with bold 3px thick borders, maximizing screen readability under bright sunlight.
Key Changes
localStorage.data-theme="high-contrast"is active.Summary by CodeRabbit