Conversation
* Initial Daisy Ui transition and 4 themes to start * Update Ui * Fix SonarCloud issues * Remove Computed Single line Chart Register * Use JSX syntax * Apply Bunny Fixes * Use card component * Use Card * Fix active and hover states * Format and Style Nav Bar Better * Move profiles over(need to fix spacing between lables) * Fix Mobile Layout * Fix mobile layout * Listen to the bunny * Update style.css * Convert Scale and Autotune * Delete dashboard.html * I like the titles on the left better
* Migrate Settings to DaisyUi and removed unused error handling * Fix eslint error var to const * Fix eslint * Remove Unused container-fluid and create context. Run Format. Migrate Process Info to daisy * Change to aria label * Use Daisy UI tool tips * Remove Unused Styles * Use Semantic Colors * Add Theme Manager to settings Right now this is a separate card but can be changed whenever * Standardize spacing and Padding as well as component usage * Fix Profile pages and use consistent styling * Improve Dark style ? * add aria * Fix Save * Consistent Styles. Use 10/5 Grid Layout * Fix Button Spacing * Prevent race condition * Fix button Position and use 5/10 layout on home page * fix card class again * javascript:void(0) * simple bp * add theme urls * add safelist * fix profile cards * FMT and When saving profile rout back to profile list * Clarify Settings Card * Fix Chart Height * This needs a bit of testing but should fix card duplication * Add Switching the layout of the home page * Format and Fix Lint errors * Fix some code reviews * Update index.jsx * Split Settings Page up a bit * Clean up Inputs * Fix Radix --------- Co-authored-by: Jochen Niebuhr <kontakt@ju-hh.de>
WalkthroughReplaces controller->deactivate() with controller->clear() on WebSocket req:change-mode; introduces DaisyUI and theme/dashboard managers; large UI refactors to Card-based layouts across many web pages; removes legacy dashboard.html; initializes theme before render and persists dashboard layout. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Web UI
participant Server as WebUIPlugin
participant Controller
Client->>Server: WS tp: "req:change-mode", mode
Server->>Controller: clear()
Server->>Controller: setMode(mode)
Controller-->>Server: ack
Server-->>Client: res:change-mode (ok)
sequenceDiagram
participant Browser
participant ThemeMgr as ThemeManager
participant DOM
Browser->>ThemeMgr: initializeTheme()
ThemeMgr->>ThemeMgr: getStoredTheme()
ThemeMgr->>DOM: document.documentElement[data-theme]=theme
sequenceDiagram
participant Page as Home Page
participant DashMgr as DashboardManager
participant Storage as localStorage
Page->>DashMgr: getDashboardLayout()
DashMgr->>Storage: read "dashboardLayout"
Storage-->>DashMgr: value
DashMgr-->>Page: layout or default
Page->>DashMgr: setDashboardLayout(layout)
DashMgr->>Storage: write "dashboardLayout"
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45–60 minutes Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 24
🔭 Outside diff range comments (11)
src/display/plugins/WebUIPlugin.cpp (1)
159-161: Fix format string in ESP_LOGI (invalid placeholder).The format string uses "%" without a specifier; this is undefined/incorrect and may garble logs.
Apply:
- ESP_LOGI("WebUIPlugin", "Received request: %", (char *)data); + ESP_LOGI("WebUIPlugin", "Received request: %s", (char *)data);web/src/pages/ShotHistory/HistoryChart.jsx (1)
114-117: Destroy Chart instance on unmount to avoid leaks.The Chart is created but never destroyed. Add a cleanup to prevent memory leaks and canvas context warnings when navigating.
Example:
useEffect(() => { const ct = new Chart(ref.current, chartData); setChart(ct); return () => { try { ct.destroy(); } catch {} }; }, [ref]);web/src/services/ApiService.js (2)
122-141: Clear timeout on resolve to avoid dangling timersIf the response arrives before the 30s timeout, the timer still fires later. Clear it upon resolve to prevent unnecessary work and potential confusion in debugging.
- return new Promise((resolve, reject) => { + return new Promise((resolve, reject) => { // Create a listener for the response with matching rid - const listenerId = this.on(returnType, response => { + let timeoutId; + const listenerId = this.on(returnType, response => { if (response.rid === rid) { // Clean up the listener this.off(returnType, listenerId); + if (timeoutId) clearTimeout(timeoutId); resolve(response); } }); // Send the request this.send(message); // Optional: Add timeout - setTimeout(() => { + timeoutId = setTimeout(() => { this.off(returnType, listenerId); reject(new Error(`Request ${data.tp} timed out`)); }, 30000); // 30 second timeout });
119-121: Validate request type and avoid brittle substring usageGuard against invalid tp values and prefer slice for clarity.
- const returnType = `res:${data.tp.substring(4)}`; + if (!data?.tp?.startsWith('req:')) { + throw new Error(`request(): expected data.tp to start with "req:". Got "${data?.tp}"`); + } + const returnType = `res:${data.tp.slice(4)}`;web/src/pages/ProfileList/ExtendedContent.jsx (1)
104-107: Destroy Chart instance on unmount to prevent memory leaksChart.js instances should be cleaned up; otherwise re-mounts or route changes can leak canvases and handlers.
useEffect(() => { const ct = new Chart(ref.current, config); + return () => ct.destroy(); }, [ref]);web/src/pages/ProfileEdit/StandardProfileForm.jsx (2)
173-190: Potential data loss when resetting volumetric targetWhen
onVolumetricTargetChangereceives a value of 0, it setstargetstonull, which removes all targets including non-volumetric ones that might exist. This could lead to unintended data loss.const onVolumetricTargetChange = value => { if (value === 0) { + // Remove only the volumetric target, preserve other targets + const nonVolumetricTargets = (phase.targets || []).filter(t => t.type !== 'volumetric'); onChange({ ...phase, - targets: null, + targets: nonVolumetricTargets.length > 0 ? nonVolumetricTargets : null, }); return; } + // Add or update volumetric target while preserving other targets + const nonVolumetricTargets = (phase.targets || []).filter(t => t.type !== 'volumetric'); onChange({ ...phase, - targets: [ + targets: [ + ...nonVolumetricTargets, { type: 'volumetric', value: value, }, ], }); };
196-199: Add defensive checks for non-numeric pump valuesThe code assumes
phase.pumpis either a number or an object withpressureandflowproperties, but doesn't handle potential undefined or malformed values safely.const pumpPower = isNumber(phase.pump) ? phase.pump : 100; - const pressure = !isNumber(phase.pump) ? phase.pump.pressure : 0; - const flow = !isNumber(phase.pump) ? phase.pump.flow : 0; + const pressure = !isNumber(phase.pump) && phase.pump ? (phase.pump.pressure || 0) : 0; + const flow = !isNumber(phase.pump) && phase.pump ? (phase.pump.flow || 0) : 0; const mode = isNumber(phase.pump) ? (phase.pump === 0 ? 'off' : 'power') : phase.pump.target;web/src/pages/ProfileList/index.jsx (1)
308-321: Add error handling for file uploadThe file upload handler lacks error handling for JSON parsing and API requests, which could leave the UI in an inconsistent state.
const onUpload = function (evt) { if (evt.target.files.length) { const file = evt.target.files[0]; const reader = new FileReader(); reader.onload = async e => { - const result = e.target.result; - if (typeof result === 'string') { - let profiles = JSON.parse(result); - if (!Array.isArray(profiles)) { - profiles = [profiles]; - } - for (const p of profiles) { - await apiService.request({ tp: 'req:profiles:save', profile: p }); - } - await loadProfiles(); - } + try { + const result = e.target.result; + if (typeof result === 'string') { + let profiles = JSON.parse(result); + if (!Array.isArray(profiles)) { + profiles = [profiles]; + } + setLoading(true); + for (const p of profiles) { + await apiService.request({ tp: 'req:profiles:save', profile: p }); + } + await loadProfiles(); + } + } catch (error) { + console.error('Failed to import profiles:', error); + setLoading(false); + // Consider adding user-facing error notification here + } }; + reader.onerror = () => { + console.error('Failed to read file'); + // Consider adding user-facing error notification here + }; reader.readAsText(file); } };web/src/pages/Settings/index.jsx (3)
191-505: Critical: Remove duplicate form sections causing JSX parsing errorsThere are two complete duplicate sections of the settings form (lines 191-505 and 506-1093), which is causing the pipeline build failures. The second section starting at line 506 contains the actual form element and should be kept, while the first section should be removed.
Remove the entire duplicate section from lines 191-505. This duplicate content is causing JSX parsing errors as reported in the pipeline failures.
- </div> - <Card xs={12} lg={6} title="Temperature settings"> - <div> - <label htmlFor="targetSteamTemp" className="block font-medium text-gray-700 dark:text-gray-400"> - Default Steam Temperature (°C) - </label> - ... [lines 192-505 should be removed] - </div> - <form key='settings' ref={formRef} method='post' action='/api/settings' onSubmit={onSubmit}>Also applies to: 506-1093
50-98: Simplify onChange handler with a more maintainable approachThe
onChangefunction has repetitive boolean toggle logic that could be simplified using a Set or array of toggle fields.const onChange = key => { return e => { let value = e.currentTarget.value; - if (key === 'homekit') { - value = !formData.homekit; - } - if (key === 'boilerFillActive') { - value = !formData.boilerFillActive; - } - if (key === 'smartGrindActive') { - value = !formData.smartGrindActive; - } - if (key === 'smartGrindToggle') { - value = !formData.smartGrindToggle; - } - if (key === 'homeAssistant') { - value = !formData.homeAssistant; - } - if (key === 'momentaryButtons') { - value = !formData.momentaryButtons; - } - if (key === 'delayAdjust') { - value = !formData.delayAdjust; - } - if (key === 'clock24hFormat') { - value = !formData.clock24hFormat; - } + const toggleFields = [ + 'homekit', 'boilerFillActive', 'smartGrindActive', 'smartGrindToggle', + 'homeAssistant', 'momentaryButtons', 'delayAdjust', 'clock24hFormat' + ]; + + if (toggleFields.includes(key)) { + value = !formData[key]; + } + if (key === 'standbyDisplayEnabled') { value = !formData.standbyDisplayEnabled; // Set standby brightness to 0 when toggle is off const newFormData = { ...formData, [key]: value, }; if (!value) { newFormData.standbyBrightness = 0; } setFormData(newFormData); return; } if (key === 'dashboardLayout') { setDashboardLayout(value); } setFormData({ ...formData, [key]: value, }); }; };
144-154: Add error handling for file uploadSimilar to ProfileList, the settings upload handler lacks error handling for JSON parsing.
const onUpload = function (evt) { if (evt.target.files.length) { const file = evt.target.files[0]; const reader = new FileReader(); reader.onload = async e => { - const data = JSON.parse(e.target.result); - setFormData(data); + try { + const data = JSON.parse(e.target.result); + setFormData(data); + } catch (error) { + console.error('Failed to parse settings file:', error); + // Consider adding user-facing error notification + } }; + reader.onerror = () => { + console.error('Failed to read settings file'); + // Consider adding user-facing error notification + }; reader.readAsText(file); } };
🧹 Nitpick comments (32)
web/src/components/Navigation.jsx (4)
5-5: Prefer DaisyUI "btn-ghost" for base state; drop manual hover/text classesThis simplifies styling, aligns with DaisyUI conventions, and avoids hard-coding theme tokens that DaisyUI already manages.
Apply this diff:
- 'btn btn-md justify-start gap-3 w-full text-base-content hover:text-base-content hover:bg-base-content/10 bg-transparent border-none px-2'; + 'btn btn-ghost btn-md justify-start gap-3 w-full px-2';
21-21: Nit: Add an accessible label to navProviding an aria-label makes the nav landmark discoverable to assistive tech.
Example:
- <nav className='hidden lg:col-span-2 lg:block'> + <nav className='hidden lg:col-span-2 lg:block' aria-label='Primary'>
23-23: Use DaisyUI "divider" instead of
for consistent themingThis aligns with the DaisyUI migration and removes reliance on height hacks.
Apply this diff:
- <hr className='h-5 border-0' /> + <div className='divider my-1'></div>(Repeat for each separator)
Also applies to: 28-28, 34-34
22-37: Reduce duplication by mapping an items config to MenuItemThis makes the menu easier to maintain and reorder, and centralizes icon/link definitions.
Example refactor (outside selected lines):
const sections = [ { items: [ { label: 'Dashboard', link: '/', icon: 'fa-solid fa-house' }, ], }, { items: [ { label: 'Profiles', link: '/profiles', icon: 'fa-solid fa-list' }, { label: 'Shot History', link: '/history', icon: 'fa-solid fa-clock-rotate-left' }, ], }, { items: [ { label: 'PID Autotune', link: '/pidtune', icon: 'fa-solid fa-temperature-half' }, { label: 'Bluetooth Scales', link: '/scales', icon: 'fa-brands fa-bluetooth-b' }, { label: 'Settings', link: '/settings', icon: 'fa-solid fa-gear' }, ], }, { items: [ { label: 'System & Updates', link: '/ota', icon: 'fa-solid fa-arrows-rotate' }, ], }, ]; export function Navigation() { return ( <nav className="hidden lg:col-span-2 lg:block" aria-label="Primary"> {sections.map((section, idx) => ( <div key={idx} className="space-y-1.5"> {section.items.map(item => ( <MenuItem key={item.link} label={item.label} link={item.link} iconClass={item.icon} /> ))} <div className="divider my-1"></div> </div> ))} </nav> ); }web/src/components/Spinner.jsx (1)
10-14: Expose className/label for theming and i18n (optional)Allow callers to pass text color (e.g., text-primary) and localized labels without changing internals.
Here’s a minimal enhancement (outside the selected range) for consideration:
export function Spinner({ size = 8, label = 'Loading', className = '', ...rest }) { const sizeMap = { 4: 'loading-xs', 8: 'loading-md', 12: 'loading-lg', 16: 'loading-lg' }; const sizeClass = sizeMap[size] || 'loading-md'; return ( <span className={`loading loading-spinner ${sizeClass} ${className}`} role="status" aria-label={label} style={!sizeMap[size] && Number.isFinite(size) ? { width: size, height: size } : undefined} {...rest} /> ); }If you’d like, I can update call sites to pass a localized label and optional className.
web/package.json (1)
29-31: Ensure Prettier loads the Tailwind plugin in all environments.prettier-plugin-tailwindcss works with Prettier 3.x, but discovery can fail in certain setups (monorepos, non-standard config resolution). If not already in your .prettierrc, explicitly declare the plugin to avoid class-ordering silently not applying.
Optionally add to .prettierrc:
{ "plugins": ["prettier-plugin-tailwindcss"] }web/index.html (1)
26-26: Prevent theme flash on first paint by seeding data-theme before app boot.bg-base-100 relies on DaisyUI theme tokens. Until initializeTheme runs, users may briefly see the wrong theme. Seed data-theme early in so CSS variables are correct on first paint.
Add a tiny inline script in :
<script> try { const stored = localStorage.getItem('theme'); const prefersDark = window.matchMedia && window.matchMedia('(prefers-color-scheme: dark)').matches; document.documentElement.setAttribute('data-theme', stored || (prefersDark ? 'dark' : 'light')); } catch (e) { // no-op } </script>web/src/services/ApiService.js (2)
82-85: Add jitter to exponential backoff to prevent synchronized reconnect stormsFormatting looks good. Add jitter so multiple clients don’t reconnect simultaneously after outages.
- const delay = Math.min( - this.baseReconnectDelay * Math.pow(2, this.reconnectAttempts), - this.maxReconnectDelay, - ); + const base = this.baseReconnectDelay * (2 ** this.reconnectAttempts); + const jitter = Math.random() * base * 0.3; // 30% jitter + const delay = Math.min(base + jitter, this.maxReconnectDelay);
5-10: Use slice instead of deprecated substr in randomIdsubstr is legacy and discouraged. slice is the modern, well-supported alternative.
return Math.random() .toString(36) .replace(/[^a-z]+/g, '') - .substr(2, 10); + .slice(2, 12);web/src/pages/ProfileList/ExtendedContent.jsx (1)
63-63: Remove unused variableThe local variable ‘labels’ is unused.
- const labels = [0];web/src/pages/ProfileList/ProfileAddCard.jsx (1)
6-13: Make the whole card body clickable for better UXExpand the anchor to fill the card body so the entire card area is a hit target.
<a href='/profiles/new' - className='text-base-content hover:text-primary flex cursor-pointer flex-col items-center justify-center gap-2 p-4 transition-colors' + className='text-base-content hover:text-primary flex cursor-pointer flex-col items-center justify-center gap-2 p-4 transition-colors block h-full w-full' >web/src/components/ProcessInfo.jsx (1)
35-36: Optional: add accessible labelling to progressScreen readers generally handle , but adding an aria-label and valuetext can improve UX.
- <progress className='progress progress-primary w-full' value={progress} max='100' /> + <progress + className='progress progress-primary w-full' + value={progress} + max='100' + aria-label={`Process progress ${progress}%`} + />web/src/utils/uuid.js (1)
2-4: Prefer native crypto.randomUUID with a safe fallbackModern browsers expose crypto.randomUUID(), which is simpler and more robust. Fallback to your current implementation (and finally to Math.random) when unavailable.
Apply this diff:
export default function uuidv4() { - return '10000000-1000-4000-8000-100000000000'.replace(/[018]/g, c => - (+c ^ (crypto.getRandomValues(new Uint8Array(1))[0] & (15 >> (+c / 4)))).toString(16), - ); + const cryptoObj = globalThis.crypto || (typeof window !== 'undefined' && window.crypto); + if (cryptoObj?.randomUUID) return cryptoObj.randomUUID(); + if (cryptoObj?.getRandomValues) { + // RFC4122 v4-like fallback + return '10000000-1000-4000-8000-100000000000'.replace( + /[018]/g, + c => (+c ^ (cryptoObj.getRandomValues(new Uint8Array(1))[0] & (15 >> (+c / 4)))).toString(16), + ); + } + // Least-preferred non-crypto fallback + return ([1e7]+-1e3+-4e3+-8e3+-1e11) + .replace(/[018]/g, c => (c ^ (Math.random() * 16 >> (c / 4))).toString(16)); }web/src/pages/_404.jsx (1)
and SPA routing to improve accessibility and UX
3-8: Use semantic
- Wrap the content in a semantic main landmark.
- If you use a client-side router elsewhere, prefer its Link component to avoid a full page reload.
Apply this diff for semantics; and consider swapping with your router’s Link if applicable:
- <div className='flex min-h-screen flex-col items-center justify-center'> + <main className='flex min-h-screen flex-col items-center justify-center' role='main' aria-labelledby='page-title'> - <h1 className='text-base-content mb-4 text-6xl font-bold'>404</h1> + <h1 id='page-title' className='text-base-content mb-4 text-6xl font-bold'>404</h1> <p className='text-base-content/70 mb-8 text-xl'>Page not found</p> - <a href='/' className='btn btn-primary'> + <a href='/' className='btn btn-primary'> Go Home </a> - </div> + </main>If you’re using a router (e.g., react-router, preact-router), confirm and I can provide a precise Link replacement.
web/src/style.css (1)
7-11: Silence Biome “unknown property” for DaisyUI theme metadata keysProperties like name, default, and prefersdark inside @plugin "daisyui/theme" blocks are DaisyUI-specific, not standard CSS, hence Biome flags them. Either ignore the rule for this file or just for these blocks.
Apply this diff to ignore the rule file-wide (simplest and future-proof), keeping other linters unaffected:
+/* biome-ignore-file lint/correctness/noUnknownProperty: DaisyUI theme plugin uses non-standard keys (name, default, prefersdark, color-scheme) */ @import url('https://fonts.googleapis.com/css2?family=Montserrat:wght@400;700&display=swap'); @import 'tailwindcss'; @plugin 'daisyui';Alternatively, add targeted ignores before each block:
-@plugin "daisyui/theme" { +/* biome-ignore lint/correctness/noUnknownProperty: DaisyUI theme metadata */ +@plugin "daisyui/theme" { name: 'light'; default: true; prefersdark: false; color-scheme: 'light';Repeat for the dark, coffee, and nord blocks.
Also applies to: 43-47, 79-83, 115-119
web/src/components/Card.jsx (1)
13-26: Minor: extract getGridClasses outside component or memoize for stabilityNot critical, but if Card re-renders frequently, consider memoizing grid class computation or extracting a pure helper to avoid recreating the function each render.
Option:
-export default function Card({ +function getGridClassesFrom({ xs, sm, md, lg, xl }) { + const breakpoints = [ + { value: xs, prefix: '' }, + { value: sm, prefix: 'sm:' }, + { value: md, prefix: 'md:' }, + { value: lg, prefix: 'lg:' }, + { value: xl, prefix: 'xl:' }, + ]; + return breakpoints + .filter(bp => bp.value && bp.value >= 1 && bp.value <= 12) + .map(bp => `${bp.prefix}col-span-${bp.value}`) + .join(' '); +} + +export default function Card({ xs, sm, md, lg, xl, @@ -}) { - const getGridClasses = () => { - const breakpoints = [ - { value: xs, prefix: '' }, - { value: sm, prefix: 'sm:' }, - { value: md, prefix: 'md:' }, - { value: lg, prefix: 'lg:' }, - { value: xl, prefix: 'xl:' }, - ]; - - return breakpoints - .filter(bp => bp.value && bp.value >= 1 && bp.value <= 12) - .map(bp => `${bp.prefix}col-span-${bp.value}`) - .join(' '); - }; +}) { @@ - const gridClasses = getGridClasses(); + const gridClasses = getGridClassesFrom({ xs, sm, md, lg, xl });web/src/pages/Settings/PluginCard.jsx (3)
108-109: Incorrect placeholder for IP address fieldThe placeholder "0" is not appropriate for an IP address field. Consider using a valid IP format.
className='input input-bordered w-full' -placeholder='0' +placeholder='192.168.1.100' value={formData.smartGrindIp}
167-169: Incorrect placeholder for IP address fieldThe placeholder "0" is not appropriate for an IP address field.
className='input input-bordered w-full' -placeholder='0' +placeholder='192.168.1.100' value={formData.haIP}
182-183: Incorrect placeholder for port fieldThe placeholder "0" might mislead users. Consider using a common MQTT port.
className='input input-bordered w-full' -placeholder='0' +placeholder='1883' value={formData.haPort}web/src/components/OverviewChart.jsx (1)
115-119: Consider memoizing the tick callback functionThe callback function creates new Date instances on every tick render. Consider memoizing or optimizing for better performance.
You could extract this to a utility function or memoize it to avoid repeated Date instantiations:
const createTickCallback = () => { return (value, index, ticks) => { const now = Date.now(); const diff = Math.ceil((now - value) / 1000); return `-${diff}s`; }; };web/src/utils/themeManager.js (1)
14-26: Consider separating validation from side effectsThe
setStoredThemefunction both validates and applies the theme. Consider separating these concerns for better testability.Consider splitting the validation and application:
export function setStoredTheme(theme) { try { if (AVAILABLE_THEMES.includes(theme)) { localStorage.setItem(THEME_STORAGE_KEY, theme); - applyTheme(theme); return true; } return false; } catch (error) { console.warn('Failed to set stored theme:', error); return false; } } +export function setAndApplyTheme(theme) { + const success = setStoredTheme(theme); + if (success) { + applyTheme(theme); + } + return success; +}Then update
handleThemeChangeto usesetAndApplyTheme.web/src/pages/ShotHistory/HistoryCard.jsx (1)
50-55: Render volume when it’s 0 (falsy) by checking for null/undefined insteadIf a shot’s volume is 0g, it won’t render. Prefer a null/undefined check.
Apply this diff:
- {shot.volume && ( + {shot.volume != null && ( <div className='flex flex-row items-center gap-2'> <span className='fa fa-scale-balanced'></span> {shot.volume}g </div> )}web/src/pages/Home/index.jsx (1)
65-76: Nit: prefer boolean prop shorthandMinor readability tweak.
Apply this diff:
- <Card sm={10} lg={6} title='Temperature & Pressure Chart' fullHeight={true}> + <Card sm={10} lg={6} title='Temperature & Pressure Chart' fullHeight> <OverviewChart /> </Card>Repeat for the second chart card below.
Also applies to: 71-73
web/src/pages/ProfileEdit/ProfileTypeSelection.jsx (1)
22-24: Grammar nit: tighten phrasing“Supports creating of profiles...” reads awkwardly.
Apply this diff:
- <span className='text-base-content/70 text-center text-sm'> - Supports creating of profiles with different brew phases and targets. - </span> + <span className='text-base-content/70 text-center text-sm'> + Supports creating profiles with different brew phases and targets. + </span>web/src/pages/Autotune/index.jsx (1)
131-149: UX: guard against accidental double starts (minor)You already hide the Start button when active, but adding a defensive early-return in
onStartimproves robustness.For example:
const onStart = useCallback(() => { if (active) return; apiService.send({ tp: 'req:autotune-start', time, samples }); setActive(true); }, [active, time, samples, apiService]);web/src/pages/ShotHistory/index.jsx (1)
72-72: Unnecessary wrapper function for onDelete propThe arrow function wrapper
id => onDelete(id)is redundant sinceonDeletealready has the same signature.- <HistoryCard shot={item} key={idx} onDelete={id => onDelete(id)} /> + <HistoryCard shot={item} key={idx} onDelete={onDelete} />web/src/pages/Scales/index.jsx (1)
91-95: Consider using the Spinner component for consistencyThe code uses
loading loading-spinner loading-lgdirectly instead of theSpinnercomponent that's available and used elsewhere in the codebase.+import { Spinner } from '../../components/Spinner.jsx';{isLoading || isInfoLoading ? ( <div className='flex items-center justify-center py-12'> - <span className='loading loading-spinner loading-lg' /> + <Spinner size={12} /> <span className='text-base-content/70 ml-3'>Loading scales...</span> </div>web/src/pages/Home/ProcessControls.jsx (2)
225-240: Accessibility: Icon-only buttons need proper aria-labelsThe Time/Weight toggle buttons display icons with text, but the icon elements should have proper ARIA attributes for screen readers.
<button className={`flex-1 rounded-full px-3 py-2 text-sm transition-all duration-200 ${brewTarget === 0 ? 'bg-primary text-primary-content font-medium' : 'text-base-content/60 hover:text-base-content'}`} onClick={() => changeTarget(0)} + aria-label="Select time-based brewing" > <i className='fa-solid fa-clock' /> <span className='ml-1'>Time</span> </button> <button className={`flex-1 rounded-full px-3 py-2 text-sm transition-all duration-200 ${brewTarget === 1 ? 'bg-primary text-primary-content font-medium' : 'text-base-content/60 hover:text-base-content'}`} onClick={() => changeTarget(1)} + aria-label="Select weight-based brewing" > <i className='fa-solid fa-weight-scale' /> <span className='ml-1'>Weight</span> </button>
243-244: Icon-only control button needs aria-labelThe main control button only has an icon without accessible text for screen readers.
- <button className='btn btn-circle btn-lg btn-primary' onClick={handleButtonClick}> + <button + className='btn btn-circle btn-lg btn-primary' + onClick={handleButtonClick} + aria-label={active ? 'Pause' : finished ? 'Clear' : 'Start'} + > <i className={`text-2xl ${getButtonIcon()}`} /> </button>web/src/pages/ProfileEdit/StandardProfileForm.jsx (1)
271-272: Extract reusable label componentsThe Volumetric Target and Pressure/Flow labels contain conditional text that could be extracted into a more readable format.
Consider extracting the conditional label logic:
const getFieldLabel = (mode, fieldType) => { if (fieldType === 'pressure') { return mode === 'pressure' ? 'Pressure Target' : 'Pressure Limit'; } if (fieldType === 'flow') { return mode === 'flow' ? 'Flow Target' : 'Flow Limit'; } return ''; };Then use it in the labels:
- Pressure {mode === 'pressure' ? 'Target' : 'Limit'} + {getFieldLabel(mode, 'pressure')}Also applies to: 398-400, 421-422
web/src/pages/ProfileList/index.jsx (2)
213-213: Simplify favoriteCount calculationThe current implementation uses
mapfollowed byreduce, which is less efficient than a singlereduceoperation.- const favoriteCount = profiles.map(p => (p.favorite ? 1 : 0)).reduce((a, b) => a + b, 0); + const favoriteCount = profiles.reduce((count, p) => count + (p.favorite ? 1 : 0), 0);
286-294: Improve data handling for export functionalityThe export function creates a new object for each profile just to delete properties. Consider using destructuring for cleaner code.
const onExport = useCallback(() => { - const exportedProfiles = profiles.map(p => { - const ep = { - ...p, - }; - delete ep.id; - delete ep.selected; - delete ep.favorite; - return ep; - }); + const exportedProfiles = profiles.map(({ id, selected, favorite, ...profile }) => profile);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
web/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (36)
src/display/plugins/WebUIPlugin.cpp(1 hunks)web/.prettierrc(1 hunks)web/dashboard.html(0 hunks)web/index.html(1 hunks)web/package.json(2 hunks)web/src/components/Card.jsx(1 hunks)web/src/components/Footer.jsx(1 hunks)web/src/components/Header.jsx(1 hunks)web/src/components/Navigation.jsx(1 hunks)web/src/components/OverviewChart.jsx(6 hunks)web/src/components/ProcessInfo.jsx(1 hunks)web/src/components/Spinner.jsx(1 hunks)web/src/index.jsx(3 hunks)web/src/pages/Autotune/index.jsx(3 hunks)web/src/pages/Home/ProcessControls.jsx(2 hunks)web/src/pages/Home/index.jsx(1 hunks)web/src/pages/OTA/index.jsx(6 hunks)web/src/pages/ProfileEdit/ProfileTypeSelection.jsx(1 hunks)web/src/pages/ProfileEdit/StandardProfileForm.jsx(5 hunks)web/src/pages/ProfileEdit/index.jsx(1 hunks)web/src/pages/ProfileList/ExtendedContent.jsx(4 hunks)web/src/pages/ProfileList/ProfileAddCard.jsx(1 hunks)web/src/pages/ProfileList/index.jsx(8 hunks)web/src/pages/Scales/index.jsx(2 hunks)web/src/pages/Settings/PluginCard.jsx(1 hunks)web/src/pages/Settings/index.jsx(10 hunks)web/src/pages/ShotHistory/HistoryCard.jsx(1 hunks)web/src/pages/ShotHistory/HistoryChart.jsx(3 hunks)web/src/pages/ShotHistory/index.jsx(1 hunks)web/src/pages/ShotHistory/utils.js(1 hunks)web/src/pages/_404.jsx(1 hunks)web/src/services/ApiService.js(2 hunks)web/src/style.css(1 hunks)web/src/utils/dashboardManager.js(1 hunks)web/src/utils/themeManager.js(1 hunks)web/src/utils/uuid.js(1 hunks)
🔥 Files not summarized due to errors (1)
- web/src/components/Navigation.jsx: Error: Server error: no LLM provider could handle the message
💤 Files with no reviewable changes (1)
- web/dashboard.html
🧰 Additional context used
🧬 Code Graph Analysis (20)
web/src/pages/ShotHistory/HistoryChart.jsx (2)
web/src/components/OverviewChart.jsx (1)
ref(133-133)web/src/pages/ProfileList/ExtendedContent.jsx (1)
ref(101-101)
web/src/pages/Settings/PluginCard.jsx (2)
web/src/pages/OTA/index.jsx (1)
formData(10-10)web/src/pages/Settings/index.jsx (2)
formData(17-17)onChange(50-98)
web/src/pages/ProfileList/ProfileAddCard.jsx (1)
web/src/components/Card.jsx (1)
Card(1-45)
web/src/pages/ShotHistory/HistoryCard.jsx (4)
web/src/components/Card.jsx (1)
Card(1-45)web/src/pages/ProfileList/index.jsx (2)
onExport(285-302)onDelete(232-239)web/src/pages/Settings/index.jsx (1)
onExport(134-142)web/src/pages/ShotHistory/index.jsx (1)
onDelete(47-54)
web/src/pages/ProfileEdit/ProfileTypeSelection.jsx (2)
web/src/components/Card.jsx (1)
Card(1-45)web/src/pages/ProfileList/index.jsx (1)
onSelect(241-248)
web/src/components/Card.jsx (2)
web/src/components/Navigation.jsx (1)
className(4-5)web/src/components/Header.jsx (1)
className(6-7)
web/src/index.jsx (1)
web/src/utils/themeManager.js (1)
initializeTheme(42-45)
web/src/pages/Scales/index.jsx (1)
web/src/components/Card.jsx (1)
Card(1-45)
web/src/pages/ProfileEdit/index.jsx (3)
web/src/components/Spinner.jsx (1)
Spinner(1-16)web/src/pages/ProfileEdit/ProfileTypeSelection.jsx (1)
ProfileTypeSelection(3-46)web/src/pages/ProfileEdit/StandardProfileForm.jsx (1)
StandardProfileForm(5-163)
web/src/components/OverviewChart.jsx (2)
web/src/pages/ShotHistory/HistoryChart.jsx (3)
ref(112-112)chartData(113-113)chart(111-111)web/src/services/ApiService.js (2)
machine(193-207)machine(193-207)
web/src/components/ProcessInfo.jsx (2)
web/src/pages/OTA/index.jsx (1)
progress(12-12)web/src/pages/Home/ProcessControls.jsx (1)
progress(19-19)
web/src/pages/Autotune/index.jsx (12)
web/src/index.jsx (1)
apiService(23-23)web/src/pages/OTA/index.jsx (1)
apiService(7-7)web/src/pages/Home/index.jsx (1)
apiService(23-23)web/src/pages/Home/ProcessControls.jsx (3)
apiService(72-72)active(18-18)active(70-70)web/src/pages/ProfileEdit/index.jsx (1)
apiService(13-13)web/src/pages/ProfileList/index.jsx (1)
apiService(210-210)web/src/pages/ShotHistory/index.jsx (1)
apiService(32-32)web/src/services/ApiService.js (2)
ApiServiceContext(191-191)ApiServiceContext(191-191)web/src/pages/ProfileList/ExtendedContent.jsx (2)
time(64-64)time(70-70)web/src/components/Card.jsx (1)
Card(1-45)web/src/components/OverviewChart.jsx (1)
OverviewChart(131-203)web/src/components/Spinner.jsx (1)
Spinner(1-16)
web/src/pages/ShotHistory/index.jsx (4)
web/src/pages/ProfileList/index.jsx (3)
onDelete(232-239)apiService(210-210)loading(212-212)web/src/pages/Home/index.jsx (1)
apiService(23-23)web/src/components/Spinner.jsx (1)
Spinner(1-16)web/src/pages/ShotHistory/HistoryCard.jsx (1)
HistoryCard(5-62)
web/src/pages/Home/index.jsx (6)
web/src/utils/dashboardManager.js (4)
setDashboardLayout(16-37)setDashboardLayout(16-37)getDashboardLayout(3-14)getDashboardLayout(3-14)web/src/index.jsx (1)
apiService(23-23)web/src/pages/Home/ProcessControls.jsx (2)
apiService(72-72)ProcessControls(65-250)web/src/services/ApiService.js (4)
ApiServiceContext(191-191)ApiServiceContext(191-191)machine(193-207)machine(193-207)web/src/components/Card.jsx (1)
Card(1-45)web/src/components/OverviewChart.jsx (1)
OverviewChart(131-203)
web/src/pages/Home/ProcessControls.jsx (4)
web/src/components/ProcessInfo.jsx (3)
processInfo(4-4)progress(23-26)target(20-21)web/src/index.jsx (1)
apiService(23-23)web/src/pages/Home/index.jsx (3)
apiService(23-23)mode(50-50)changeMode(41-49)web/src/services/ApiService.js (2)
ApiServiceContext(191-191)ApiServiceContext(191-191)
web/src/pages/Settings/index.jsx (7)
web/src/services/ApiService.js (2)
machine(193-207)machine(193-207)web/src/utils/themeManager.js (2)
getStoredTheme(4-12)handleThemeChange(48-51)web/src/utils/dashboardManager.js (2)
setDashboardLayout(16-37)setDashboardLayout(16-37)web/src/components/Spinner.jsx (1)
Spinner(1-16)web/src/components/Card.jsx (1)
Card(1-45)web/src/config/zones.js (2)
timezones(1-463)timezones(1-463)web/src/pages/Settings/PluginCard.jsx (1)
PluginCard(3-222)
web/src/pages/OTA/index.jsx (10)
web/src/index.jsx (1)
apiService(23-23)web/src/pages/Autotune/index.jsx (1)
apiService(8-8)web/src/pages/Home/index.jsx (1)
apiService(23-23)web/src/pages/Home/ProcessControls.jsx (1)
apiService(72-72)web/src/pages/ProfileEdit/index.jsx (1)
apiService(13-13)web/src/pages/ProfileList/index.jsx (1)
apiService(210-210)web/src/pages/ShotHistory/index.jsx (1)
apiService(32-32)web/src/pages/Settings/index.jsx (1)
submitting(15-15)web/src/components/Spinner.jsx (1)
Spinner(1-16)web/src/components/Card.jsx (1)
Card(1-45)
web/src/pages/ProfileList/index.jsx (6)
web/src/pages/ProfileList/ExtendedContent.jsx (3)
data(69-69)i(72-72)ExtendedContent(100-112)web/src/components/Card.jsx (1)
Card(1-45)web/src/index.jsx (1)
apiService(23-23)web/src/pages/Home/index.jsx (1)
apiService(23-23)web/src/pages/Settings/index.jsx (1)
onExport(134-142)web/src/components/Spinner.jsx (1)
Spinner(1-16)
web/src/pages/ProfileEdit/StandardProfileForm.jsx (3)
web/src/pages/ProfileEdit/index.jsx (3)
data(18-18)onSave(65-74)pressureAvailable(10-10)web/src/components/Card.jsx (1)
Card(1-45)web/src/components/Spinner.jsx (1)
Spinner(1-16)
web/src/components/Navigation.jsx (1)
web/src/components/Header.jsx (1)
useLocation(5-5)
🪛 GitHub Actions: Deploy PR Preview
web/src/pages/Settings/index.jsx
[error] 1094-1094: Step: 'vite build' failed. Transform failed: Unexpected closing fragment tag does not match opening 'Card' tag.
[error] 1097-1097: Step: 'vite build' failed. Transform failed: Unexpected end of file before a closing fragment tag.
🪛 Biome (2.1.2)
web/src/style.css
[error] 7-7: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 8-8: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 9-9: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 43-43: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 44-44: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 45-45: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 79-79: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 80-80: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 81-81: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 115-115: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 116-116: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 117-117: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test
- GitHub Check: test
🔇 Additional comments (26)
web/src/components/Spinner.jsx (1)
2-6: Keeploading-xl— it's a valid DaisyUI spinner size (do not apply the suggested diff)DaisyUI supports loading-xs, loading-sm, loading-md, loading-lg, and loading-xl (current docs). So the original replacement is unnecessary unless your project uses an older DaisyUI version.
- File: web/src/components/Spinner.jsx — sizeMap entry
16: 'loading-xl'— leave as-is.- If the class has no effect locally, check your daisyui version in package.json/lockfile and upgrade (older releases only went up to loading-lg).
Likely an incorrect or invalid review comment.
web/package.json (1)
16-16: daisyUI v5 is compatible with Tailwind v4 — verify project configShort: daisyUI v5.0.x supports Tailwind CSS v4 and the new CSS
@pluginAPI, but you must register daisyUI in your main CSS (not via JS) and use Tailwind’s Vite plugin (@tailwindcss/vite). Please check/adjust the following.
- Files to verify/modify:
- web/package.json — daisyUI version (OK if ^5.0.50):
"daisyui": "^5.0.50",- main CSS entry (e.g., app.css / index.css): ensure you use
(Move any daisyUI options into the@import "tailwindcss"; @plugin "daisyui";@plugin "daisyui" { ... }block if needed.)- tailwind.config.js: remove
require('daisyui')from the JSpluginsarray (plugin JS registration is deprecated for v5).- vite.config.{js,ts}: ensure you use the Vite plugin:
import tailwind from "@tailwindcss/vite"; export default defineConfig({ plugins: [tailwind()] });- PostCSS config: audit/remove legacy plugins (postcss-import, autoprefixer, postcss-nesting) as recommended by Tailwind v4 migration.
If you’d like, I can scan the repo to confirm these files and show exact lines to change.
src/display/plugins/WebUIPlugin.cpp (1)
179-185: Ignore: clear() does not imply deactivate() — the current sequence is intentionalShort explanation: I inspected the implementations: deactivate() stops the active process (moves currentProcess → lastProcess and emits end events) while clear() deletes lastProcess (and emits brew:clear). clear() does not stop an active process, so calling deactivate(); clear(); before setMode(...) is correct.
Relevant locations:
- src/display/plugins/WebUIPlugin.cpp — ws handler for "req:change-mode": calls controller->deactivate(); controller->clear(); controller->setMode(mode);
- src/display/core/Controller.cpp — deactivate() (stops currentProcess, sets lastProcess) and clear() (deletes lastProcess and triggers brew:clear)
Action: No change required. Optional: if you want an atomic operation, add a Controller::switchMode(int) / switchMode helper that encapsulates deactivate + clear + setMode.
Likely an incorrect or invalid review comment.
web/src/pages/ShotHistory/utils.js (2)
9-9: Good: parseInt radix specified.Specifying base 10 removes environment-dependent parsing. Change looks correct.
17-17: Good: t sample parsed with explicit radix.Consistent with the header timestamp fix. LGTM.
web/src/pages/ShotHistory/HistoryChart.jsx (3)
83-86: Formatting-only change is fine.Arrow callback parens removal doesn't affect behavior. Safe.
94-97: Formatting-only change is fine.Same here; safe and consistent with Prettier rules.
128-128: Formatting-only JSX quote change is fine.No functional impact. LGTM.
web/.prettierrc (1)
2-16: Prettier v3 + Tailwind plugin configuration looks consistentThe config aligns with the code changes (single quotes in JSX, trailing commas, arrowParens: avoid) and the Tailwind plugin usage. No issues from a tooling perspective.
web/src/pages/ProfileList/ExtendedContent.jsx (1)
11-13: Stylistic arrow fn and className tweaks: LGTMThe changes are purely stylistic and match the updated Prettier rules. No behavioral impact.
Also applies to: 23-25, 64-66, 108-110
web/src/pages/ProfileList/ProfileAddCard.jsx (2)
3-14: Card wrapper migration and simplified markup: LGTMGood move to consistent Card usage; semantics and accessibility look fine (icon marked aria-hidden, visible text provided).
6-13: Verify SPA navigation needsIf you’re using a client-side router, will cause a full page reload. Consider using the router’s Link component for in-app navigation.
Would you like me to refactor this to use your router’s Link (if present) and handle active states?
web/src/components/ProcessInfo.jsx (1)
20-27: Progress calculation and markup swap: LGTMCalculation guards for zero targets and the replacement are sound and improve semantics.
Also applies to: 29-36
web/src/style.css (1)
153-153: Confirm Tailwind v4 @source usage is intentional@source inline(...) is Tailwind v4’s content hint. Ensure your build uses Tailwind v4 and that your tooling (PostCSS/CLI) processes this file so the generated col-span utilities are retained in production builds.
If needed, I can help verify tailwind and daisyUI versions in package.json and your build pipeline.
web/src/pages/Settings/PluginCard.jsx (1)
208-209: Consider using a password manager-friendly name attributeThe name attribute "haPassword" is close but could be more standardized for password managers.
id='haPassword' -name='haPassword' +name='haPassword' type='password'Actually, upon review, the name attribute is fine as-is for this context.
web/src/components/Header.jsx (2)
5-12: Clean path comparison implementationThe path-based active state detection using
useLocationis well implemented. The conditional className assignment provides clear visual feedback for the current route.
78-80: Correct use of camelCase attributesGood correction from
fillRuleandclipRuleto use camelCase as required in JSX.web/src/components/OverviewChart.jsx (2)
135-149: Well-structured chart lifecycle managementThe chart creation and cleanup logic is properly implemented with the destroy call in the cleanup function, preventing memory leaks.
162-196: Comprehensive responsive behavior implementationThe resize event handler effectively adjusts font sizes and tick limits based on screen size. The use of
chart.update('none')for performance optimization is a good practice.web/src/components/Footer.jsx (1)
1-36: Consistent styling updates aligned with DaisyUI themeThe Footer component correctly adopts the new DaisyUI theme tokens (e.g.,
border-base-content/10,text-error) and maintains proper accessibility withrel="noreferrer"on external links.web/src/utils/themeManager.js (2)
4-12: Robust theme retrieval with proper validationThe
getStoredThemefunction correctly validates againstAVAILABLE_THEMESand provides appropriate fallback handling with error logging.
42-45: Clean initialization functionThe
initializeThemefunction provides a clean entry point for theme initialization on app load.web/src/pages/Home/index.jsx (1)
25-39: Storage event only fires across documents (e.g., other tabs), not within the same tabIf layout is changed in the same tab, this listener won’t fire. Ensure the setter also updates local component state or use a shared signal/event bus.
Do you update
dashboardLayoutstate in the same tab when calling setDashboardLayout? If not, I can propose a small shared store or context to keep all views in sync without relying on the storage event.web/src/pages/Autotune/index.jsx (1)
86-105: Verify API contract: input ranges and labels no longer match sent parametersUI labels imply “Tuning Goal” [0–100] and “Window Size” [1–10], yet you’re sending
{ time, samples }as-is toreq:autotune-start. If the backend expects seconds fortime, this is a breaking change.Please confirm:
- Does
req:autotune-startexpecttimein seconds or a 0–100 goal?- Does
samplesalign with “Window Size” 1–10?If the API still expects seconds/samples, consider either mapping UI inputs to expected units or renaming state and payload fields. I can generate a patch once you confirm the intended contract.
Also applies to: 107-124, 14-21
web/src/index.jsx (1)
63-65: LGTM! Theme initialization before render prevents flash of unstyled contentGood practice to initialize the theme before rendering to ensure consistent styling from the start.
web/src/pages/ProfileList/index.jsx (1)
21-30: LGTM! Clean Chart.js registrationThe consolidated Chart.register call is cleaner and follows Chart.js best practices.
| className={`card bg-base-100 shadow-xl ${gridClasses} ${fullHeight ? 'h-full' : ''} ${className}`} | ||
| role={role} | ||
| > | ||
| {title && ( | ||
| <div className='card-header px-4 pt-4'> | ||
| <h2 className='card-title text-lg sm:text-xl'>{title}</h2> | ||
| </div> | ||
| )} | ||
| <div className={`card-body flex flex-col gap-2 p-4 ${fullHeight ? 'flex-1' : ''}`}> | ||
| {children} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Align Card markup with DaisyUI conventions; remove non-standard card-header
DaisyUI expects card-title inside card-body. card-header is not a documented class and may not style as intended.
Apply this diff to move the title into card-body and avoid redundant padding:
- <div
- className={`card bg-base-100 shadow-xl ${gridClasses} ${fullHeight ? 'h-full' : ''} ${className}`}
- role={role}
- >
- {title && (
- <div className='card-header px-4 pt-4'>
- <h2 className='card-title text-lg sm:text-xl'>{title}</h2>
- </div>
- )}
- <div className={`card-body flex flex-col gap-2 p-4 ${fullHeight ? 'flex-1' : ''}`}>
- {children}
- </div>
+ <div
+ className={`card bg-base-100 shadow-xl ${gridClasses} ${fullHeight ? 'h-full' : ''} ${className}`}
+ role={role}
+ >
+ <div className={`card-body flex flex-col gap-2 ${fullHeight ? 'flex-1' : ''}`}>
+ {title && <h2 className='card-title text-lg sm:text-xl'>{title}</h2>}
+ {children}
+ </div>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| className={`card bg-base-100 shadow-xl ${gridClasses} ${fullHeight ? 'h-full' : ''} ${className}`} | |
| role={role} | |
| > | |
| {title && ( | |
| <div className='card-header px-4 pt-4'> | |
| <h2 className='card-title text-lg sm:text-xl'>{title}</h2> | |
| </div> | |
| )} | |
| <div className={`card-body flex flex-col gap-2 p-4 ${fullHeight ? 'flex-1' : ''}`}> | |
| {children} | |
| <div | |
| className={`card bg-base-100 shadow-xl ${gridClasses} ${fullHeight ? 'h-full' : ''} ${className}`} | |
| role={role} | |
| > | |
| <div className={`card-body flex flex-col gap-2 ${fullHeight ? 'flex-1' : ''}`}> | |
| {title && <h2 className='card-title text-lg sm:text-xl'>{title}</h2>} | |
| {children} | |
| </div> |
🤖 Prompt for AI Agents
In web/src/components/Card.jsx around lines 32 to 41, the component uses a
non-standard card-header wrapper which conflicts with DaisyUI conventions; move
the title markup into the card-body and remove the extra header wrapper and its
padding. Specifically, remove the <div className='card-header px-4 pt-4'> block
and place the <h2 className='card-title text-lg sm:text-xl'>{title}</h2>
directly at the top of the card-body container, adjusting the card-body
padding/classes to avoid redundant padding when title is present so the title
uses DaisyUI's card-title inside card-body.
| className = | ||
| 'btn btn-md justify-start gap-3 w-full bg-primary text-primary-content hover:bg-primary hover:text-primary-content px-2'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Active state should handle nested routes; use DaisyUI "btn-primary" for active
Exact path equality fails to mark parent sections active for nested routes (e.g., /settings/plugins). Also, let DaisyUI handle active colors via btn-primary instead of manual bg/text classes.
Apply this diff:
- className =
- 'btn btn-md justify-start gap-3 w-full bg-primary text-primary-content hover:bg-primary hover:text-primary-content px-2';
+ className =
+ 'btn btn-primary btn-md justify-start gap-3 w-full px-2';And extend the condition to include nested paths:
- if (props.active || path === props.link) {
+ if (
+ props.active ||
+ path === props.link ||
+ (props.link !== '/' && path.startsWith(props.link))
+ ) {Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In web/src/components/Navigation.jsx around lines 8-9, the current className
uses hardcoded bg/text hover classes and exact path equality which fails to mark
parent items active for nested routes; replace the manual
bg-primary/text-primary-content classes with DaisyUI's "btn-primary" for the
active state and update the active-condition to match nested routes (e.g., use
location.pathname.startsWith(item.path) or NavLink's isActive callback) so
parent sections (like /settings) are active when child routes
(/settings/plugins) are visited; ensure the class string toggles "btn btn-md
justify-start gap-3 w-full px-2" plus "btn-primary" only when the nested-match
condition is true.
web/src/components/Navigation.jsx
Outdated
| <i className={props.iconClass || ''} /> | ||
| <span>{props.label}</span> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Treat icons as decorative for a11y
Mark icons as aria-hidden so screen readers don’t announce them redundantly (the label text already conveys meaning).
Apply this diff:
- <i className={props.iconClass || ''} />
+ <i className={props.iconClass || ''} aria-hidden="true" />Additionally, consider setting aria-current="page" on the anchor when active. Example (outside the changed lines, for illustration):
// inside MenuItem return:
<a
href={props.link}
className={className}
aria-current={
props.active ||
path === props.link ||
(props.link !== '/' && path.startsWith(props.link))
? 'page'
: undefined
}
>🤖 Prompt for AI Agents
In web/src/components/Navigation.jsx around lines 13-14, the icon element is not
marked decorative; add aria-hidden="true" to the <i> element so screen readers
ignore it (keep props.iconClass fallback), and ensure accessibility by setting
aria-current="page" on the anchor when the item is active (compute active using
props.active or current path matching props.link and set aria-current to 'page'
only when active, otherwise leave it undefined).
| <MenuItem label='Dashboard' link='/' iconClass='fa fa-home' /> | ||
| <hr className='h-5 border-0' /> | ||
| <div className='space-y-1.5'> | ||
| <MenuItem label='Profiles' link='/profiles' iconClass='fa fa-list' /> | ||
| <MenuItem label='Shot History' link='/history' iconClass='fa fa-timeline' /> | ||
| </div> | ||
| <hr className="h-5 border-0" /> | ||
| <div class="space-y-1.5"> | ||
| <MenuItem label="PID Autotune" link="/pidtune" iconClass="fa fa-temperature-half" /> | ||
| <MenuItem label="Bluetooth Scales" link="/scales" iconClass="fa-brands fa-bluetooth-b" /> | ||
| <MenuItem label="Settings" link="/settings" iconClass="fa fa-cog" /> | ||
| <hr className='h-5 border-0' /> | ||
| <div className='space-y-1.5'> | ||
| <MenuItem label='PID Autotune' link='/pidtune' iconClass='fa fa-temperature-half' /> | ||
| <MenuItem label='Bluetooth Scales' link='/scales' iconClass='fa-brands fa-bluetooth-b' /> | ||
| <MenuItem label='Settings' link='/settings' iconClass='fa fa-cog' /> | ||
| </div> | ||
| <hr class="h-5 border-0" /> | ||
| <div class="space-y-1.5"> | ||
| <MenuItem label="System & Updates" link="/ota" iconClass="fa fa-rotate" /> | ||
| <hr className='h-5 border-0' /> | ||
| <div className='space-y-1.5'> | ||
| <MenuItem label='System & Updates' link='/ota' iconClass='fa fa-rotate' /> | ||
| </div> |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Standardize Font Awesome version/prefixes; some icon names may be invalid
The usage mixes FA4/5/6 conventions: 'fa', 'fa-brands', and icon names like 'fa-timeline' and 'fa-rotate' which may not exist (or be Pro-only) depending on the FA version. This can lead to missing icons.
Verification script to determine FA version and usages:
If using FA6, consider normalizing to 'fa-solid'/'fa-regular'/'fa-brands' and valid icon names. Example mappings:
- Dashboard: fa-solid fa-house
- Shot History: fa-solid fa-clock-rotate-left (instead of fa-timeline)
- System & Updates: fa-solid fa-arrows-rotate (instead of fa-rotate)
- Settings: fa-solid fa-gear (fa-cog alias may still work)
Optionally, introduce a small Icon component to enforce consistent prefixes:
function Icon({ name, style = 'solid' }) {
const prefix = style === 'brands' ? 'fa-brands' : style === 'regular' ? 'fa-regular' : 'fa-solid';
return <i className={`${prefix} ${name}`} aria-hidden="true" />;
}🏁 Script executed:
#!/bin/bash
# Find Font Awesome references and determine version
rg -n "fontawesome|fortawesome|fa[- ]" -A 2
fd package.json | xargs -I{} sh -c 'echo "== {} =="; rg -n "@fortawesome|fontawesome" {}'
# Check CSS includes in HTML
fd -e html | xargs -I{} sh -c 'echo "== {} =="; rg -n "fontawesome|fortawesome|fa-" {} -A 2'Length of output: 36709
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== root package.json (first 200 lines) ==="
if [ -f package.json ]; then sed -n '1,200p' package.json; else echo "no root package.json"; fi
echo
echo "=== web/package.json (full) ==="
if [ -f web/package.json ]; then cat web/package.json; else echo "no web/package.json"; fi
echo
echo "=== rg search for Font Awesome / kit / prefixes across repo ==="
rg -n "@fortawesome|fontawesome|font-awesome|kit.fontawesome|use.fontawesome|cdnjs.cloudflare.com/ajax/libs/font-awesome|maxcdn.bootstrapcdn.com/font-awesome|fontawesome-free|fa-brands|fa-solid|fa-regular|fab|fas|far|fal|\\bfa\\b" -S || true
echo
echo "=== HTML files that include Font Awesome/kit references (matches) ==="
rg -n "kit.fontawesome|use.fontawesome|cdnjs.cloudflare.com/ajax/libs/font-awesome|maxcdn.bootstrapcdn.com/font-awesome|fontawesome.com" -S || true
echo
echo "=== CSS files importing fontawesome (matches) ==="
rg -n "@import .*font-awesome|@import .*fontawesome|@import .*fortawesome" -S || trueLength of output: 268485
Action: standardize Font Awesome usage to the FA6 prefixes and valid icon names
Short summary — web/index.html loads Font Awesome 6.7.2 (CDN) but the codebase mixes "fa", "fa-solid", and "fa-brands" and uses names that may be missing or Pro-only (e.g. fa-timeline, fa-rotate). That will produce missing icons at runtime.
Files I found that need attention (non-exhaustive)
- web/index.html (includes https://cdnjs.cloudflare.com/ajax/libs/font-awesome/6.7.2/css/all.min.css)
- web/src/components/Navigation.jsx (lines ~22–37)
- web/src/components/Header.jsx (menu icons)
- web/src/pages/Home/ProcessControls.jsx (mixed prefixes)
- web/src/pages/ShotHistory/HistoryCard.jsx, web/src/pages/Settings/index.jsx, web/src/pages/ProfileList/, web/src/pages/ProfileEdit/ and many other files (rg found many "fa" occurrences)
Suggested replacements (example for Navigation.jsx — update to FA6-style prefixes and known free icons):
<MenuItem label='Dashboard' link='/' iconClass='fa-solid fa-house' />
<hr className='h-5 border-0' />
<div className='space-y-1.5'>
<MenuItem label='Profiles' link='/profiles' iconClass='fa-solid fa-list' />
<MenuItem label='Shot History' link='/history' iconClass='fa-solid fa-clock' />
</div>
<hr className='h-5 border-0' />
<div className='space-y-1.5'>
<MenuItem label='PID Autotune' link='/pidtune' iconClass='fa-solid fa-temperature-half' />
<MenuItem label='Bluetooth Scales' link='/scales' iconClass='fa-brands fa-bluetooth-b' />
<MenuItem label='Settings' link='/settings' iconClass='fa-solid fa-gear' />
</div>
<hr className='h-5 border-0' />
<div className='space-y-1.5'>
<MenuItem label='System & Updates' link='/ota' iconClass='fa-solid fa-arrows-rotate' />
</div>
Notes / next steps (short)
- Verify each icon name against the Font Awesome 6 free catalogue (some names in the repo may be Pro-only or renamed). Replace invalid names with FA6 equivalents (or swap to acceptable free icons).
- Normalize prefixes across the repo to fa-solid / fa-regular / fa-brands (or keep a single style if you prefer).
- Optionally add a small Icon wrapper to centralize prefix logic and avoid repeating incorrect classes:
function Icon({ name, style = 'solid' }) {
const prefix = style === 'brands' ? 'fa-brands' : style === 'regular' ? 'fa-regular' : 'fa-solid';
return <i className={`${prefix} ${name}`} aria-hidden="true" />;
}I verified the FA6 CDN reference and many mixed usages with ripgrep; please update the files above and run the app to confirm there are no missing glyphs.
🤖 Prompt for AI Agents
In web/src/components/Navigation.jsx around lines 22 to 37, the menu uses
mixed/legacy Font Awesome classes and some icon names that are missing or
Pro-only (e.g. "fa", "fa-timeline", "fa-rotate"); update each MenuItem to use
FA6 free prefixes and valid icon names (use fa-solid / fa-regular / fa-brands as
appropriate), replacing invalid names with FA6 free equivalents (examples: use
fa-solid fa-house for Dashboard, fa-solid fa-clock for Shot History, fa-solid
fa-gear for Settings, fa-solid fa-arrows-rotate for System & Updates, keep
fa-brands fa-bluetooth-b for Bluetooth), then scan and normalize other files
(Header.jsx, ProcessControls.jsx, HistoryCard.jsx, ProfileList/Edit, Settings,
etc.) to the same FA6 prefixes and verify every icon name against the Font
Awesome 6 free catalog; optionally add a small shared Icon wrapper to centralize
prefix logic to avoid future inconsistencies.
| <span | ||
| className={`loading loading-spinner ${sizeMap[size] || 'loading-md'}`} | ||
| role='status' | ||
| aria-label='Loading' | ||
| /> |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Preserve numeric size behavior: add inline-style fallback for arbitrary numbers
Today, unknown numeric sizes silently fall back to 'md', which changes visuals relative to a pixel-based SVG. Add an inline width/height fallback so numeric values still work as before.
Apply this diff:
className={`loading loading-spinner ${sizeMap[size] || 'loading-md'}`}
role='status'
- aria-label='Loading'
+ aria-label='Loading'
+ style={!sizeMap[size] && Number.isFinite(size) ? { width: size, height: size } : undefined}Optionally verify existing usages and size values to assess impact:
🏁 Script executed:
#!/bin/bash
# List all Spinner usages
rg -n "<Spinner" -S
echo
echo "Spinner usages with explicit numeric sizes (by value):"
# Extract numeric sizes passed as JSX expressions: size={N}
rg -n --pcre2 "<Spinner[^>]*size=\{([0-9]+)\}" -or '$1' | sort -n | uniq -cLength of output: 1315
Preserve numeric size behavior: add inline-style fallback for arbitrary numbers
Unknown numeric sizes currently fall back to 'loading-md', changing visuals vs. pixel-based numeric sizes. Add an inline width/height fallback so numeric props render as before.
Files to update / check:
- Change: web/src/components/Spinner.jsx (the span at lines ~10-14)
- Affected usages with numeric sizes (will be preserved by this change):
- web/src/pages/OTA/index.jsx:62 —
- web/src/pages/OTA/index.jsx:70 —
- web/src/pages/Autotune/index.jsx:48 —
- web/src/pages/ProfileEdit/index.jsx:79 —
- web/src/pages/ProfileEdit/StandardProfileForm.jsx:157 — {saving && }
- web/src/pages/Settings/index.jsx:159 —
- web/src/pages/Settings/index.jsx:1079 — {submitting && }
- web/src/pages/ShotHistory/index.jsx:59 —
- web/src/pages/ProfileList/index.jsx:333 —
Apply this diff:
className={`loading loading-spinner ${sizeMap[size] || 'loading-md'}`}
role='status'
- aria-label='Loading'
+ aria-label='Loading'
+ style={!sizeMap[size] && Number.isFinite(size) ? { width: size, height: size } : undefined}Notes: this preserves numeric (number-typed) size props. If you also pass numeric sizes as strings (e.g., "8"), consider parsing or handling those separately.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <span | |
| className={`loading loading-spinner ${sizeMap[size] || 'loading-md'}`} | |
| role='status' | |
| aria-label='Loading' | |
| /> | |
| <span | |
| className={`loading loading-spinner ${sizeMap[size] || 'loading-md'}`} | |
| role='status' | |
| aria-label='Loading' | |
| style={!sizeMap[size] && Number.isFinite(size) ? { width: size, height: size } : undefined} | |
| /> |
🤖 Prompt for AI Agents
In web/src/components/Spinner.jsx around lines 10-14, the span currently only
uses className fallback 'loading-md' which changes visuals for numeric sizes;
detect if the incoming size prop is a number and, when so, add an inline style
object setting width and height to `${size}px` (merge with any existing style
prop), while keeping the existing className expression (sizeMap[size] ||
'loading-md') for non-numeric or mapped sizes so numeric-typed sizes render as
before.
| useEffect(() => { | ||
| if (connected.value) { | ||
| await loadHistory(); | ||
| loadHistory(); | ||
| } | ||
| }, [connected.value]); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Async function removed from useEffect could impact error handling
The change from async to synchronous removes the ability to catch errors from loadHistory(). Consider wrapping the call in a try-catch or handling errors within loadHistory() itself.
🏁 Script executed:
#!/bin/bash
# Check if loadHistory has internal error handling
ast-grep --pattern $'const loadHistory = async () => {
$$$
}'Length of output: 532
Handle errors from loadHistory invoked in useEffect
loadHistory (web/src/pages/ShotHistory/index.jsx — lines ~35–40) performs an async request but has no try/catch. Calling it from useEffect (lines ~41–45) without awaiting or catching can produce unhandled promise rejections. Add error handling either inside loadHistory or around the call in useEffect.
Files / locations to update:
- web/src/pages/ShotHistory/index.jsx
- loadHistory implementation (around lines 35–40)
- useEffect invoking loadHistory (around lines 41–45)
Suggested fixes (pick one):
Option A — handle errors inside loadHistory:
const loadHistory = async () => {
try {
const response = await apiService.request({ tp: 'req:history:list' });
const history = response.history.map(parseHistoryData).reverse();
setHistory(history);
} catch (err) {
console.error('Failed to load history', err);
// optionally set error state / show toast
} finally {
setLoading(false);
}
};Option B — catch around the call in useEffect:
useEffect(() => {
if (connected.value) {
(async () => {
try {
await loadHistory();
} catch (err) {
console.error('Failed to load history', err);
}
})();
}
}, [connected.value]);🤖 Prompt for AI Agents
In web/src/pages/ShotHistory/index.jsx around lines 35–45, the async loadHistory
call can produce unhandled promise rejections; update either the loadHistory
implementation (around lines ~35–40) to wrap its await logic in
try/catch/finally and handle/log errors and ensure loading state is cleared, or
update the useEffect (lines ~41–45) to call loadHistory inside an
immediately-invoked async function that awaits loadHistory and catches/logs
errors; pick one approach and keep error handling consistent (log error and
optionally set an error state or show a toast).
| try { | ||
| return localStorage.getItem(DASHBOARD_LAYOUT_KEY) || 'process-first'; | ||
| } catch (error) { | ||
| console.warn('getDashboardLayout: localStorage access failed:', error); | ||
| return 'process-first'; | ||
| } | ||
| }; |
There was a problem hiding this comment.
Bug: getDashboardLayout returns JSON string (with quotes) rather than the actual value
setDashboardLayout JSON-stringifies the layout, but getDashboardLayout returns the raw stored string without JSON.parse. Consumers will see '"process-first"' instead of 'process-first'.
Apply this diff to parse safely and fall back correctly:
try {
- return localStorage.getItem(DASHBOARD_LAYOUT_KEY) || 'process-first';
+ const raw = localStorage.getItem(DASHBOARD_LAYOUT_KEY);
+ if (!raw) return 'process-first';
+ try {
+ const parsed = JSON.parse(raw);
+ return typeof parsed === 'string' ? parsed : 'process-first';
+ } catch {
+ // Support legacy/plain storage without JSON
+ return raw;
+ }
} catch (error) {
console.warn('getDashboardLayout: localStorage access failed:', error);
return 'process-first';
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| try { | |
| return localStorage.getItem(DASHBOARD_LAYOUT_KEY) || 'process-first'; | |
| } catch (error) { | |
| console.warn('getDashboardLayout: localStorage access failed:', error); | |
| return 'process-first'; | |
| } | |
| }; | |
| try { | |
| const raw = localStorage.getItem(DASHBOARD_LAYOUT_KEY); | |
| if (!raw) return 'process-first'; | |
| try { | |
| const parsed = JSON.parse(raw); | |
| return typeof parsed === 'string' ? parsed : 'process-first'; | |
| } catch { | |
| // Support legacy/plain storage without JSON | |
| return raw; | |
| } | |
| } catch (error) { | |
| console.warn('getDashboardLayout: localStorage access failed:', error); | |
| return 'process-first'; | |
| } | |
| }; |
🤖 Prompt for AI Agents
In web/src/utils/dashboardManager.js around lines 8 to 14, getDashboardLayout
currently returns the raw stored string (possibly JSON-stringified with quotes)
causing consumers to receive values like '"process-first"'; update the function
to read the localStorage value, JSON.parse it inside a try/catch (handling
non-JSON strings by falling back to the raw value if parse fails), and return
the parsed value or the default 'process-first' on error, while preserving the
existing console.warn fallback behavior.
| export const setDashboardLayout = layout => { | ||
| if (layout === null || layout === undefined) { | ||
| console.error('setDashboardLayout: Layout cannot be null or undefined'); | ||
| return false; | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
SSR/unsupported environment guard missing in setDashboardLayout
In non-browser contexts, referencing localStorage can throw. Mirror the guard you used in getDashboardLayout.
Apply this diff:
export const setDashboardLayout = layout => {
+ if (typeof window === 'undefined' || !window.localStorage) {
+ console.warn('setDashboardLayout: localStorage not available; ignoring.');
+ return false;
+ }
if (layout === null || layout === undefined) {
console.error('setDashboardLayout: Layout cannot be null or undefined');
return false;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const setDashboardLayout = layout => { | |
| if (layout === null || layout === undefined) { | |
| console.error('setDashboardLayout: Layout cannot be null or undefined'); | |
| return false; | |
| } | |
| export const setDashboardLayout = layout => { | |
| if (typeof window === 'undefined' || !window.localStorage) { | |
| console.warn('setDashboardLayout: localStorage not available; ignoring.'); | |
| return false; | |
| } | |
| if (layout === null || layout === undefined) { | |
| console.error('setDashboardLayout: Layout cannot be null or undefined'); | |
| return false; | |
| } |
🤖 Prompt for AI Agents
In web/src/utils/dashboardManager.js around lines 16 to 21, setDashboardLayout
currently accesses localStorage without checking for a browser/SSR environment;
add the same guard used in getDashboardLayout (e.g. verify typeof window !==
'undefined' and window.localStorage exists) before using localStorage, and if
the guard fails return false and avoid any localStorage calls or errors.
| let serializedLayout; | ||
| try { | ||
| serializedLayout = JSON.stringify(layout); | ||
| } catch (error) { | ||
| console.error('setDashboardLayout: Layout is not JSON-serializable:', error); | ||
| return false; | ||
| } | ||
|
|
||
| try { | ||
| localStorage.setItem(DASHBOARD_LAYOUT_KEY, serializedLayout); | ||
| return true; | ||
| } catch (error) { | ||
| console.error('setDashboardLayout: Failed to store layout in localStorage:', error); | ||
| return false; | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Validate input against allowed layouts to prevent bad state
Constrain layout to DASHBOARD_LAYOUTS values; this prevents storing arbitrary/invalid values and makes downstream comparisons reliable.
Apply this diff:
export const setDashboardLayout = layout => {
@@
- let serializedLayout;
+ // Validate against known layouts
+ if (typeof layout !== 'string' || !Object.values(DASHBOARD_LAYOUTS).includes(layout)) {
+ console.error('setDashboardLayout: Invalid layout value', layout);
+ return false;
+ }
+
+ let serializedLayout;
try {
serializedLayout = JSON.stringify(layout);
} catch (error) {
console.error('setDashboardLayout: Layout is not JSON-serializable:', error);
return false;
}Note: Referencing DASHBOARD_LAYOUTS here is safe at runtime since module initialization completes before any function is called.
Also applies to: 39-42
🤖 Prompt for AI Agents
In web/src/utils/dashboardManager.js around lines 22 to 37 (and also apply same
change at lines 39 to 42), the function currently serializes and stores any
layout value; restrict accepted layouts to the predefined DASHBOARD_LAYOUTS to
avoid persisting invalid state. Before JSON.stringify, check that layout is one
of the allowed values (e.g., DASHBOARD_LAYOUTS.includes(layout) or equivalent),
log a clear error and return false if it is not allowed, and only then proceed
to serialize and write to localStorage; apply the same validation check in the
other code path at lines 39–42.
|
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
web/src/components/Navigation.jsx (1)
4-10: Use DaisyUI btn-primary/btn-ghost and support nested routes; add aria-currentUse DaisyUI’s button variants instead of manual bg/text classes, and mark parent items active for nested paths (e.g., /settings/plugins). Also expose active state to ATs via aria-current.
- let className = - 'btn btn-md justify-start gap-3 w-full text-base-content hover:text-base-content hover:bg-base-content/10 bg-transparent border-none px-2'; - const { path } = useLocation(); - if (props.active || path === props.link) { - className = - 'btn btn-md justify-start gap-3 w-full bg-primary text-primary-content hover:bg-primary hover:text-primary-content px-2'; - } + const { path } = useLocation(); + const isActive = + props.active || + path === props.link || + (props.link !== '/' && path.startsWith(props.link)); + const className = `btn btn-md justify-start gap-3 w-full px-2 ${isActive ? 'btn-primary' : 'btn-ghost'}`; @@ - <a href={props.link} className={className}> + <a href={props.link} className={className} aria-current={isActive ? 'page' : undefined}>Also applies to: 12-12
web/src/pages/Settings/index.jsx (1)
433-437: Fix deprecatedselectedattribute usage.The
selectedattribute on<option>elements is deprecated in React. The timezone select should be controlled via thevalueprop.Apply this fix to make it a controlled component:
<select id='timezone' name='timezone' className='select select-bordered w-full' + value={formData.timezone || ''} onChange={onChange('timezone')} > {timezones.map(timezone => ( - <option key={timezone} value={timezone} selected={formData.timezone === timezone}> + <option key={timezone} value={timezone}> {timezone} </option> ))} </select>web/src/pages/Home/index.jsx (2)
17-17: Use the centralized storage key to avoid driftImport the layout storage key constant to keep the event listener aligned with the manager.
Apply this diff:
-import { getDashboardLayout } from '../../utils/dashboardManager.js'; +import { getDashboardLayout, DASHBOARD_LAYOUT_KEY } from '../../utils/dashboardManager.js';
25-39: Parse storage event values and use the shared key; current code will set state to a quoted JSON stringThe storage event handler has two issues:
- It uses a hardcoded string 'dashboardLayout' instead of the imported constant
- It doesn't parse the JSON-serialized value from localStorage, which will cause state to be set to a quoted string like '"process-first"' instead of 'process-first'
Apply this diff to fix both issues:
useEffect(() => { setDashboardLayout(getDashboardLayout()); const handleStorageChange = e => { - if (e.key === 'dashboardLayout') { - setDashboardLayout(e.newValue || 'process-first'); + if (e.key === DASHBOARD_LAYOUT_KEY) { + try { + const layout = e.newValue ? JSON.parse(e.newValue) : 'process-first'; + setDashboardLayout(layout); + } catch { + // Fallback for non-JSON values + setDashboardLayout('process-first'); + } } };Additionally, update
getDashboardLayoutin dashboardManager.js to parse the stored value:export const getDashboardLayout = () => { if (typeof window === 'undefined' || !window.localStorage) { return 'process-first'; } try { const raw = localStorage.getItem(DASHBOARD_LAYOUT_KEY); return raw ? JSON.parse(raw) : 'process-first'; } catch (error) { console.warn('getDashboardLayout: Failed to parse layout:', error); return 'process-first'; } };
🧹 Nitpick comments (6)
web/src/components/Navigation.jsx (2)
13-15: Decorative icon a11y looks good; optionally render icon conditionallyGood call adding aria-hidden on the icon. Optionally, avoid rendering an empty when iconClass is falsy.
- <i className={props.iconClass || ''} aria-hidden='true' /> + {props.iconClass && <i className={props.iconClass} aria-hidden='true' />}
21-21: Label the navigation landmarkAdd an aria-label so screen readers can identify this as the primary navigation.
- <nav className='hidden lg:col-span-2 lg:block'> + <nav className='hidden lg:col-span-2 lg:block' aria-label='Primary navigation'>web/src/pages/Settings/index.jsx (2)
16-16: Consider removing unnecessary state mutation.The
genstate is now a constant and doesn't seem to be used for triggering re-fetches. If it's not needed for cache invalidation, consider removing it entirely.-const [gen] = useState(0);And update the query key:
-const { isLoading, data: fetchedSettings } = useQuery(`settings/${gen}`, async () => { +const { isLoading, data: fetchedSettings } = useQuery('settings', async () => {
131-131: Minor: Updated dependency array includes unused formRef.The
formRefin the dependency array is unnecessary since refs don't trigger re-renders when their current value changes.-[setFormData, formRef, formData], +[setFormData, formData],web/src/components/OverviewChart.jsx (2)
131-149: Consider adding error handling for Chart.js instantiation.The chart creation could fail if the canvas element is corrupted or if Chart.js encounters an initialization error. Consider wrapping the chart creation in a try-catch block.
Apply this diff to add error handling:
useEffect(() => { if (!ref.current) return; - const chartData = getChartData(machine.value.history); - const newChart = new Chart(ref.current, chartData); - setChart(newChart); + try { + const chartData = getChartData(machine.value.history); + const newChart = new Chart(ref.current, chartData); + setChart(newChart); - // Cleanup function to destroy chart on unmount - return () => { - if (newChart) { - newChart.destroy(); - } - }; + // Cleanup function to destroy chart on unmount + return () => { + if (newChart) { + newChart.destroy(); + } + }; + } catch (error) { + console.error('Failed to initialize chart:', error); + } }, []); // Empty dependency array - only run on mount
161-196: Consider throttling resize events for better performance.While using
'none'mode for updates helps, frequent resize events could still impact performance. Consider throttling the resize handler.Add a throttle utility at the top of the file:
const throttle = (func, delay) => { let timeout; let lastRan; return (...args) => { if (!lastRan) { func(...args); lastRan = Date.now(); } else { clearTimeout(timeout); timeout = setTimeout(() => { if ((Date.now() - lastRan) >= delay) { func(...args); lastRan = Date.now(); } }, delay - (Date.now() - lastRan)); } }; };Then apply this diff to throttle the resize handler:
- const handleResize = () => { + const handleResize = throttle(() => { const isSmallScreen = window.innerWidth < 640; // Update legend font size chart.options.plugins.legend.labels.font.size = isSmallScreen ? 10 : 12; // Update title font size chart.options.plugins.title.font.size = isSmallScreen ? 14 : 16; // Update axis font sizes chart.options.scales.y.ticks.font.size = isSmallScreen ? 10 : 12; chart.options.scales.y1.ticks.font.size = isSmallScreen ? 10 : 12; chart.options.scales.x.ticks.font.size = isSmallScreen ? 10 : 12; // Update maxTicksLimit for x-axis chart.options.scales.x.ticks.maxTicksLimit = isSmallScreen ? 5 : 10; // Update the chart to apply changes chart.update('none'); // Use 'none' mode for better performance - }; + }, 250);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
web/src/components/Navigation.jsx(1 hunks)web/src/components/OverviewChart.jsx(6 hunks)web/src/pages/Home/ProcessControls.jsx(2 hunks)web/src/pages/Home/index.jsx(1 hunks)web/src/pages/ProfileEdit/StandardProfileForm.jsx(5 hunks)web/src/pages/Settings/index.jsx(6 hunks)web/src/pages/ShotHistory/HistoryChart.jsx(3 hunks)web/src/pages/ShotHistory/index.jsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- web/src/pages/ShotHistory/HistoryChart.jsx
- web/src/pages/ShotHistory/index.jsx
- web/src/pages/ProfileEdit/StandardProfileForm.jsx
🧰 Additional context used
🧬 Code Graph Analysis (5)
web/src/components/Navigation.jsx (1)
web/src/components/Header.jsx (2)
useLocation(5-5)className(6-7)
web/src/components/OverviewChart.jsx (2)
web/src/pages/ShotHistory/HistoryChart.jsx (3)
ref(111-111)chartData(112-112)chart(110-110)web/src/services/ApiService.js (2)
machine(193-207)machine(193-207)
web/src/pages/Home/ProcessControls.jsx (6)
web/src/components/ProcessInfo.jsx (1)
processInfo(4-4)web/src/pages/Home/index.jsx (3)
apiService(23-23)mode(50-50)changeMode(41-49)web/src/pages/ShotHistory/index.jsx (1)
apiService(32-32)web/src/index.jsx (1)
apiService(23-23)web/src/pages/ProfileList/index.jsx (1)
apiService(210-210)web/src/services/ApiService.js (2)
ApiServiceContext(191-191)ApiServiceContext(191-191)
web/src/pages/Settings/index.jsx (5)
web/src/utils/themeManager.js (2)
getStoredTheme(4-12)handleThemeChange(48-51)web/src/utils/dashboardManager.js (2)
setDashboardLayout(16-37)setDashboardLayout(16-37)web/src/components/Spinner.jsx (1)
Spinner(1-16)web/src/config/zones.js (2)
timezones(1-463)timezones(1-463)web/src/pages/Settings/PluginCard.jsx (1)
PluginCard(3-222)
web/src/pages/Home/index.jsx (6)
web/src/utils/dashboardManager.js (4)
setDashboardLayout(16-37)setDashboardLayout(16-37)getDashboardLayout(3-14)getDashboardLayout(3-14)web/src/pages/Home/ProcessControls.jsx (2)
apiService(72-72)ProcessControls(65-252)web/src/index.jsx (1)
apiService(23-23)web/src/services/ApiService.js (4)
ApiServiceContext(191-191)ApiServiceContext(191-191)machine(193-207)machine(193-207)web/src/components/Card.jsx (1)
Card(1-45)web/src/components/OverviewChart.jsx (1)
OverviewChart(131-203)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: deploy
- GitHub Check: test
- GitHub Check: test
🔇 Additional comments (30)
web/src/pages/Settings/index.jsx (17)
8-10: LGTM! Clean integration of theme and dashboard management.The imports properly utilize the theme and dashboard utility modules with clean, focused function imports.
18-18: LGTM! Theme state management properly initialized.The
currentThemestate correctly tracks the current theme selection and is properly initialized from storage.
37-37: Good default value handling for dashboard layout.The fallback to 'process-first' ensures consistent behavior when the setting is not yet configured.
45-48: LGTM! Proper theme initialization from storage.The useEffect correctly initializes the theme state from localStorage using the theme manager utility.
50-98: Enhanced change handler with dashboard layout support.The updated
onChangefunction properly handles the new dashboard layout control by callingsetDashboardLayout()to persist changes.
135-142: LGTM! Improved export implementation with template literals.The updated export function uses modern string interpolation and maintains the same functionality while being more readable.
148-148: LGTM! Modern arrow function syntax.The upload handler now uses cleaner arrow function syntax while maintaining the same functionality.
158-161: LGTM! Updated loading state with proper styling.The loading spinner now uses DaisyUI classes with proper flex layout and centered positioning.
165-190: LGTM! Clean header layout with improved button styling.The new header layout uses proper flex positioning and DaisyUI button styling. The import/export functionality is preserved with better visual hierarchy.
192-329: LGTM! Well-structured card-based layout for core settings.The Temperature settings and User preferences cards are well-organized with proper form controls and DaisyUI styling. The grid layout provides good visual separation.
331-375: LGTM! Excellent theme and dashboard layout controls.The Web Settings card properly integrates theme management with controlled select components. The dashboard layout control correctly calls
setDashboardLayout()for persistence.
377-456: LGTM! Comprehensive system preferences card.The System preferences card includes all necessary network and time settings with proper form controls and validation attributes.
458-548: LGTM! Detailed machine configuration settings.The Machine settings card provides comprehensive hardware configuration options with appropriate input types, placeholders, and validation constraints.
550-633: LGTM! Well-organized display settings with proper state management.The Display settings card correctly handles the standby display toggle logic and properly disables the brightness input when standby display is disabled.
635-762: LGTM! Conditional LED control settings with proper layout.The Sunrise Settings card is properly conditionally rendered based on LED control capability and uses a well-structured grid layout for color controls.
764-766: LGTM! Clean plugin integration.The Plugins card properly integrates the PluginCard component with the existing form data and change handlers.
769-793: LGTM! Improved footer layout with proper button styling.The footer section uses proper flex layout, DaisyUI styling, and maintains all the necessary form submission functionality including the restart option.
web/src/components/OverviewChart.jsx (3)
5-129: LGTM!The
getChartDatafunction properly configures Chart.js with responsive settings and appropriate scales. The responsive font sizing and dynamic tick formatting are well-implemented.
151-159: LGTM!The chart update logic correctly handles data changes and guards against null chart instances.
198-203: LGTM!The responsive container with flex layout and min-height constraints ensures proper chart rendering across different screen sizes.
web/src/pages/Home/ProcessControls.jsx (8)
1-14: LGTM!Clean imports and utility functions are well-structured.
16-63: LGTM!The BrewProgress component has good visual hierarchy and responsive design with appropriate text sizing for different screen sizes.
105-123: LGTM!The button click handlers and icon logic are clear and follow a logical state progression.
125-147: LGTM!The tab-based mode selector with responsive styling provides a clean UI for mode switching.
178-223: LGTM!The conditional rendering logic for expanded and collapsed states is well-organized and provides appropriate user feedback for each mode.
225-252: LGTM!The brew target selector and action button implementation is clean and follows consistent styling patterns.
254-260: LGTM!PropTypes validation is properly defined for all required props.
149-176: Add null checks for pressure values to prevent runtime errors.The pressure display uses optional chaining for
toFixed(0)but still attempts to call the method on potentially undefined values, which could cause runtime errors.Apply this diff to add proper null checks:
<span className='text-base-content'> - {status.value.currentPressure?.toFixed(0) || 0} /{' '} - {status.value.targetPressure?.toFixed(0) || 0} bar + {status.value.currentPressure != null ? status.value.currentPressure.toFixed(0) : 0} /{' '} + {status.value.targetPressure != null ? status.value.targetPressure.toFixed(0) : 0} bar </span>Likely an incorrect or invalid review comment.
web/src/pages/Home/index.jsx (2)
41-50: LGTM!The mode change callback is properly memoized and correctly uses the ApiServiceContext.
52-89: LGTM!The responsive grid layout with conditional rendering based on dashboard layout preference is well-implemented. The Card components are properly configured with appropriate responsive breakpoints.
| <MenuItem label='Dashboard' link='/' iconClass='fa fa-home' /> | ||
| <hr className='h-5 border-0' /> | ||
| <div className='space-y-1.5'> | ||
| <MenuItem label='Profiles' link='/profiles' iconClass='fa fa-list' /> | ||
| <MenuItem label='Shot History' link='/history' iconClass='fa fa-timeline' /> | ||
| </div> | ||
| <hr className="h-5 border-0" /> | ||
| <div class="space-y-1.5"> | ||
| <MenuItem label="PID Autotune" link="/pidtune" iconClass="fa fa-temperature-half" /> | ||
| <MenuItem label="Bluetooth Scales" link="/scales" iconClass="fa-brands fa-bluetooth-b" /> | ||
| <MenuItem label="Settings" link="/settings" iconClass="fa fa-cog" /> | ||
| <hr className='h-5 border-0' /> | ||
| <div className='space-y-1.5'> | ||
| <MenuItem label='PID Autotune' link='/pidtune' iconClass='fa fa-temperature-half' /> | ||
| <MenuItem label='Bluetooth Scales' link='/scales' iconClass='fa-brands fa-bluetooth-b' /> | ||
| <MenuItem label='Settings' link='/settings' iconClass='fa fa-cog' /> | ||
| </div> | ||
| <hr class="h-5 border-0" /> | ||
| <div class="space-y-1.5"> | ||
| <MenuItem label="System & Updates" link="/ota" iconClass="fa fa-rotate" /> | ||
| <hr className='h-5 border-0' /> | ||
| <div className='space-y-1.5'> | ||
| <MenuItem label='System & Updates' link='/ota' iconClass='fa fa-rotate' /> |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Normalize to Font Awesome 6 prefixes and valid free icon names
The menu mixes legacy FA4-style classes (fa fa-*) and uses names that don’t exist in FA6 free (e.g., fa-timeline, fa-rotate). Align with FA6 free prefixes to avoid missing glyphs.
- <MenuItem label='Dashboard' link='/' iconClass='fa fa-home' />
+ <MenuItem label='Dashboard' link='/' iconClass='fa-solid fa-house' />
@@
- <MenuItem label='Profiles' link='/profiles' iconClass='fa fa-list' />
- <MenuItem label='Shot History' link='/history' iconClass='fa fa-timeline' />
+ <MenuItem label='Profiles' link='/profiles' iconClass='fa-solid fa-list' />
+ <MenuItem label='Shot History' link='/history' iconClass='fa-solid fa-clock-rotate-left' />
@@
- <MenuItem label='PID Autotune' link='/pidtune' iconClass='fa fa-temperature-half' />
+ <MenuItem label='PID Autotune' link='/pidtune' iconClass='fa-solid fa-temperature-half' />
<MenuItem label='Bluetooth Scales' link='/scales' iconClass='fa-brands fa-bluetooth-b' />
- <MenuItem label='Settings' link='/settings' iconClass='fa fa-cog' />
+ <MenuItem label='Settings' link='/settings' iconClass='fa-solid fa-gear' />
@@
- <MenuItem label='System & Updates' link='/ota' iconClass='fa fa-rotate' />
+ <MenuItem label='System & Updates' link='/ota' iconClass='fa-solid fa-arrows-rotate' />Run this to find remaining legacy usages across the repo and ensure consistency with FA6:
🏁 Script executed:
#!/bin/bash
# Find legacy FA4-style usages and potential invalid icon names
set -euo pipefail
echo "Legacy FA4 'fa ' prefix (excluding fa-brands/fa-solid/fa-regular):"
rg -n "class(Name)?=.[^'\"]*\\bfa\\s(?!brands|solid|regular)\\b" -S || true
echo
echo "Potentially invalid FA icon names seen in code (timeline|rotate without arrows):"
rg -n "fa-((timeline)|(rotate)(?!-))" -S || true
echo
echo "All FA class usages for manual inspection:"
rg -n "class(Name)?=.[^'\"]*fa(-brands|-solid|-regular)?\\sfa-[a-z0-9-]+" -S || trueLength of output: 4107
Normalize Font Awesome 6 usage (replace legacy "fa" classes and invalid icon names) — action required
Ripgrep shows many legacy "fa" usages across the codebase. Replace plain "fa" (FA4) with the appropriate FA6 prefix (fa-solid / fa-regular / fa-brands) and verify each icon name exists in Font Awesome Free 6.
Files/lines that need attention (from repo scan):
- web/src/components/Header.jsx:50, 61
- web/src/pages/ProfileEdit/StandardProfileForm.jsx:118, 137, 243
- web/src/pages/ProfileEdit/ProfileTypeSelection.jsx:20, 36
- web/src/pages/ShotHistory/HistoryCard.jsx:31, 40, 47, 52
- web/src/pages/Settings/index.jsx:174, 181
- web/src/pages/ProfileList/ProfileAddCard.jsx:10
- web/src/pages/Home/ProcessControls.jsx:151, 159, 173, 232, 239
- web/src/pages/ProfileList/index.jsx:123, 130, 137, 144, 151, 186, 348, 356
Suggested replacements for the Navigation snippet (update iconClass to FA6 prefixes / valid names):
- <MenuItem label='Dashboard' link='/' iconClass='fa fa-home' />
+ <MenuItem label='Dashboard' link='/' iconClass='fa-solid fa-house' />
@@
- <MenuItem label='Profiles' link='/profiles' iconClass='fa fa-list' />
- <MenuItem label='Shot History' link='/history' iconClass='fa fa-timeline' />
+ <MenuItem label='Profiles' link='/profiles' iconClass='fa-solid fa-list' />
+ <MenuItem label='Shot History' link='/history' iconClass='fa-solid fa-clock-rotate-left' />
@@
- <MenuItem label='PID Autotune' link='/pidtune' iconClass='fa fa-temperature-half' />
+ <MenuItem label='PID Autotune' link='/pidtune' iconClass='fa-solid fa-temperature-half' />
<MenuItem label='Bluetooth Scales' link='/scales' iconClass='fa-brands fa-bluetooth-b' />
- <MenuItem label='Settings' link='/settings' iconClass='fa fa-cog' />
+ <MenuItem label='Settings' link='/settings' iconClass='fa-solid fa-gear' />
@@
- <MenuItem label='System & Updates' link='/ota' iconClass='fa fa-rotate' />
+ <MenuItem label='System & Updates' link='/ota' iconClass='fa-solid fa-arrows-rotate' />Run this improved scan locally to find remaining legacy usages and to extract unique fa-* tokens for manual verification:
#!/bin/bash
set -euo pipefail
echo "All lines containing 'fa' in class/className:"
rg -n "class(Name)?=.[^'\"\n]*\bfa\b" -S || true
echo
echo "Lines that likely still use legacy FA4 (exclude fa-solid/fa-regular/fa-brands):"
rg -n "class(Name)?=.[^'\"\n]*\bfa\b" -S | rg -v "fa-(brands|solid|regular)" || true
echo
echo "Unique fa-* tokens found (inspect these against FA6 Free):"
rg -n "class(Name)?=.[^'\"\n]*\bfa\b" -S | sed -E 's/^[^:]+:[0-9]+://' | grep -oE "fa-[a-z0-9-]+" | sort -u || trueNotes:
- Confirm each replacement icon exists in Font Awesome Free 6 before committing (some FA4 names were renamed).
- Keep fa-brands usage for brand icons (GitHub, Discord) and use fa-solid/fa-regular for UI glyphs.
| const ProcessControls = props => { | ||
| // brew is true when mode equals 1 (Brew mode), false otherwise | ||
| const { brew, mode, changeMode } = props; | ||
| const brewTarget = status.value.brewTarget; | ||
| const processInfo = status.value.process; | ||
| const active = !!processInfo?.a; | ||
| const finished = !!processInfo && !active; | ||
| const apiService = useContext(ApiServiceContext); | ||
|
|
||
| // Determine if we should show expanded view | ||
| const shouldExpand = brew && (active || finished || (brew && !active && !finished)); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Validate the shouldExpand logic for all mode transitions.
The shouldExpand logic on line 75 seems to have redundant conditions. When brew is true and neither active nor finished, the condition brew && !active && !finished is already satisfied, making the entire expression simplify to just brew.
🏁 Script executed:
#!/bin/bash
# Check if there are any other usages of shouldExpand that might explain the logic
rg -A 5 "shouldExpand"Length of output: 1490
Simplify shouldExpand — the expression is a tautology and should be reduced to brew.
The current expression always evaluates to brew (the inner clause is true for all combinations of active/finished when brew is true). Ripgrep shows shouldExpand is only used in web/src/pages/Home/ProcessControls.jsx.
Files to change:
- web/src/pages/Home/ProcessControls.jsx — replace the declaration
Suggested replacement:
- const shouldExpand = brew && (active || finished || (brew && !active && !finished));
+ const shouldExpand = !!brew;Optional: while not required, you can also replace other explicit ... && brew && ... checks with shouldExpand && ... for clarity.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const ProcessControls = props => { | |
| // brew is true when mode equals 1 (Brew mode), false otherwise | |
| const { brew, mode, changeMode } = props; | |
| const brewTarget = status.value.brewTarget; | |
| const processInfo = status.value.process; | |
| const active = !!processInfo?.a; | |
| const finished = !!processInfo && !active; | |
| const apiService = useContext(ApiServiceContext); | |
| // Determine if we should show expanded view | |
| const shouldExpand = brew && (active || finished || (brew && !active && !finished)); | |
| const ProcessControls = props => { | |
| // brew is true when mode equals 1 (Brew mode), false otherwise | |
| const { brew, mode, changeMode } = props; | |
| const brewTarget = status.value.brewTarget; | |
| const processInfo = status.value.process; | |
| const active = !!processInfo?.a; | |
| const finished = !!processInfo && !active; | |
| const apiService = useContext(ApiServiceContext); | |
| // Determine if we should show expanded view | |
| const shouldExpand = !!brew; |
🤖 Prompt for AI Agents
In web/src/pages/Home/ProcessControls.jsx around lines 65 to 75, the
shouldExpand expression is a tautology; replace the current complex expression
with a simple boolean that equals brew (i.e., set shouldExpand = brew). Update
any nearby uses that redundantly check brew when they can use shouldExpand for
clarity, but the primary fix is to replace the existing declaration with a
single-line assignment of shouldExpand to brew.
* Add Daisy Ui (jniebuhr#339) * Initial Daisy Ui transition and 4 themes to start * Update Ui * Fix SonarCloud issues * Remove Computed Single line Chart Register * Use JSX syntax * Apply Bunny Fixes * Use card component * Use Card * Fix active and hover states * Format and Style Nav Bar Better * Move profiles over(need to fix spacing between lables) * Fix Mobile Layout * Fix mobile layout * Listen to the bunny * Update style.css * Convert Scale and Autotune * Delete dashboard.html * I like the titles on the left better * fix: Fix some bugs in the UI * feat: Change overview chart readability * Dasiy Ui (jniebuhr#347) * Migrate Settings to DaisyUi and removed unused error handling * Fix eslint error var to const * Fix eslint * Remove Unused container-fluid and create context. Run Format. Migrate Process Info to daisy * Change to aria label * Use Daisy UI tool tips * Remove Unused Styles * Use Semantic Colors * Add Theme Manager to settings Right now this is a separate card but can be changed whenever * Standardize spacing and Padding as well as component usage * Fix Profile pages and use consistent styling * Improve Dark style ? * add aria * Fix Save * Consistent Styles. Use 10/5 Grid Layout * Fix Button Spacing * Prevent race condition * Fix button Position and use 5/10 layout on home page * fix card class again * javascript:void(0) * simple bp * add theme urls * add safelist * fix profile cards * FMT and When saving profile rout back to profile list * Clarify Settings Card * Fix Chart Height * This needs a bit of testing but should fix card duplication * Add Switching the layout of the home page * Format and Fix Lint errors * Fix some code reviews * Update index.jsx * Split Settings Page up a bit * Clean up Inputs * Fix Radix --------- Co-authored-by: Jochen Niebuhr <kontakt@ju-hh.de> * feat: Add landscape dashboard layout, fix profile edit --------- Co-authored-by: DreadPirateRoberts <46958197+ManfredRichthofen@users.noreply.github.com>
* Add Daisy Ui (jniebuhr#339) * Initial Daisy Ui transition and 4 themes to start * Update Ui * Fix SonarCloud issues * Remove Computed Single line Chart Register * Use JSX syntax * Apply Bunny Fixes * Use card component * Use Card * Fix active and hover states * Format and Style Nav Bar Better * Move profiles over(need to fix spacing between lables) * Fix Mobile Layout * Fix mobile layout * Listen to the bunny * Update style.css * Convert Scale and Autotune * Delete dashboard.html * I like the titles on the left better * fix: Fix some bugs in the UI * feat: Change overview chart readability * Dasiy Ui (jniebuhr#347) * Migrate Settings to DaisyUi and removed unused error handling * Fix eslint error var to const * Fix eslint * Remove Unused container-fluid and create context. Run Format. Migrate Process Info to daisy * Change to aria label * Use Daisy UI tool tips * Remove Unused Styles * Use Semantic Colors * Add Theme Manager to settings Right now this is a separate card but can be changed whenever * Standardize spacing and Padding as well as component usage * Fix Profile pages and use consistent styling * Improve Dark style ? * add aria * Fix Save * Consistent Styles. Use 10/5 Grid Layout * Fix Button Spacing * Prevent race condition * Fix button Position and use 5/10 layout on home page * fix card class again * javascript:void(0) * simple bp * add theme urls * add safelist * fix profile cards * FMT and When saving profile rout back to profile list * Clarify Settings Card * Fix Chart Height * This needs a bit of testing but should fix card duplication * Add Switching the layout of the home page * Format and Fix Lint errors * Fix some code reviews * Update index.jsx * Split Settings Page up a bit * Clean up Inputs * Fix Radix --------- Co-authored-by: Jochen Niebuhr <kontakt@ju-hh.de> * feat: Add landscape dashboard layout, fix profile edit --------- Co-authored-by: DreadPirateRoberts <46958197+ManfredRichthofen@users.noreply.github.com>
* Add Daisy Ui (jniebuhr#339) * Initial Daisy Ui transition and 4 themes to start * Update Ui * Fix SonarCloud issues * Remove Computed Single line Chart Register * Use JSX syntax * Apply Bunny Fixes * Use card component * Use Card * Fix active and hover states * Format and Style Nav Bar Better * Move profiles over(need to fix spacing between lables) * Fix Mobile Layout * Fix mobile layout * Listen to the bunny * Update style.css * Convert Scale and Autotune * Delete dashboard.html * I like the titles on the left better * fix: Fix some bugs in the UI * feat: Change overview chart readability * Dasiy Ui (jniebuhr#347) * Migrate Settings to DaisyUi and removed unused error handling * Fix eslint error var to const * Fix eslint * Remove Unused container-fluid and create context. Run Format. Migrate Process Info to daisy * Change to aria label * Use Daisy UI tool tips * Remove Unused Styles * Use Semantic Colors * Add Theme Manager to settings Right now this is a separate card but can be changed whenever * Standardize spacing and Padding as well as component usage * Fix Profile pages and use consistent styling * Improve Dark style ? * add aria * Fix Save * Consistent Styles. Use 10/5 Grid Layout * Fix Button Spacing * Prevent race condition * Fix button Position and use 5/10 layout on home page * fix card class again * javascript:void(0) * simple bp * add theme urls * add safelist * fix profile cards * FMT and When saving profile rout back to profile list * Clarify Settings Card * Fix Chart Height * This needs a bit of testing but should fix card duplication * Add Switching the layout of the home page * Format and Fix Lint errors * Fix some code reviews * Update index.jsx * Split Settings Page up a bit * Clean up Inputs * Fix Radix --------- Co-authored-by: Jochen Niebuhr <kontakt@ju-hh.de> * feat: Add landscape dashboard layout, fix profile edit --------- Co-authored-by: DreadPirateRoberts <46958197+ManfredRichthofen@users.noreply.github.com>
2 participants
By @ManfredRichthofen
Summary by CodeRabbit
New Features
UI/UX
Bug Fixes
Chores