Conversation
… Process Info to daisy
Right now this is a separate card but can be changed whenever
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughAdds an ESP32-S3-SuperMini PlatformIO board and headless environments; introduces theme and dashboard layout managers with persistence; large UI refactors to card/grid layouts, accessibility, and styling (DaisyUI/theme updates); chart, spinner, and form lifecycle/structure changes; many presentational and minor formatting edits across the web app. Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant ThemeManager
participant Document
App->>ThemeManager: initializeTheme()
ThemeManager->>ThemeManager: getStoredTheme()
ThemeManager->>Document: applyTheme(data-theme)
Document-->>App: DOM has theme applied
App->>App: render UI
sequenceDiagram
participant Settings
participant DashboardManager
participant LocalStorage
participant Home
Settings->>DashboardManager: setDashboardLayout(value)
DashboardManager->>LocalStorage: write 'dashboardLayout'
LocalStorage-->>Home: storage event fired
Home->>DashboardManager: getDashboardLayout()
DashboardManager-->>Home: layout value
Home->>Home: re-render with new layout
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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 review |
✅ Actions performedReview triggered.
|
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🔭 Outside diff range comments (5)
web/src/services/ApiService.js (1)
124-140: request() timeout not cleared on resolve → avoid stray timeouts/rejects.When the response arrives, the 30s timeout is not cleared. It’s a no-op after resolve, but still runs and wastes resources. Clear it on success.
- return new Promise((resolve, reject) => { + return new Promise((resolve, reject) => { + let timeoutId; // Create a listener for the response with matching rid - const listenerId = this.on(returnType, response => { + const listenerId = this.on(returnType, response => { if (response.rid === rid) { // Clean up the listener this.off(returnType, listenerId); + 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 });web/src/pages/ProfileList/ExtendedContent.jsx (1)
104-106: Chart cleanup on unmount to prevent leaks.Destroy the Chart instance in a cleanup to avoid canvas memory leaks and duplicate instances on remount.
- useEffect(() => { - const ct = new Chart(ref.current, config); - }, [ref]); + useEffect(() => { + const chart = new Chart(ref.current, config); + return () => chart.destroy(); + }, [config]);web/src/components/OverviewChart.jsx (1)
123-123: Inconsistent maxTicksLimit initializationThe initial chart configuration sets
maxTicksLimit: window.innerWidth < 640 ? 5 : 5(line 123), but the resize handler updates it toisSmallScreen ? 5 : 10(line 180). The initial configuration should match the resize logic.- maxTicksLimit: window.innerWidth < 640 ? 5 : 5, + maxTicksLimit: window.innerWidth < 640 ? 5 : 10,Also applies to: 180-180
web/src/pages/ProfileList/index.jsx (1)
269-283: Add null check for profile duplicationThe
findoperation on line 271 could return undefined if the profile is not found, which would cause issues.const onDuplicate = useCallback( async id => { setLoading(true); const original = profiles.find(p => p.id === id); - if (original) { + if (!original) { + console.error(`Profile with id ${id} not found`); + setLoading(false); + return; + } const copy = { ...original }; delete copy.id; delete copy.selected; delete copy.favorite; copy.label = `${original.label} Copy`; await apiService.request({ tp: 'req:profiles:save', profile: copy }); - } await loadProfiles(); }, [apiService, profiles, setLoading], );web/src/pages/ProfileEdit/StandardProfileForm.jsx (1)
173-190: Improve numeric comparison for volumetric targetUsing strict equality with 0 for a float value might cause issues. Consider using a threshold or handling empty/null values explicitly.
const onVolumetricTargetChange = value => { - if (value === 0) { + if (!value || value === 0) { onChange({ ...phase, targets: null, }); return; }
🧹 Nitpick comments (20)
boards/esp32-s3-supermini.json (3)
17-19: Flash speed 80MHz QIO: confirm stability on this module.Some ESP32‑S3 modules are unstable at 80MHz depending on flash chip routing. If you see random boot/reset issues, consider 40MHz.
Optional fallback diff:
- "f_flash": "80000000L", + "f_flash": "40000000L",
19-19: PSRAM type set to qio: confirm against SuperMini’s BOM.Many S3 boards ship OPI PSRAM. Using the wrong PSRAM type can cause boot failures or crashes. Please confirm whether the SuperMini is QSPI (qio) or OPI.
If it’s OPI, adjust:
- "psram_type": "qio", + "psram_type": "opi",
44-53: Metadata nits: board URL/vendor may not match “SuperMini.”The URL targets DevKitC‑1, vendor is “Espressif.” If this SuperMini is by another vendor, consider updating these fields for clarity.
web/package.json (1)
29-30: Prettier v3 + Tailwind plugin: confirm plugin loading and stability.Looks good. Since .prettierrc includes the plugin entry, Prettier v3 will auto-load it. Two quick follow-ups:
- Consider adding a CI-friendly check script (e.g., "format:check": "prettier -c .") to prevent churn.
- The Tailwind plugin’s class sorting can be noisy; if you want determinism, consider pinning the plugin version instead of using a caret.
Do you want me to add a "format:check" script and open a quick follow-up PR?
web/index.html (1)
26-26: Relying on DaisyUI theme tokens is fine—minimize FOUC by setting data-theme earlier.Switching to bg-base-100 aligns with theme tokens. To further reduce any flash of incorrect theme, consider inlining a tiny head script to set data-theme from localStorage before CSS paints.
Example (in ):
<script> try { const t = localStorage.getItem('theme'); if (t) document.documentElement.setAttribute('data-theme', t); } catch {} </script>web/src/utils/uuid.js (1)
2-4: Verify ES2017+ support for trailing commas in calls
We didn’t find any browserslist, Vite build target, or esbuild target config in the repo. Trailing commas in function calls are only valid in ES2017 and above—older browsers or embedded webviews will error on syntax.• File to review:
web/src/utils/uuid.js(lines 2–4)
• Either add an explicit browserslist or Vite/esbuildtarget: "es2017"(or higher)
• Or adjust Prettier to"trailingComma": "es5"(which omits trailing commas in call sites)Please confirm your supported runtime baseline or update the config accordingly.
web/.prettierrc (1)
2-16: Prettier config aligns with Tailwind class sorting.The options look consistent with the repo-wide formatting shift. One note: arrowParens: "avoid" may reduce diff noise but can hinder readability for complex params; fine as long as linting rules don’t conflict.
web/src/pages/ProfileList/ExtendedContent.jsx (1)
108-109: Minor a11y: consider providing fallback text inside canvas.Optionally add fallback text for screen readers inside the canvas.
- <canvas className='max-h-36 w-full' ref={ref} /> + <canvas className='max-h-36 w-full' ref={ref}> + Pressure/Flow profile visualization + </canvas>web/src/components/Navigation.jsx (1)
21-37: Grid span adjustment: confirm main content spans sum to 12 at lg.Nav moved lg:col-span-3 → lg:col-span-2. Ensure the main content updated accordingly (e.g., lg:col-span-10) so the grid remains consistent with 12 columns.
web/src/components/Card.jsx (1)
2-15: Consider memoizing getGridClasses to avoid recreation on each renderThe
getGridClassesfunction is recreated on every render. Since it only depends on props, consider usinguseMemofor optimization.+import { useMemo } from 'react'; + export default function Card({ xs, sm, md, lg, xl, title, children, className = '', role, fullHeight = false }) { - const getGridClasses = () => { + const gridClasses = useMemo(() => { 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(); + }, [xs, sm, md, lg, xl]);web/src/pages/OTA/index.jsx (1)
158-175: Potential form submission conflict with button typesThe Update Display and Update Controller buttons have
type="submit"but useonClickhandlers that prevent default form submission. Consider usingtype="button"to avoid confusion.<button - type='submit' + type='button' name='update' className='btn btn-secondary' disabled={!formData.displayUpdateAvailable || submitting} onClick={() => onUpdate('display')} > Update Display </button> <button - type='submit' + type='button' name='update' className='btn btn-accent' disabled={!formData.controllerUpdateAvailable || submitting} onClick={() => onUpdate('controller')} > Update Controller </button>web/src/utils/themeManager.js (2)
1-2: Consider exporting AVAILABLE_THEMESThe
AVAILABLE_THEMESarray could be useful in other components (like Settings.jsx) to maintain consistency.const THEME_STORAGE_KEY = 'gaggimate-daisyui-theme'; -const AVAILABLE_THEMES = ['light', 'dark', 'coffee', 'nord']; +export const AVAILABLE_THEMES = ['light', 'dark', 'coffee', 'nord'];
48-51: Add defensive check for event structureThe
handleThemeChangefunction assumesevent.target.valueexists. Consider adding a defensive check.export function handleThemeChange(event) { + if (!event?.target?.value) { + console.warn('Invalid event structure for theme change'); + return false; + } const theme = event.target.value; - setStoredTheme(theme); + return setStoredTheme(theme); }web/src/pages/Settings/index.jsx (2)
16-16: Unused state variablegenThe
genstate is initialized but never updated. If it's no longer needed for triggering re-fetches, consider removing it or document why it's kept.- const [gen] = useState(0);Or if needed for initial fetch:
- const { isLoading, data: fetchedSettings } = useQuery(`settings/${gen}`, async () => { + const { isLoading, data: fetchedSettings } = useQuery('settings', async () => {
434-438: Use value prop instead of selected attribute for option elementsReact recommends using the
valueprop on theselectelement rather than theselectedattribute onoptionelements.<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>This pattern should be applied to all select elements in the file (lines 237-243, 620-623, 874-884).
web/src/pages/ShotHistory/HistoryCard.jsx (1)
27-27: Simplify onClick handlersThe arrow functions wrapping the onClick handlers are unnecessary when passing arguments directly.
- onClick={() => onExport()} + onClick={onExport}- onClick={() => onDelete(shot.id)} + onClick={() => onDelete(shot.id)}Note: Only the first onClick can be simplified since
onDeleterequires an argument.Also applies to: 36-36
web/src/pages/ProfileEdit/ProfileTypeSelection.jsx (1)
10-15: Consider browser compatibility for Space key detectionFor better browser compatibility, especially with older browsers, consider checking for both ' ' and 'Space' when detecting the spacebar key press.
onKeyDown={e => { - if (e.key === 'Enter' || e.key === ' ') { + if (e.key === 'Enter' || e.key === ' ' || e.key === 'Spacebar') { e.preventDefault(); onSelect('standard'); } }}web/src/pages/ProfileList/index.jsx (2)
213-213: Simplify favoriteCount calculationThe current reduce operation can be simplified using filter for better readability.
- const favoriteCount = profiles.map(p => (p.favorite ? 1 : 0)).reduce((a, b) => a + b, 0); + const favoriteCount = profiles.filter(p => p.favorite).length;
295-301: Add Firefox compatibility commentFor consistency with other export implementations in the codebase, add the Firefox compatibility comment.
downloadAnchorNode.setAttribute('href', dataStr); downloadAnchorNode.setAttribute('download', 'profiles.json'); - document.body.appendChild(downloadAnchorNode); + document.body.appendChild(downloadAnchorNode); // required for firefox downloadAnchorNode.click(); downloadAnchorNode.remove();web/src/pages/ProfileEdit/StandardProfileForm.jsx (1)
333-333: Simplify pump mode button logicThe onClick conditions for pump mode buttons could be clearer and more consistent.
- onClick={() => mode !== 'power' && onFieldChange('pump', 100)} + onClick={() => { + if (mode !== 'power') { + onFieldChange('pump', 100); + } + }}Similarly for pressure and flow mode buttons:
- onClick={() => - mode !== 'pressure' && - onFieldChange('pump', { target: 'pressure', pressure: 0, flow: 0 }) - } + onClick={() => { + if (mode !== 'pressure') { + onFieldChange('pump', { target: 'pressure', pressure: 0, flow: 0 }); + } + }}Also applies to: 345-346, 357-358
📜 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 (34)
boards/esp32-s3-supermini.json(1 hunks)platformio.ini(1 hunks)web/.prettierrc(1 hunks)web/index.html(1 hunks)web/package.json(1 hunks)web/src/components/Card.jsx(1 hunks)web/src/components/Footer.jsx(1 hunks)web/src/components/Header.jsx(2 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(6 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(9 hunks)web/src/pages/Scales/index.jsx(3 hunks)web/src/pages/Settings/index.jsx(6 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/_404.jsx(1 hunks)web/src/services/ApiService.js(2 hunks)web/src/style.css(6 hunks)web/src/utils/dashboardManager.js(1 hunks)web/src/utils/themeManager.js(1 hunks)web/src/utils/uuid.js(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (18)
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/ProfileList/ExtendedContent.jsx (2)
web/src/components/OverviewChart.jsx (1)
ref(133-133)web/src/pages/ShotHistory/HistoryChart.jsx (1)
ref(112-112)
web/src/pages/ShotHistory/HistoryCard.jsx (4)
web/src/components/Card.jsx (1)
Card(1-29)web/src/pages/Settings/index.jsx (1)
onExport(134-142)web/src/pages/ProfileList/index.jsx (2)
onExport(285-302)onDelete(232-239)web/src/pages/ShotHistory/index.jsx (1)
onDelete(47-54)
web/src/index.jsx (1)
web/src/utils/themeManager.js (1)
initializeTheme(42-45)
web/src/pages/ProfileEdit/index.jsx (5)
web/src/pages/Home/index.jsx (1)
apiService(23-23)web/src/index.jsx (1)
apiService(23-23)web/src/pages/ProfileList/index.jsx (1)
apiService(210-210)web/src/pages/ProfileEdit/ProfileTypeSelection.jsx (1)
ProfileTypeSelection(3-46)web/src/pages/ProfileEdit/StandardProfileForm.jsx (1)
StandardProfileForm(5-163)
web/src/pages/ProfileList/ProfileAddCard.jsx (1)
web/src/components/Card.jsx (1)
Card(1-29)
web/src/pages/Home/index.jsx (5)
web/src/utils/dashboardManager.js (4)
setDashboardLayout(7-9)setDashboardLayout(7-9)getDashboardLayout(3-5)getDashboardLayout(3-5)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-29)web/src/components/OverviewChart.jsx (1)
OverviewChart(131-203)
web/src/pages/ShotHistory/index.jsx (3)
web/src/pages/ShotHistory/utils.js (1)
parseHistoryData(1-37)web/src/components/Spinner.jsx (1)
Spinner(1-16)web/src/pages/ShotHistory/HistoryCard.jsx (1)
HistoryCard(5-62)
web/src/pages/ProfileEdit/ProfileTypeSelection.jsx (2)
web/src/components/Card.jsx (1)
Card(1-29)web/src/pages/ProfileList/index.jsx (1)
onSelect(241-248)
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/pages/Autotune/index.jsx (10)
web/src/pages/Home/index.jsx (1)
apiService(23-23)web/src/index.jsx (1)
apiService(23-23)web/src/pages/ProfileEdit/index.jsx (1)
apiService(13-13)web/src/pages/OTA/index.jsx (1)
apiService(7-7)web/src/pages/ShotHistory/index.jsx (1)
apiService(32-32)web/src/pages/Home/ProcessControls.jsx (1)
apiService(72-72)web/src/pages/ProfileList/index.jsx (1)
apiService(210-210)web/src/components/Card.jsx (1)
Card(1-29)web/src/components/OverviewChart.jsx (1)
OverviewChart(131-203)web/src/components/Spinner.jsx (1)
Spinner(1-16)
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/Scales/index.jsx (1)
web/src/components/Card.jsx (1)
Card(1-29)
web/src/pages/Settings/index.jsx (6)
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(7-9)setDashboardLayout(7-9)web/src/components/Spinner.jsx (1)
Spinner(1-16)web/src/components/Card.jsx (1)
Card(1-29)web/src/config/zones.js (2)
timezones(1-463)timezones(1-463)
web/src/pages/Home/ProcessControls.jsx (6)
web/src/components/ProcessInfo.jsx (1)
processInfo(4-4)web/src/pages/Home/index.jsx (2)
apiService(23-23)mode(50-50)web/src/index.jsx (1)
apiService(23-23)web/src/pages/ProfileEdit/index.jsx (1)
apiService(13-13)web/src/pages/ShotHistory/index.jsx (1)
apiService(32-32)web/src/pages/ProfileList/index.jsx (1)
apiService(210-210)
web/src/pages/ProfileList/index.jsx (6)
web/src/components/Card.jsx (1)
Card(1-29)web/src/pages/Home/index.jsx (1)
apiService(23-23)web/src/index.jsx (1)
apiService(23-23)web/src/pages/ShotHistory/HistoryCard.jsx (1)
onExport(7-16)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-29)web/src/components/Spinner.jsx (1)
Spinner(1-16)
web/src/pages/OTA/index.jsx (4)
web/src/pages/Home/index.jsx (1)
apiService(23-23)web/src/index.jsx (1)
apiService(23-23)web/src/pages/Settings/index.jsx (3)
formRef(25-25)formData(17-17)onSubmit(100-132)web/src/components/Card.jsx (1)
Card(1-29)
🪛 Biome (2.1.2)
web/src/style.css
[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)
🔇 Additional comments (40)
boards/esp32-s3-supermini.json (2)
1-54: Baseline board definition looks consistent.Flags, clock settings, and Arduino/IDF framework compatibility look aligned with typical ESP32‑S3 settings. Good addition.
20-25: USB VID:PID sanity check.0x303A:0x1001 is common for Espressif DevKitC‑1, but the SuperMini may enumerate differently. Mismatched IDs can affect port auto-detection.
To verify locally:
- Windows: Device Manager → Ports/USB → Details → Hardware IDs.
- Linux: lsusb -v | grep -A2 303a or dmesg after plugging the board.
- macOS: system_profiler SPUSBDataType.
If different, update hwids accordingly.platformio.ini (1)
87-90: Headless 8MB env add looks good.Extends display-headless cleanly and targets seeed_xiao_esp32s3 as expected.
web/src/pages/_404.jsx (1)
3-9: Clean DaisyUI migration!The 404 page has been successfully migrated to DaisyUI with proper semantic color tokens and button styling. The simplified layout structure is more maintainable.
web/src/components/ProcessInfo.jsx (2)
23-26: Good defensive programming with progress boundsThe addition of
Math.min(100, ...)ensures the progress never exceeds 100%, preventing visual glitches.
35-35: Excellent use of native HTML progress elementReplacing the custom div-based progress bar with the native
<progress>element improves accessibility and semantic HTML. The DaisyUIprogress-primaryclass provides consistent theming.web/src/components/Footer.jsx (1)
21-25: Good security practice with rel="noreferrer"Adding
rel='noreferrer'to external links prevents potential security issues and information leakage through the Referer header.web/src/components/OverviewChart.jsx (2)
136-149: Excellent chart lifecycle managementThe refactored chart initialization with proper cleanup prevents memory leaks. Creating the chart once on mount and destroying it on unmount is the correct approach.
162-196: Well-implemented responsive behaviorThe resize handler properly updates all font sizes and tick limits based on viewport width, ensuring the chart remains readable on all screen sizes. Using
'none'mode for updates improves performance.web/src/pages/Autotune/index.jsx (1)
86-125: Great accessibility improvements!The addition of proper labels with
htmlForattributes and unique IDs for form inputs significantly improves accessibility. The responsive grid layout and descriptive helper text also enhance the user experience.web/src/components/Header.jsx (1)
88-140: Well-structured navigation menuThe navigation menu is well-organized with logical groupings, proper dividers, and consistent styling. The addition of the System & Updates section aligns well with the OTA functionality.
web/src/pages/Scales/index.jsx (1)
77-87: Excellent UX improvementsGreat implementation of the scanning state with loading spinner and disabled state. The button text change from "Scan for Scales" to "Scanning..." provides clear feedback to users.
web/src/pages/Settings/index.jsx (2)
359-370: Dashboard layout changes persist immediatelyThe dashboard layout is saved to localStorage immediately on change, before the form is submitted. This differs from other settings that only persist after form submission. Consider if this immediate persistence is intentional.
If immediate persistence is desired, this implementation is correct. Otherwise, consider deferring the
setDashboardLayoutcall to the form submission handler.
756-973: Well-structured plugin configuration sectionsThe plugin sections are excellently organized with:
- Consistent visual structure using bg-base-200 cards
- Clear enable/disable toggles
- Conditional rendering of configuration options
- Helpful descriptive text for users
web/src/style.css (3)
5-147: Excellent DaisyUI theme migration with comprehensive color token updates.The theme definitions are well-structured with proper documentation via generator URLs. The comprehensive rebaselining of color tokens across all themes (light, dark, coffee, nord) ensures consistency across the UI.
43-45: Static analysis warnings are false positives.The static analysis tool is incorrectly flagging
default,prefersdark, andcolor-schemeas unknown properties. These are valid DaisyUI theme configuration properties, not standard CSS properties.
153-153: Good addition of responsive grid utility generation.The
@source inlinedirective efficiently generates responsive column span classes, eliminating the need for explicit component-level styling while supporting the new grid-based layouts.web/src/pages/ShotHistory/HistoryChart.jsx (2)
83-96: LGTM! Clean arrow function syntax simplification.The removal of unnecessary parentheses around single parameters in the tick callbacks improves code readability without changing functionality.
128-128: LGTM! Consistent quote style.Single quotes align with the project's standardized styling approach.
web/src/index.jsx (4)
4-4: Good addition of theme initialization.Importing the theme manager for centralized theme handling aligns with the DaisyUI migration.
63-64: Correct timing for theme initialization.Calling
initializeTheme()beforerender()ensures that the stored theme is applied before the UI mounts, preventing flash of unstyled content.
6-6: Clean import optimization.Removing
createContextsince it's not used in this file improves import clarity.
29-37: Improved responsive layout structure.The updated layout classes and grid structure provide better responsive behavior and align with the DaisyUI design system.
web/src/components/Spinner.jsx (1)
1-16: Excellent refactor to leverage DaisyUI spinner classes.The migration from SVG to CSS-based spinner provides several benefits:
- Cleaner code with less markup
- Consistent styling with DaisyUI theme
- Better performance (CSS animations vs SVG)
- Maintains backward compatibility with the size prop
The sizeMap approach elegantly translates numeric sizes to appropriate CSS classes with a sensible fallback.
web/src/pages/Home/ProcessControls.jsx (2)
16-16: Clean arrow function syntax improvements.Removing unnecessary parentheses around single parameters improves readability while maintaining the same functionality.
Also applies to: 65-65
23-248: Excellent migration to DaisyUI design tokens.The comprehensive update to use DaisyUI color tokens (base-content, primary, etc.) throughout the component ensures consistent theming and better integration with the design system. The layout and functionality remain unchanged.
web/src/pages/ShotHistory/index.jsx (4)
37-37: History ordering changed to reverse chronological.The
.reverse()call means history will now display newest shots first. Ensure this aligns with the intended user experience.
41-45: Good cleanup of async useEffect pattern.Moving the async logic into a separate function and calling it directly is cleaner than having an async function directly in useEffect.
47-54: Clean arrow function syntax.Simplified parameter syntax improves readability while maintaining the same callback behavior.
66-80: Improved responsive layout structure.The new grid layout with responsive breakpoints provides better organization and aligns with the DaisyUI migration.
web/src/pages/ProfileEdit/index.jsx (3)
65-74: Improved save flow and dependency array fix.Two excellent improvements:
- Always routing to '/profiles' after save provides consistent UX
- Adding proper dependency array
[apiService, params.id, location]prevents stale closure issues
84-102: Clean layout simplification.The fragment wrapper and updated header structure provide a cleaner, more focused layout while maintaining the same functionality.
92-96: Consistent arrow function syntax.The simplified callback syntax aligns with the broader code style improvements throughout the PR.
web/src/pages/Home/index.jsx (3)
17-17: Excellent implementation of persistent dashboard layout with cross-tab sync.The integration of dashboard layout management provides several benefits:
- Layout preference persists across sessions
- Changes sync across browser tabs in real-time
- Clean storage event handling with proper cleanup
- Sensible default fallback to 'process-first'
Also applies to: 22-22, 25-39
58-81: Well-structured conditional layout rendering.The responsive Card-based layout system allows users to prioritize either process controls or the chart based on their workflow. The consistent use of responsive props (sm={10}, lg={4/6}) ensures good mobile and desktop experiences.
41-49: Clean arrow function syntax improvement.Removing unnecessary parentheses from the single parameter improves code consistency throughout the codebase.
web/src/pages/ShotHistory/HistoryCard.jsx (2)
18-44: Excellent accessibility improvements!The conversion from anchor links to buttons with proper aria-labels and tooltips significantly improves the accessibility of the component. The use of semantic HTML elements and ARIA attributes follows best practices.
7-16: Add dependency array to useCallbackThe
onExportcallback should include an empty dependency array since it doesn't depend on any external values. This follows React best practices and ensures the callback is memoized correctly.const onExport = useCallback(() => { const dataStr = 'data:text/json;charset=utf-8,' + encodeURIComponent(JSON.stringify(shot, undefined, 2)); const downloadAnchorNode = document.createElement('a'); downloadAnchorNode.setAttribute('href', dataStr); downloadAnchorNode.setAttribute('download', shot.id + '.json'); document.body.appendChild(downloadAnchorNode); // required for firefox downloadAnchorNode.click(); downloadAnchorNode.remove(); - }); + }, [shot]);Likely an incorrect or invalid review comment.
web/src/pages/ProfileEdit/ProfileTypeSelection.jsx (1)
5-44: Well-implemented responsive layout and accessibilityThe migration to a grid-based layout with proper Card components and comprehensive ARIA attributes greatly improves both the visual consistency and accessibility of the profile type selection. The disabled state for the Pro profile is correctly implemented with appropriate ARIA attributes.
web/src/pages/ProfileEdit/StandardProfileForm.jsx (1)
53-162: Excellent form structure and accessibilityThe form implementation with proper submission handling, comprehensive ARIA attributes, and clear visual feedback (loading states, disabled states) provides an excellent user experience. The responsive grid layout and semantic HTML usage align perfectly with the Daisy UI migration goals.
| "build": { | ||
| "arduino":{ | ||
| "ldscript": "esp32s3_out.ld", | ||
| "partitions": "no_ota.csv", |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify partitions file path (no_ota.csv) exists/used as intended.
If the file is missing or located elsewhere, builds will fail. Please confirm it’s present in the repo and discoverable by PlatformIO.
Run this to verify:
🏁 Script executed:
#!/bin/bash
fd -a --glob "no_ota.csv" | sed 's|^|FOUND: |'
# Also show any partitions referenced in platformio.ini
rg -n "board_build\.partitions|partitions\s*=" -A1Length of output: 99
🏁 Script executed:
#!/bin/bash
# List all CSV files in the repository to check for the presence of no_ota.csv
fd --extension csv | sed 's|^|FOUND CSV: |'Length of output: 45
🏁 Script executed:
#!/bin/bash
# Search for no_ota.csv anywhere in the project:
find . -type f -iname "no_ota.csv" -print | sed 's|^|FOUND: |'
# Double‐check references to no_ota.csv in any text file:
rg -l "no_ota\\.csv"Length of output: 115
Missing partitions file: no_ota.csv
The board definition at boards/esp32-s3-supermini.json still references "partitions": "no_ota.csv", but no_ota.csv cannot be found anywhere in the repo. PlatformIO builds will fail unless this file is added or the path is corrected.
• boards/esp32-s3-supermini.json: line 5 – partitions → "no_ota.csv"
• No matching CSV file in project root or subdirectories
Please add the missing no_ota.csv (or update the reference to the correct filename/path).
🤖 Prompt for AI Agents
In boards/esp32-s3-supermini.json around line 5 the board references
"partitions": "no_ota.csv" but that CSV is missing from the repository; fix by
either adding the missing no_ota.csv into the repo (preferably under boards/ or
the project's root partitions/ folder) or update the JSON to point to the
existing partitions CSV (e.g., correct filename/path), and ensure the referenced
path is reachable by PlatformIO (relative to project root) so builds succeed.
| [env:display-headless-4m] | ||
| extends = env:display-headless | ||
| board = esp32-s3-supermini | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
New 4MB headless env: ensure custom board is picked up and partitions resolve.
PlatformIO should detect boards/esp32-s3-supermini.json automatically. Also confirm the board’s no_ota partitions are intentional for this env.
Run this quick check:
🏁 Script executed:
#!/bin/bash
# Confirm the custom board file exists
fd -a --glob "esp32-s3-supermini.json"
# Show all envs referencing esp32-s3-supermini
rg -n "esp32-s3-supermini" -n platformio.ini -A2
# Confirm no lingering per-env partition overrides
rg -n "board_build\.partitions" -n platformio.ini -A1Length of output: 247
We still need to confirm that the custom boards/esp32-s3-supermini.json is being picked up by PlatformIO (i.e. that platformio.ini has a board_dir pointing at boards/) before we can resolve this. Please verify that with:
If board_dir = boards (or equivalent) is present, then the custom board will be loaded automatically and no per-env partition overrides exist—this is correct. Otherwise, you’ll need to add or adjust board_dir to include your boards folder.
🏁 Script executed:
#!/bin/bash
# Check for custom board_dir setting in platformio.ini
rg -n "^\s*board_dir" -n platformio.ini -A2
# Check for any 'boards' reference indicating where custom boards live
rg -n "boards" -n platformio.ini -A2Length of output: 82
Add board_dir to include your custom boards
PlatformIO won’t pick up boards/esp32-s3-supermini.json unless you tell it where to look. I didn’t find any board_dir setting in platformio.ini, so please add one (either globally or under your env) such as:
[platformio]
board_dir = boards– This ensures esp32-s3-supermini.json is detected automatically.
– I confirmed there are no per-env board_build.partitions overrides, so your env will use the partitions defined in the JSON—please verify that its “no_ota” partition layout is correct for this 4 MB headless build.
🤖 Prompt for AI Agents
platformio.ini around lines 91-94: the env [env:display-headless-4m] extends a
custom board named esp32-s3-supermini but platformio.ini has no board_dir set so
PlatformIO will not discover boards/esp32-s3-supermini.json; add a board_dir
setting (either at the top-level [platformio] or inside the env) pointing to
your boards directory (e.g. boards) so the JSON is picked up, and then verify
the JSON’s partition table (especially the no_ota layout) matches the required
4MB headless configuration.
| return breakpoints | ||
| .filter(bp => bp.value && bp.value >= 1 && bp.value <= 12) | ||
| .map(bp => `${bp.prefix}col-span-${bp.value}`) | ||
| .join(' '); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Dynamic Tailwind class generation may not work with PurgeCSS/JIT
Tailwind CSS's PurgeCSS/JIT compiler needs to see complete class names in the source code. Dynamic class generation using template literals might result in missing styles in production builds.
Consider using a class name map or safelist configuration:
- return breakpoints
- .filter(bp => bp.value && bp.value >= 1 && bp.value <= 12)
- .map(bp => `${bp.prefix}col-span-${bp.value}`)
- .join(' ');
+ const classMap = {
+ '': ['col-span-1', 'col-span-2', 'col-span-3', 'col-span-4', 'col-span-5', 'col-span-6',
+ 'col-span-7', 'col-span-8', 'col-span-9', 'col-span-10', 'col-span-11', 'col-span-12'],
+ 'sm:': ['sm:col-span-1', 'sm:col-span-2', /* ... etc ... */],
+ 'md:': ['md:col-span-1', 'md:col-span-2', /* ... etc ... */],
+ 'lg:': ['lg:col-span-1', 'lg:col-span-2', /* ... etc ... */],
+ 'xl:': ['xl:col-span-1', 'xl:col-span-2', /* ... etc ... */],
+ };
+
+ return breakpoints
+ .filter(bp => bp.value && bp.value >= 1 && bp.value <= 12)
+ .map(bp => classMap[bp.prefix][bp.value - 1])
+ .join(' ');Alternatively, add these classes to your Tailwind safelist configuration.
🏁 Script executed:
#!/bin/bash
# Check if Tailwind config includes safelist for dynamic classes
fd -e js -e json "tailwind.config" --exec cat {} \; | grep -A 10 "safelist"Length of output: 76
🏁 Script executed:
#!/bin/bash
# Locate any Tailwind configuration files and display their contents
echo "Searching for Tailwind config files..."
fd tailwind.config.* --max-depth 1
echo "Displaying contents of discovered config files (first 200 lines)..."
for cfg in $(fd -e js -e cjs -e ts tailwind.config.* --max-depth 1); do
echo "---- $cfg ----"
sed -n '1,200p' "$cfg"
doneLength of output: 718
Ensure Dynamic col-span Classes Are Included in Production
Your project’s Tailwind config (tailwind.config.js) doesn’t define a safelist, so any classes generated only at runtime (e.g. ${bp.prefix}col-span-${bp.value}) risk being purged by JIT/PurgeCSS. You have two paths:
• Add a safelist pattern in tailwind.config.js to cover all breakpoints and values
• Or replace the dynamic template with a static lookup (class map)
Suggested changes:
- In tailwind.config.js, add a safelist entry:
module.exports = {
content: ["./web/**/*.{html,js}"],
+ safelist: [
+ {
+ pattern: /^(sm:|md:|lg:|xl:)?col-span-(?:[1-9]|1[0-2])$/
+ }
+ ],
theme: {
extend: {},
fontFamily: {
logo: ['Montserrat']
}
},
plugins: [],
}- Alternatively, in web/src/components/Card.jsx replace the runtime template with a static map:
- return breakpoints
- .filter(bp => bp.value && bp.value >= 1 && bp.value <= 12)
- .map(bp => `${bp.prefix}col-span-${bp.value}`)
- .join(' ');
+ const classMap = {
+ '': ['col-span-1','col-span-2',/*…*/,'col-span-12'],
+ 'sm:': ['sm:col-span-1','sm:col-span-2',/*…*/,'sm:col-span-12'],
+ 'md:': ['md:col-span-1','md:col-span-2',/*…*/,'md:col-span-12'],
+ 'lg:': ['lg:col-span-1','lg:col-span-2',/*…*/,'lg:col-span-12'],
+ 'xl:': ['xl:col-span-1','xl:col-span-2',/*…*/,'xl:col-span-12'],
+ };
+ return breakpoints
+ .filter(bp => bp.value >= 1 && bp.value <= 12)
+ .map(bp => classMap[bp.prefix][bp.value - 1])
+ .join(' ');Either approach will guarantee your responsive col-span-{n} classes aren’t stripped out.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In web/src/components/Card.jsx around lines 11 to 14, the runtime-generated
class string `${bp.prefix}col-span-${bp.value}` will be purged by Tailwind in
production; either add a safelist in tailwind.config.js that covers all
breakpoint prefixes and col-span values used (e.g. patterns for each prefix +
values 1–12), or replace the dynamic template with a static lookup: build a
class map that enumerates every prefix→col-span-N string and use that map to
produce the class names so Tailwind sees the classes at build time.
There was a problem hiding this comment.
It does define a safelist but in v4 it is found in the styles.css
There was a problem hiding this comment.
@ManfredRichthofen, you're absolutely right! I see now that in web/src/style.css, there's the @source inline("{,sm:,md:,lg:,xl:}col-span-{1..12}") directive, which is the Tailwind v4 way of handling safelist requirements. This ensures all the col-span classes for all breakpoints are included in the build, so the dynamic class generation in Card.jsx will work correctly in production.
Thank you for the clarification about Tailwind v4's approach!
✏️ Learnings added
Learnt from: ManfredRichthofen
PR: jniebuhr/gaggimate#347
File: web/src/components/Card.jsx:11-14
Timestamp: 2025-08-11T19:00:02.295Z
Learning: In Tailwind CSS v4, safelist patterns are defined using `source` directives in the CSS file (e.g., `source inline("{,sm:,md:,lg:,xl:}col-span-{1..12}")` in style.css) rather than in the tailwind.config.js file. This ensures that dynamically generated classes are included in the production build.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| className='input input-bordered' | ||
| type='number' | ||
| value={targetWeight} | ||
| onChange={e => onVolumetricTargetChange(parseFloat(e.target.value))} |
There was a problem hiding this comment.
Add validation for parseFloat to prevent NaN values
The parseFloat calls should validate the input to prevent NaN values from being stored in the state.
- onChange={e => onVolumetricTargetChange(parseFloat(e.target.value))}
+ onChange={e => {
+ const value = parseFloat(e.target.value);
+ if (!isNaN(value)) {
+ onVolumetricTargetChange(value);
+ }
+ }}Apply similar validation for other parseFloat calls:
- Line 385:
onFieldChange('pump', parseFloat(e.target.value)) - Lines 409, 431: pump pressure and flow values
- onChange={e => onFieldChange('pump', parseFloat(e.target.value))}
+ onChange={e => {
+ const value = parseFloat(e.target.value);
+ if (!isNaN(value)) {
+ onFieldChange('pump', value);
+ }
+ }}Also applies to: 385-385, 407-410, 429-432
🤖 Prompt for AI Agents
In web/src/pages/ProfileEdit/StandardProfileForm.jsx around lines 279, 385,
407-410 and 429-432, the parseFloat calls can produce NaN which gets stored in
state; update each handler to parse the input, check the result, and only pass a
numeric value or a safe fallback (e.g. null) to the state updater. Concretely:
compute v = parseFloat(e.target.value) (or Number(...)), then call the existing
onChange/onFieldChange with v if Number.isFinite(v) (or !Number.isNaN(v)),
otherwise pass null (or an appropriate default) so NaN is never stored. Ensure
this pattern is applied to the volumetric target, pump, pump pressure and flow
handlers mentioned.
| borderColor: ctx => skipped(ctx, 'rgb(75, 192, 192, 0.4)'), | ||
| borderDash: ctx => skipped(ctx, [6, 6]), | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix invalid CSS color: use rgba() for alpha.
The strings like 'rgb(..., 0.4)' aren’t valid in older CSS parsing. Use rgba() (or rgb with slash syntax) to ensure alpha is applied.
- borderColor: ctx => skipped(ctx, 'rgb(75, 192, 192, 0.4)'),
+ borderColor: ctx => skipped(ctx, 'rgba(75, 192, 192, 0.4)'),
- borderDash: ctx => skipped(ctx, [6, 6]),
+ borderDash: ctx => skipped(ctx, [6, 6]),- borderColor: ctx => skipped(ctx, 'rgb(255, 192, 192, 0.4)'),
+ borderColor: ctx => skipped(ctx, 'rgba(255, 192, 192, 0.4)'),
- borderDash: ctx => skipped(ctx, [6, 6]),
+ borderDash: ctx => skipped(ctx, [6, 6]),Also applies to: 23-25
🤖 Prompt for AI Agents
In web/src/pages/ProfileList/ExtendedContent.jsx around lines 11-13 (and also
lines 23-25), the CSS color strings use the invalid 'rgb(..., 0.4)' alpha
syntax; replace those with valid rgba(...) (e.g. 'rgba(75, 192, 192, 0.4)') or
the modern 'rgb(... / 0.4)' form so borderColor and any other color returns
produce a valid CSS color string, and update any similar occurrences to use rgba
or the slash syntax consistently.
| <Card sm={12} role='listitem'> | ||
| <a | ||
| href="/profiles/new" | ||
| className="flex flex-col gap-3 items-center justify-center cursor-pointer text-base-content hover:text-primary hover:bg-base-content/5 transition-colors" | ||
| 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' | ||
| aria-label='Create new profile' | ||
| role='button' | ||
| tabIndex={0} | ||
| > | ||
| <i className="fa fa-plus text-4xl" /> | ||
| <span className="text-base font-medium">Add new</span> | ||
| <i className='fa fa-plus text-4xl' aria-hidden='true' /> | ||
| <span className='text-base font-medium'>Add new profile</span> | ||
| </a> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
A11y: don’t use role='button' on anchors; remove redundant tabIndex and aria-label.
Anchors are already keyboard-focusable and correctly announced as links. role='button' misrepresents semantics and would require spacebar handling. The visible text provides the accessible name, so aria-label is redundant.
- <Card sm={12} role='listitem'>
+ <Card sm={12} role='listitem'>
<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'
- aria-label='Create new profile'
- role='button'
- tabIndex={0}
+ className='text-base-content hover:text-primary flex cursor-pointer flex-col items-center justify-center gap-2 p-4 transition-colors'
>
<i className='fa fa-plus text-4xl' aria-hidden='true' />
<span className='text-base font-medium'>Add new profile</span>
</a>
</Card>📝 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.
| <Card sm={12} role='listitem'> | |
| <a | |
| href="/profiles/new" | |
| className="flex flex-col gap-3 items-center justify-center cursor-pointer text-base-content hover:text-primary hover:bg-base-content/5 transition-colors" | |
| 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' | |
| aria-label='Create new profile' | |
| role='button' | |
| tabIndex={0} | |
| > | |
| <i className="fa fa-plus text-4xl" /> | |
| <span className="text-base font-medium">Add new</span> | |
| <i className='fa fa-plus text-4xl' aria-hidden='true' /> | |
| <span className='text-base font-medium'>Add new profile</span> | |
| </a> | |
| <Card sm={12} role='listitem'> | |
| <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' | |
| > | |
| <i className='fa fa-plus text-4xl' aria-hidden='true' /> | |
| <span className='text-base font-medium'>Add new profile</span> | |
| </a> | |
| </Card> |
🤖 Prompt for AI Agents
In web/src/pages/ProfileList/ProfileAddCard.jsx around lines 5 to 15, the anchor
incorrectly includes role='button', tabIndex={0}, and an unnecessary aria-label
which misrepresents semantics and is redundant; remove role='button', remove
tabIndex={0}, and delete the aria-label attribute so the link remains a plain,
keyboard-focusable anchor announced by its visible text; if you intended button
behavior instead, convert the element to a <button> and implement keyboard
handling, otherwise keep the href as-is and only remove those three attributes.
| const delay = Math.min( | ||
| this.baseReconnectDelay * Math.pow(2, this.reconnectAttempts), | ||
| this.maxReconnectDelay, | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Exponential backoff: add jitter to avoid thundering herd across tabs.
Adding a small random jitter improves reconnect behavior when multiple tabs reconnect simultaneously.
- const delay = Math.min(
- this.baseReconnectDelay * Math.pow(2, this.reconnectAttempts),
- this.maxReconnectDelay,
- );
+ const base = Math.min(
+ this.baseReconnectDelay * Math.pow(2, this.reconnectAttempts),
+ this.maxReconnectDelay,
+ );
+ // add 0–250ms jitter
+ const delay = base + Math.floor(Math.random() * 250);📝 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 delay = Math.min( | |
| this.baseReconnectDelay * Math.pow(2, this.reconnectAttempts), | |
| this.maxReconnectDelay, | |
| ); | |
| const base = Math.min( | |
| this.baseReconnectDelay * Math.pow(2, this.reconnectAttempts), | |
| this.maxReconnectDelay, | |
| ); | |
| // add 0–250ms jitter | |
| const delay = base + Math.floor(Math.random() * 250); |
🤖 Prompt for AI Agents
In web/src/services/ApiService.js around lines 82 to 85, the exponential backoff
delay is computed deterministically which can cause many tabs to reconnect at
the same time; add a small random jitter to the delay calculation to avoid
thundering-herd behavior by applying a jitter factor (for example multiply by 1
+ (Math.random() - 0.5) * jitterFraction or add a random value up to jitterMs),
ensure the result is still bounded by this.maxReconnectDelay and non-negative,
and use that jittered value in place of the current delay variable.
There was a problem hiding this comment.
Actionable comments posted: 7
🔭 Outside diff range comments (4)
web/src/services/ApiService.js (1)
124-130: Clear request timeout on response to avoid stray timer and duplicate off().Store timeout ID and clear it when the response arrives.
- const listenerId = this.on(returnType, response => { + const listenerId = this.on(returnType, response => { if (response.rid === rid) { // Clean up the listener this.off(returnType, listenerId); + clearTimeout(timeoutId); resolve(response); } }); @@ - setTimeout(() => { + const timeoutId = setTimeout(() => { this.off(returnType, listenerId); reject(new Error(`Request ${data.tp} timed out`)); }, 30000); // 30 second timeoutweb/src/components/OverviewChart.jsx (1)
123-124: Inconsistent initial maxTicksLimit valueLine 123 sets
maxTicksLimit: window.innerWidth < 640 ? 5 : 5(always 5), but line 180 updates it toisSmallScreen ? 5 : 10. The initial value should probably be:- maxTicksLimit: window.innerWidth < 640 ? 5 : 5, + maxTicksLimit: window.innerWidth < 640 ? 5 : 10,Also applies to: 180-180
web/src/pages/ProfileList/index.jsx (1)
304-323: Harden import: guard JSON parsing and shape.Add try/catch so malformed files don’t crash the UI; treat non-array input as single profile as you do now.
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(); - } - }; + reader.onload = async e => { + try { + const result = e.target.result; + if (typeof result === 'string') { + let profiles = JSON.parse(result); + profiles = Array.isArray(profiles) ? profiles : [profiles]; + for (const p of profiles) { + await apiService.request({ tp: 'req:profiles:save', profile: p }); + } + await loadProfiles(); + } + } catch (err) { + console.error('Failed to import profiles', err); + alert('Import failed: invalid JSON file.'); + } + }; reader.readAsText(file); } };web/src/pages/Settings/index.jsx (1)
148-152: Harden settings import: guard JSON parse.Wrap JSON.parse in try/catch to avoid crashing on invalid files.
- reader.onload = async e => { - const data = JSON.parse(e.target.result); - setFormData(data); - }; + reader.onload = async e => { + try { + const data = JSON.parse(e.target.result); + setFormData(data); + } catch (err) { + console.error('Failed to import settings', err); + alert('Import failed: invalid JSON file.'); + } + };
♻️ Duplicate comments (9)
web/src/services/ApiService.js (1)
82-85: Exponential backoff: add jitter to avoid herd effects.Re-raising prior suggestion: add small random jitter so multiple tabs don’t reconnect simultaneously.
- const delay = Math.min( - this.baseReconnectDelay * Math.pow(2, this.reconnectAttempts), - this.maxReconnectDelay, - ); + const base = Math.min( + this.baseReconnectDelay * Math.pow(2, this.reconnectAttempts), + this.maxReconnectDelay, + ); + // add 0–250ms jitter + const delay = Math.max(0, Math.min(this.maxReconnectDelay, base + Math.floor(Math.random() * 250)));web/src/pages/ProfileList/ExtendedContent.jsx (1)
11-13: Fix invalid rgb alpha syntax: use rgba(..., a) or modern rgb(... / a).Reiterating prior note: 'rgb(..., 0.4)' is invalid; switch to rgba(...) or 'rgb(... / 0.4)'.
- borderColor: ctx => skipped(ctx, 'rgb(75, 192, 192, 0.4)'), + borderColor: ctx => skipped(ctx, 'rgba(75, 192, 192, 0.4)'), @@ - borderColor: ctx => skipped(ctx, 'rgb(255, 192, 192, 0.4)'), + borderColor: ctx => skipped(ctx, 'rgba(255, 192, 192, 0.4)'),Also applies to: 23-25
web/src/components/Card.jsx (1)
11-14: Dynamic class generation is properly handled.The dynamic Tailwind class generation using template literals is correctly addressed by the
@source inline("{,sm:,md:,lg:,xl:}col-span-{1..12}");directive inweb/src/style.cssat line 153. This ensures all the dynamically generated grid classes are included in the production build.web/src/pages/ProfileList/ProfileAddCard.jsx (1)
5-15: Remove incorrect accessibility attributes on anchor element.The anchor element has several accessibility issues:
role='button'misrepresents the semantics of a linktabIndex={0}is redundant as anchors are already keyboard-focusablearia-label='Create new profile'is unnecessary when the visible text "Add new profile" provides the same informationApply this diff to fix the accessibility issues:
<Card sm={12} role='listitem'> <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' - aria-label='Create new profile' - role='button' - tabIndex={0} + className='text-base-content hover:text-primary flex cursor-pointer flex-col items-center justify-center gap-2 p-4 transition-colors' >platformio.ini (1)
91-94: Custom board directory not configured for PlatformIO.The new
[env:display-headless-4m]environment references the customesp32-s3-superminiboard, but PlatformIO won't be able to findboards/esp32-s3-supermini.jsonwithout aboard_dirconfiguration.Add the board directory configuration to the
[platformio]section:+[platformio] +board_dir = boards + [env] platform = espressif32@6.9.0This ensures PlatformIO can discover and use your custom board definitions.
web/src/pages/ProfileEdit/StandardProfileForm.jsx (4)
279-283: Guard parseFloat to avoid NaN in state (previous feedback).Add validation before updating to ensure NaN isn’t stored.
- onChange={e => onVolumetricTargetChange(parseFloat(e.target.value))} + onChange={e => { + const v = parseFloat(e.target.value); + onVolumetricTargetChange(Number.isFinite(v) ? v : 0); + }}
385-387: Guard parseFloat for pump power (previous feedback).Prevent NaN assignment.
- onChange={e => onFieldChange('pump', parseFloat(e.target.value))} + onChange={e => { + const v = parseFloat(e.target.value); + onFieldChange('pump', Number.isFinite(v) ? v : 0); + }}
407-410: Guard parseFloat for pressure (previous feedback).Ensure valid numeric value before writing.
- onChange={e => - onFieldChange('pump', { ...phase.pump, pressure: parseFloat(e.target.value) }) - } + onChange={e => { + const v = parseFloat(e.target.value); + onFieldChange('pump', { ...phase.pump, pressure: Number.isFinite(v) ? v : 0 }); + }}
429-432: Guard parseFloat for flow (previous feedback).Same rationale as pressure.
- onChange={e => - onFieldChange('pump', { ...phase.pump, flow: parseFloat(e.target.value) }) - } + onChange={e => { + const v = parseFloat(e.target.value); + onFieldChange('pump', { ...phase.pump, flow: Number.isFinite(v) ? v : 0 }); + }}
🧹 Nitpick comments (11)
web/package.json (1)
29-31: Prettier 3 + Tailwind plugin: good upgrade. Consider adding a non-mutating format check.LGTM. To enforce formatting in CI without writes, add a format check script.
Example scripts addition:
{
"scripts": {
"format": "prettier -w .",
"format:check": "prettier -c ."
}
}web/src/components/Navigation.jsx (1)
21-37: Nav width and formatting: LGTM. Add aria-label and consider nested-route active matching.
- Add aria-label on nav for a11y.
- Optional: treat nested paths (e.g., /profiles/123) as active.
- Verify lg:col-span-2 aligns with your main grid template.
Example tweaks:
- <nav className='hidden lg:col-span-2 lg:block'> + <nav aria-label='Primary navigation' className='hidden lg:col-span-2 lg:block'>And optionally pass active based on prefix:
// where you render MenuItem <MenuItem label='Profiles' link='/profiles' active={path.startsWith('/profiles')} />web/src/pages/ProfileList/ExtendedContent.jsx (1)
64-65: Remove unused variable; simplify time computation.
labelsis unused and the reduce can be simplified.- const labels = [0]; - let time = phases.map(p => p.duration).reduce((a, b) => a + b, 0); + const time = phases.reduce((sum, { duration }) => sum + duration, 0);web/src/components/Spinner.jsx (1)
1-16: Good migration to DaisyUI loading classesThe switch from SVG to CSS-based spinner simplifies the component and ensures theme consistency. The accessibility attributes are properly maintained.
Consider adding support for custom className to allow consumers to apply additional styling when needed:
-export function Spinner({ size = 8 }) { +export function Spinner({ size = 8, className = '' }) { const sizeMap = { }; return ( <span - className={`loading loading-spinner ${sizeMap[size] || 'loading-md'}`} + className={`loading loading-spinner ${sizeMap[size] || 'loading-md'} ${className}`} role='status' aria-label='Loading' /> ); }web/src/pages/Home/index.jsx (1)
22-22: Initialize from storage to avoid initial flicker (defensive SSR guard).Use a lazy initializer so the first render uses the stored layout when available, with a try/catch for environments without localStorage.
- const [dashboardLayout, setDashboardLayout] = useState('process-first'); + const [dashboardLayout, setDashboardLayout] = useState(() => { + try { + return getDashboardLayout(); + } catch { + return 'process-first'; + } + });web/src/pages/ProfileList/index.jsx (1)
85-91: Use onChange for checkbox instead of onClick.onChange is the semantic event for form controls and aligns better with assistive tech.
- <input + <input checked={data.selected} type='checkbox' - onClick={() => onSelect(data.id)} + onChange={() => onSelect(data.id)} className='checkbox checkbox-success' aria-label={`Select ${data.label} profile`} />web/src/pages/Autotune/index.jsx (1)
86-121: Optional: align variable naming with UI (“Tuning Goal”).State is named time but the label is Tuning Goal (0–100). Consider renaming local state to tuningGoal and mapping to API “time” on send for readability.
No functional change; can be done later for clarity.
web/src/pages/Settings/index.jsx (1)
424-441: Prefer controlled select for timezone (consistency with other fields).Using per-option selected is less idiomatic in React/Preact and can lead to mismatches. Control the select’s value and drop selected from options.
- <select + <select id='timezone' name='timezone' className='select select-bordered w-full' - onChange={onChange('timezone')} + value={formData.timezone || ''} + onChange={onChange('timezone')} > - {timezones.map(timezone => ( - <option key={timezone} value={timezone} selected={formData.timezone === timezone}> + {timezones.map(timezone => ( + <option key={timezone} value={timezone}> {timezone} </option> ))} </select>web/src/pages/ProfileEdit/StandardProfileForm.jsx (3)
39-50: Simplify phase removal logic.Use filter to build the new phases array more readably and efficiently.
- const onPhaseRemove = index => { - const newData = { - ...data, - phases: [], - }; - for (let i = 0; i < data.phases.length; i++) { - if (i !== index) { - newData.phases.push(data.phases[i]); - } - } - onChange(newData); - }; + const onPhaseRemove = index => { + onChange({ + ...data, + phases: data.phases.filter((_, i) => i !== index), + }); + };
1-4: Avoid importing chart.js/helpers just for isNumber.This import couples the form to Chart.js internals. Prefer a tiny local helper.
-import { isNumber } from 'chart.js/helpers'; +const isNumber = v => typeof v === 'number' && Number.isFinite(v);No functional change; reduces bundle coupling.
Also applies to: 196-200
256-263: Potential type consistency: ensure numeric state for numeric fields.Duration and Temperature currently pass string values. If downstream logic assumes numbers, consider parsing and guarding similarly to other fields.
If safe, update handlers:
- onChange={e => onFieldChange('temperature', e.target.value)} + onChange={e => { + const v = parseFloat(e.target.value); + onFieldChange('temperature', Number.isFinite(v) ? v : 0); +}}and
- onChange={e => onFieldChange('duration', e.target.value)} + onChange={e => { + const v = parseInt(e.target.value, 10); + onFieldChange('duration', Number.isFinite(v) ? v : 0); +}}Confirm server expects numeric types to avoid regressions.
Also applies to: 96-104
📜 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 (34)
boards/esp32-s3-supermini.json(1 hunks)platformio.ini(1 hunks)web/.prettierrc(1 hunks)web/index.html(1 hunks)web/package.json(1 hunks)web/src/components/Card.jsx(1 hunks)web/src/components/Footer.jsx(1 hunks)web/src/components/Header.jsx(2 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(6 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(9 hunks)web/src/pages/Scales/index.jsx(3 hunks)web/src/pages/Settings/index.jsx(6 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/_404.jsx(1 hunks)web/src/services/ApiService.js(2 hunks)web/src/style.css(6 hunks)web/src/utils/dashboardManager.js(1 hunks)web/src/utils/themeManager.js(1 hunks)web/src/utils/uuid.js(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-11T19:00:02.295Z
Learnt from: ManfredRichthofen
PR: jniebuhr/gaggimate#347
File: web/src/components/Card.jsx:11-14
Timestamp: 2025-08-11T19:00:02.295Z
Learning: In Tailwind CSS v4, safelist patterns are defined using `source` directives in the CSS file (e.g., `source inline("{,sm:,md:,lg:,xl:}col-span-{1..12}")` in style.css) rather than in the tailwind.config.js file. This ensures that dynamically generated classes are included in the production build.
Applied to files:
web/src/components/Card.jsx
🧬 Code Graph Analysis (18)
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/ProfileEdit/ProfileTypeSelection.jsx (2)
web/src/components/Card.jsx (1)
Card(1-29)web/src/pages/ProfileList/index.jsx (1)
onSelect(241-248)
web/src/components/Card.jsx (2)
web/src/components/Header.jsx (1)
className(6-7)web/src/components/Navigation.jsx (1)
className(4-5)
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/Scales/index.jsx (1)
web/src/components/Card.jsx (1)
Card(1-29)
web/src/index.jsx (1)
web/src/utils/themeManager.js (1)
initializeTheme(42-45)
web/src/pages/ProfileEdit/index.jsx (5)
web/src/pages/Home/index.jsx (1)
apiService(23-23)web/src/index.jsx (1)
apiService(23-23)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/pages/ShotHistory/index.jsx (6)
web/src/pages/ShotHistory/utils.js (1)
parseHistoryData(1-37)web/src/pages/ProfileList/index.jsx (2)
onDelete(232-239)apiService(210-210)web/src/pages/Home/index.jsx (1)
apiService(23-23)web/src/index.jsx (1)
apiService(23-23)web/src/pages/Home/ProcessControls.jsx (1)
apiService(72-72)web/src/pages/ShotHistory/HistoryCard.jsx (1)
HistoryCard(5-62)
web/src/pages/ProfileList/ProfileAddCard.jsx (1)
web/src/components/Card.jsx (1)
Card(1-29)
web/src/pages/ProfileList/index.jsx (6)
web/src/components/Card.jsx (1)
Card(1-29)web/src/pages/Home/index.jsx (1)
apiService(23-23)web/src/index.jsx (1)
apiService(23-23)web/src/pages/Settings/index.jsx (1)
onExport(134-142)web/src/pages/ShotHistory/HistoryCard.jsx (1)
onExport(7-16)web/src/components/Spinner.jsx (1)
Spinner(1-16)
web/src/pages/ProfileEdit/StandardProfileForm.jsx (3)
web/src/pages/ProfileEdit/index.jsx (4)
data(18-18)onSave(65-74)pressureAvailable(10-10)saving(16-16)web/src/components/Card.jsx (1)
Card(1-29)web/src/components/Spinner.jsx (1)
Spinner(1-16)
web/src/pages/Autotune/index.jsx (3)
web/src/components/Card.jsx (1)
Card(1-29)web/src/components/OverviewChart.jsx (1)
OverviewChart(131-203)web/src/components/Spinner.jsx (1)
Spinner(1-16)
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/pages/Home/index.jsx (2)
web/src/utils/dashboardManager.js (4)
setDashboardLayout(7-9)setDashboardLayout(7-9)getDashboardLayout(3-5)getDashboardLayout(3-5)web/src/services/ApiService.js (4)
ApiServiceContext(191-191)ApiServiceContext(191-191)machine(193-207)machine(193-207)
web/src/pages/Settings/index.jsx (6)
web/src/utils/themeManager.js (2)
getStoredTheme(4-12)handleThemeChange(48-51)web/src/utils/dashboardManager.js (2)
setDashboardLayout(7-9)setDashboardLayout(7-9)web/src/pages/ProfileList/index.jsx (1)
onExport(285-302)web/src/components/Spinner.jsx (1)
Spinner(1-16)web/src/components/Card.jsx (1)
Card(1-29)web/src/config/zones.js (2)
timezones(1-463)timezones(1-463)
web/src/pages/Home/ProcessControls.jsx (8)
web/src/pages/Autotune/index.jsx (2)
active(9-9)apiService(8-8)web/src/components/ProcessInfo.jsx (3)
processInfo(4-4)progress(23-26)target(20-21)web/src/pages/OTA/index.jsx (2)
progress(12-12)apiService(7-7)web/src/pages/Home/index.jsx (2)
apiService(23-23)mode(50-50)web/src/index.jsx (1)
apiService(23-23)web/src/pages/ProfileEdit/index.jsx (1)
apiService(13-13)web/src/pages/ShotHistory/index.jsx (1)
apiService(32-32)web/src/pages/ProfileList/index.jsx (1)
apiService(210-210)
web/src/pages/OTA/index.jsx (4)
web/src/pages/Home/index.jsx (1)
apiService(23-23)web/src/pages/Settings/index.jsx (3)
formRef(25-25)formData(17-17)onSubmit(100-132)web/src/components/Spinner.jsx (1)
Spinner(1-16)web/src/components/Card.jsx (1)
Card(1-29)
web/src/pages/ShotHistory/HistoryCard.jsx (4)
web/src/components/Card.jsx (1)
Card(1-29)web/src/pages/Settings/index.jsx (1)
onExport(134-142)web/src/pages/ProfileList/index.jsx (2)
onExport(285-302)onDelete(232-239)web/src/pages/ShotHistory/index.jsx (1)
onDelete(47-54)
🪛 Biome (2.1.2)
web/src/style.css
[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)
🔇 Additional comments (41)
web/src/pages/ShotHistory/HistoryChart.jsx (1)
83-86: Arrow callback style change: LGTM.No functional change; consistent with Prettier config.
Also applies to: 94-97
web/src/utils/uuid.js (1)
2-4: Formatting-only change: LGTM.Arrow parens removal and trailing comma align with the new Prettier rules; no functional impact.
web/.prettierrc (1)
2-16: Prettier config overhaul: LGTM. Confirm target browsers handle trailing commas in calls.
trailingComma: "all"introduces trailing commas in function calls. This is supported in modern browsers; verify your target baseline is modern enough (Vite defaults are).If wider support is required, consider
trailingComma: "es5".web/src/pages/_404.jsx (1)
3-9: Clean migration to DaisyUI stylingThe refactor to use flexbox layout and DaisyUI classes is well-executed. The theme-aware color classes (
text-base-content) and button styling (btn btn-primary) properly integrate with the new theming system.web/src/components/ProcessInfo.jsx (1)
29-36: Excellent use of native HTML progress elementThe migration from a custom div-based progress bar to the native
<progress>element with DaisyUI styling improves both semantics and accessibility. The styling updates with consistent spacing and text sizing enhance the component's visual hierarchy.web/src/components/Footer.jsx (1)
6-25: Well-executed theme migration with security improvementThe migration to DaisyUI theme tokens (
text-base-content,text-primary,text-error) ensures proper theming support. Additionally, addingrel='noreferrer'to the external link is a good security practice that prevents window.opener attacks.web/src/components/OverviewChart.jsx (2)
136-149: Excellent chart lifecycle managementThe separation of chart creation and cleanup logic prevents memory leaks and unnecessary re-instantiation. This is a significant improvement over the previous implementation.
162-196: Chart.js v4.4.8 ‘none’ update mode is supported – no changes required
- web/package.json shows
"chart.js": "^4.4.8", and Chart.js v4 accepts"none"as a valid update mode for disabling animations.- Direct mutation of
chart.optionsfollowed bychart.update('none')is an approved pattern for responsive resizes.web/src/pages/Home/ProcessControls.jsx (4)
16-16: LGTM! Arrow function parameter syntax simplification.Removing parentheses around single arrow function parameters follows modern JavaScript conventions.
23-62: Excellent DaisyUI theme token migration.The BrewProgress component has been successfully updated to use DaisyUI theme tokens (
text-base-content,text-base-content/60,bg-base-content/20,bg-primary, etc.) which ensures consistent theming across different color schemes. The responsive design improvements and layout adjustments enhance the user experience.
77-85: Clean callback refactoring.The changeTarget callback has been simplified with proper parameter syntax and the dependency array correctly includes only
apiService.
125-249: Comprehensive UI modernization with DaisyUI integration.The ProcessControls component has been thoroughly updated with:
- Consistent DaisyUI theme tokens throughout
- Improved responsive design with proper breakpoints
- Better spacing and layout using Tailwind utilities
- Enhanced accessibility with proper color contrasts
- Modern button styling with DaisyUI button classes
The functionality remains intact while providing a more polished and themeable interface.
web/src/components/Card.jsx (1)
1-29: Excellent Card component implementation.This Card component provides a clean, flexible API for responsive grid layouts with proper prop destructuring and conditional rendering. The implementation correctly:
- Filters invalid grid values (1-12 range validation)
- Generates proper Tailwind responsive prefixes
- Supports optional title, fullHeight, and role props
- Uses consistent DaisyUI styling
The component will serve as a solid foundation for the grid-based layouts across the application.
web/src/components/Header.jsx (3)
25-29: Clean arrow function parameter syntax.Good modernization of the callback parameter syntax, removing unnecessary parentheses around single parameters.
71-82: Improved SVG icon markup.The hamburger menu SVG has been properly formatted with multiline attributes and correctly quoted
fillRuleandclipRuleproperties. This improves code readability and follows HTML standards.
88-140: Comprehensive mobile navigation structure.Excellent reorganization of the mobile navigation using HeaderItem components. The menu is now well-structured with logical groupings:
- Dashboard (standalone)
- Profiles & Shot History (user content)
- PID Autotune & Bluetooth Scales & Settings (tools)
- System & Updates (admin)
The use of
<hr>elements with appropriate spacing provides good visual separation between sections.web/src/utils/themeManager.js (4)
1-12: Robust theme retrieval with validation.Excellent implementation with proper localStorage error handling and theme validation. The fallback to 'light' theme ensures the application remains functional even when localStorage is unavailable or contains invalid data.
14-26: Well-designed theme persistence function.The setStoredTheme function properly validates themes against AVAILABLE_THEMES, applies the theme immediately, and provides clear return values for success/failure states. The error handling with console.warn is appropriate for debugging.
34-39: Clean theme options generation.The getAvailableThemes function provides a nice abstraction for UI components, generating properly formatted options with capitalized labels. This makes it easy to populate theme selection dropdowns.
42-51: Complete theme management API.The initialization and event handler functions complete the theme management API nicely. initializeTheme() ensures themes are applied on app startup, while handleThemeChange() provides a convenient event handler for UI components.
web/src/pages/Scales/index.jsx (4)
61-61: Clean arrow function parameter syntax.Good modernization removing unnecessary parentheses around single parameters.
77-87: Excellent header layout with loading state.The new header layout with title and scan button is well-designed. The loading state handling on the scan button with proper disabled state and visual feedback enhances the user experience.
89-139: Comprehensive layout modernization.Outstanding refactoring to a Card-based responsive grid layout. Key improvements:
- Consistent Card component usage with proper grid sizing
- Enhanced loading states with spinner and descriptive text
- Better error handling with alert styling
- Improved empty state with emoji and helpful messaging
- Scale items now use clean bordered containers with proper spacing and typography
- DaisyUI theme tokens ensure consistent theming
The layout is now more maintainable and visually cohesive.
142-150: Improved informational messaging.The scales refresh note has been properly styled with DaisyUI alert components and positioned appropriately within the grid layout.
web/src/style.css (5)
5-5: Well-documented theme updates.Excellent addition of theme generator comments with URLs for easy theme modification and documentation. This makes it much easier for developers to understand and modify the themes in the future.
Also applies to: 41-41, 77-77, 113-113
27-27: Light theme warning color adjustment.The warning color has been updated from a more muted tone to a brighter, more accessible yellow. This improves visibility and contrast for warning messages.
47-66: Comprehensive theme token rebaselining.Both dark and nord themes have been extensively updated with new color tokens. The oklch color space values ensure better color consistency and accessibility across different displays. The systematic approach to updating all base, primary, secondary, accent, neutral, info, success, warning, and error tokens creates cohesive theme palettes.
Also applies to: 119-138
153-153: Smart solution for dynamic grid classes.The
@source inlinedirective ensures that all responsive col-span classes (from col-span-1 to col-span-12 across all breakpoints) are included in the production build. This addresses the dynamic class generation in the Card component and aligns with Tailwind CSS v4 best practices.
43-45: Static analysis false positives on DaisyUI theme properties.The static analysis tool is flagging
name,default, andprefersdarkas unknown properties, but these are valid DaisyUI theme configuration properties that are processed by the DaisyUI plugin. These properties are correctly defined according to DaisyUI's theme system.platformio.ini (1)
87-90: New headless environments well structured.The new headless environments properly extend the base
env:display-headlessconfiguration, maintaining clean inheritance and avoiding duplication. The 8MB variant usingseeed_xiao_esp32s3should work correctly as it's a standard PlatformIO board.web/src/index.jsx (1)
63-64: Good practice: Theme initialization before render.Calling
initializeTheme()before rendering prevents flash of unstyled content and ensures the correct theme is applied immediately.web/src/pages/ProfileEdit/index.jsx (1)
65-74: Consistent routing behavior implemented.The
onSavecallback now consistently routes to '/profiles' after saving, providing a uniform user experience regardless of whether creating or editing profiles. The dependency array correctly includes all used variables.web/src/pages/ShotHistory/index.jsx (2)
37-37: Good: Reversed history order for better UX.Displaying history in reverse order (newest first) provides better user experience by showing the most recent shots at the top.
41-45: Fixed: Proper async handling in useEffect.Good fix - calling the async function
loadHistory()directly instead of making the useEffect callback async, which is an anti-pattern.web/src/pages/ShotHistory/HistoryCard.jsx (1)
25-42: Excellent accessibility improvements.Converting from anchor tags to proper button elements with ARIA labels and tooltips significantly improves accessibility and user experience. The semantic button elements are more appropriate for these actions.
web/src/pages/OTA/index.jsx (1)
98-149: Good UX improvement: Added profile backup warning.The warning message about backing up profiles before updating the display is a valuable addition that helps prevent data loss. The Card-based layout also improves the visual organization of system information.
web/src/pages/ProfileEdit/ProfileTypeSelection.jsx (2)
10-18: Excellent keyboard accessibility implementation.Adding
onKeyDownsupport for Enter and Space keys, along with properrole,tabIndex, andaria-labelattributes, makes the component fully keyboard accessible and screen reader friendly.
30-34: Proper disabled state implementation.Using
aria-disabled='true'and appropriate visual styling (cursor-not-allowed, reduced opacity) correctly indicates the disabled state while maintaining semantic meaning.web/src/pages/Home/index.jsx (1)
58-80: Layout composition reads well and matches Card API.Grid spans sum to lg:10 and use Card fullHeight appropriately with OverviewChart’s flexible wrapper. Looks good.
web/src/pages/ProfileList/index.jsx (1)
21-30: Resolved: Chart.register moved to module scope.One-time registration now happens at load, avoiding re-registration on re-renders. Good.
web/src/pages/Autotune/index.jsx (1)
23-31: Good event lifecycle handling.Listener is registered once and removed on unmount; result state is set and active reset. Looks solid.
| <title>Gaggimate Web UI</title> | ||
| </head> | ||
| <body class="bg-slate-100 dark:bg-slate-900"> | ||
| <body class="bg-base-100"> |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Align with DaisyUI theming and avoid FOUC: set data-theme early.
With runtime theme init, users may briefly see the wrong background. Set data-theme in the head before paint.
Minimal inline in :
<script> try { const theme = localStorage.getItem('theme') || 'light'; document.documentElement.setAttribute('data-theme', theme); } catch (e) {} </script>🏁 Script executed:
#!/bin/bash
# Search for DaisyUI references
rg "daisyui" -n .
# Inspect index.html for body, head, and data-theme usage
rg "<head" -n web/index.html
rg "<body" -n web/index.html
rg "data-theme" -n web/index.html
# Locate and preview Tailwind/DaisyUI configuration
fd tailwind.config.js | xargs -I {} sed -n '1,200p' {}Length of output: 4083
🏁 Script executed:
#!/bin/bash
# Preview the themeManager.js implementation to understand runtime theme init logic
sed -n '1,200p' web/src/utils/themeManager.jsLength of output: 1364
Set data-theme in <head> to prevent FOUC
Insert a small inline script at the very top of your web/index.html <head> so the stored theme is applied before any CSS is painted:
--- web/index.html
+++ web/index.html
@@ -1,4 +1,12 @@
<head>
+ <script>
+ (function() {
+ try {
+ // Use the same storage key as themeManager.js
+ var theme = localStorage.getItem('gaggimate-daisyui-theme') || 'light';
+ document.documentElement.setAttribute('data-theme', theme);
+ } catch (e) {}
+ })();
+ </script>
<!-- your existing meta tags, CSS links, etc. -->
</head>• File to update: web/index.html
• Insert the snippet before any <link> or <style> so the correct CSS variables load on first paint.
This guarantees your DaisyUI theme (light, dark, coffee, nord) is applied immediately and avoids a flash of the default background.
📝 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.
| <body class="bg-base-100"> | |
| <head> | |
| <script> | |
| (function() { | |
| try { | |
| // Use the same storage key as themeManager.js | |
| var theme = localStorage.getItem('gaggimate-daisyui-theme') || 'light'; | |
| document.documentElement.setAttribute('data-theme', theme); | |
| } catch (e) {} | |
| })(); | |
| </script> | |
| <!-- your existing meta tags, CSS links, etc. --> | |
| </head> |
🤖 Prompt for AI Agents
In web/index.html around line 26 (the <body> line) the head currently lacks an
early theme setter which causes a flash of unthemed content; add a small inline
script at the very top of the <head> (before any <link> or <style>) that reads
the stored theme (e.g., from localStorage) or falls back to a system preference
and sets the document root's data-theme attribute (documentElement) immediately
so DaisyUI's CSS variables are applied on first paint; ensure the snippet is
inline (no external fetch) and minimal so it runs before styles load.
| useEffect(() => { | ||
| setDashboardLayout(getDashboardLayout()); | ||
|
|
||
| const handleStorageChange = (e) => { | ||
| if (e.key === 'dashboardLayout') { | ||
| setDashboardLayout(e.newValue || 'process-first'); | ||
| } | ||
| }; | ||
|
|
||
| window.addEventListener('storage', handleStorageChange); | ||
|
|
||
| return () => { | ||
| window.removeEventListener('storage', handleStorageChange); | ||
| }; | ||
| }, []); |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Same-tab layout changes won’t be reflected (storage event doesn’t fire on the same document).
Settings writes localStorage in the same tab; the storage event won’t fire here, so Home won’t update until remount. Add a lightweight CustomEvent listener and dispatch it in setDashboardLayout.
Apply in this file:
useEffect(() => {
setDashboardLayout(getDashboardLayout());
- const handleStorageChange = (e) => {
+ const handleStorageChange = e => {
if (e.key === 'dashboardLayout') {
setDashboardLayout(e.newValue || 'process-first');
}
};
+ const handleCustomLayoutChange = e => {
+ // Prefer explicit detail when provided; otherwise read from storage
+ if (typeof e.detail === 'string') {
+ setDashboardLayout(e.detail || 'process-first');
+ } else {
+ try {
+ setDashboardLayout(getDashboardLayout());
+ } catch {
+ setDashboardLayout('process-first');
+ }
+ }
+ };
window.addEventListener('storage', handleStorageChange);
+ window.addEventListener('dashboardLayoutChange', handleCustomLayoutChange);
return () => {
window.removeEventListener('storage', handleStorageChange);
+ window.removeEventListener('dashboardLayoutChange', handleCustomLayoutChange);
};
}, []);And update the setter in web/src/utils/dashboardManager.js:
// Also dispatch a same-document event so open pages update immediately.
export const setDashboardLayout = layout => {
try {
localStorage.setItem(DASHBOARD_LAYOUT_KEY, layout);
window.dispatchEvent(new CustomEvent('dashboardLayoutChange', { detail: layout }));
} catch (e) {
// no-op: storage not available
}
};Add CustomEvent listener for same-tab layout updates
The native storage event only fires in other tabs, so updating localStorage in the same document won’t trigger your listener. Introduce a lightweight dashboardLayoutChange CustomEvent to cover same-tab updates:
Locations to update:
- web/src/pages/Home/index.jsx: extend your
useEffectto listen fordashboardLayoutChangeand clean it up on unmount. - web/src/utils/dashboardManager.js: after
localStorage.setItem, dispatchwindow.dispatchEvent(new CustomEvent('dashboardLayoutChange', { detail: layout })).
Applied diff:
--- a/web/src/pages/Home/index.jsx
+++ b/web/src/pages/Home/index.jsx
@@ useEffect(() => {
setDashboardLayout(getDashboardLayout());
const handleStorageChange = e => {
if (e.key === 'dashboardLayout') {
setDashboardLayout(e.newValue || 'process-first');
}
};
+ const handleCustomLayoutChange = e => {
+ if (typeof e.detail === 'string') {
+ setDashboardLayout(e.detail || 'process-first');
+ } else {
+ setDashboardLayout(getDashboardLayout());
+ }
+ };
window.addEventListener('storage', handleStorageChange);
+ window.addEventListener('dashboardLayoutChange', handleCustomLayoutChange);
return () => {
window.removeEventListener('storage', handleStorageChange);
+ window.removeEventListener('dashboardLayoutChange', handleCustomLayoutChange);
};
}, []);--- a/web/src/utils/dashboardManager.js
+++ b/web/src/utils/dashboardManager.js
export const setDashboardLayout = layout => {
try {
localStorage.setItem(DASHBOARD_LAYOUT_KEY, layout);
- } catch (e) {
+ // Notify same-tab subscribers of the change
+ window.dispatchEvent(
+ new CustomEvent('dashboardLayoutChange', { detail: layout })
+ );
+ } catch (e) {
// no-op: storage not available
}
};📝 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.
| useEffect(() => { | |
| setDashboardLayout(getDashboardLayout()); | |
| const handleStorageChange = (e) => { | |
| if (e.key === 'dashboardLayout') { | |
| setDashboardLayout(e.newValue || 'process-first'); | |
| } | |
| }; | |
| window.addEventListener('storage', handleStorageChange); | |
| return () => { | |
| window.removeEventListener('storage', handleStorageChange); | |
| }; | |
| }, []); | |
| // File: web/src/pages/Home/index.jsx | |
| useEffect(() => { | |
| setDashboardLayout(getDashboardLayout()); | |
| const handleStorageChange = (e) => { | |
| if (e.key === 'dashboardLayout') { | |
| setDashboardLayout(e.newValue || 'process-first'); | |
| } | |
| }; | |
| const handleCustomLayoutChange = (e) => { | |
| if (typeof e.detail === 'string') { | |
| setDashboardLayout(e.detail || 'process-first'); | |
| } else { | |
| setDashboardLayout(getDashboardLayout()); | |
| } | |
| }; | |
| window.addEventListener('storage', handleStorageChange); | |
| window.addEventListener('dashboardLayoutChange', handleCustomLayoutChange); | |
| return () => { | |
| window.removeEventListener('storage', handleStorageChange); | |
| window.removeEventListener('dashboardLayoutChange', handleCustomLayoutChange); | |
| }; | |
| }, []); |
| useEffect(() => { | |
| setDashboardLayout(getDashboardLayout()); | |
| const handleStorageChange = (e) => { | |
| if (e.key === 'dashboardLayout') { | |
| setDashboardLayout(e.newValue || 'process-first'); | |
| } | |
| }; | |
| window.addEventListener('storage', handleStorageChange); | |
| return () => { | |
| window.removeEventListener('storage', handleStorageChange); | |
| }; | |
| }, []); | |
| // File: web/src/utils/dashboardManager.js | |
| export const setDashboardLayout = layout => { | |
| try { | |
| localStorage.setItem(DASHBOARD_LAYOUT_KEY, layout); | |
| // Notify same-tab subscribers of the change | |
| window.dispatchEvent( | |
| new CustomEvent('dashboardLayoutChange', { detail: layout }) | |
| ); | |
| } catch (e) { | |
| // no-op: storage not available | |
| } | |
| }; |
🤖 Prompt for AI Agents
In web/src/pages/Home/index.jsx around lines 25 to 39, the current useEffect
only listens for the storage event which does not fire for same-tab localStorage
updates; add a listener for a new 'dashboardLayoutChange' CustomEvent that reads
the layout from event.detail (or falls back to localStorage) and calls
setDashboardLayout, and remove that listener in the cleanup alongside the
existing storage listener. Also update web/src/utils/dashboardManager.js so that
after calling localStorage.setItem('dashboardLayout', layout) it dispatches
window.dispatchEvent(new CustomEvent('dashboardLayoutChange', { detail: layout
})) so same-tab changes trigger the Home listener.
| <div className='flex-grow'> | ||
| <canvas className='max-h-36 w-full' ref={ref} /> | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Destroy Chart.js instance on unmount to prevent leaks.
Add cleanup in the effect that creates the chart.
Example (outside these exact lines for context):
useEffect(() => {
const ct = new Chart(ref.current, config);
- }, [ref]);
+ return () => ct.destroy();
+ }, [ref]);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In web/src/pages/ProfileList/ExtendedContent.jsx around lines 108 to 110, the
Chart.js canvas is created but the chart instance is not destroyed on unmount;
update the effect that creates the chart to store the returned Chart instance in
a local variable (or ref) and return a cleanup function that calls
chart.destroy() (and nulls the reference) so the Chart.js instance is properly
disposed to prevent memory leaks.
| const loadProfiles = async () => { | ||
| const response = await apiService.request({ tp: 'req:profiles:list' }); | ||
| setProfiles(response.profiles); | ||
| setLoading(false); | ||
| }; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Memoize loadProfiles and update dependent hooks to avoid stale closures.
loadProfiles is recreated every render but not listed as a dependency in callbacks using it. Memoize it and include it in deps for onDelete/onSelect/... to adhere to hooks best practices.
- const loadProfiles = async () => {
+ const loadProfiles = useCallback(async () => {
const response = await apiService.request({ tp: 'req:profiles:list' });
setProfiles(response.profiles);
setLoading(false);
- };
+ }, [apiService]);Then update callback deps (examples shown below):
- const onDelete = useCallback(
- async id => {
+ const onDelete = useCallback(
+ async id => {
setLoading(true);
await apiService.request({ tp: 'req:profiles:delete', id });
await loadProfiles();
},
- [apiService, setLoading],
+ [apiService, loadProfiles, setLoading],
);
- const onSelect = useCallback(
+ const onSelect = useCallback(
async id => {
setLoading(true);
await apiService.request({ tp: 'req:profiles:select', id });
await loadProfiles();
},
- [apiService, setLoading],
+ [apiService, loadProfiles, setLoading],
);Apply the same pattern for onFavorite, onUnfavorite, and onDuplicate.
Additionally, include loadProfiles in the effect when connected.value changes if you want to quiet exhaustive-deps linters.
📝 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 loadProfiles = async () => { | |
| const response = await apiService.request({ tp: 'req:profiles:list' }); | |
| setProfiles(response.profiles); | |
| setLoading(false); | |
| }; | |
| // Memoize loadProfiles to avoid recreating it on every render | |
| const loadProfiles = useCallback(async () => { | |
| const response = await apiService.request({ tp: 'req:profiles:list' }); | |
| setProfiles(response.profiles); | |
| setLoading(false); | |
| }, [apiService]); | |
| // Update dependent hooks to include loadProfiles in their deps | |
| const onDelete = useCallback( | |
| async id => { | |
| setLoading(true); | |
| await apiService.request({ tp: 'req:profiles:delete', id }); | |
| await loadProfiles(); | |
| }, | |
| [apiService, loadProfiles, setLoading], | |
| ); | |
| const onSelect = useCallback( | |
| async id => { | |
| setLoading(true); | |
| await apiService.request({ tp: 'req:profiles:select', id }); | |
| await loadProfiles(); | |
| }, | |
| [apiService, loadProfiles, setLoading], | |
| ); | |
| // TODO: apply the same pattern for onFavorite, onUnfavorite, and onDuplicate |
🤖 Prompt for AI Agents
In web/src/pages/ProfileList/index.jsx around lines 217 to 222, loadProfiles is
recreated every render and used by callbacks without being included in their
dependency arrays; wrap loadProfiles in useCallback (so it is stable) and update
onDelete, onSelect, onFavorite, onUnfavorite, onDuplicate (and any other
callbacks that call it) to include loadProfiles in their deps, and also include
loadProfiles in the useEffect that watches connected.value (or whichever effect
triggers reloading) so exhaustive-deps is satisfied.
| <label htmlFor='webui-theme' className='label'> | ||
| <span className='label-text font-medium'>Theme</span> | ||
| </label> | ||
| <select | ||
| id='daisyui-theme' | ||
| name='daisyui-theme' | ||
| className='select select-bordered w-full' | ||
| value={currentTheme} | ||
| onChange={e => { | ||
| setCurrentTheme(e.target.value); | ||
| handleThemeChange(e); | ||
| }} |
There was a problem hiding this comment.
Fix label/field association: htmlFor/id mismatch.
The label points to 'webui-theme' but the select id is 'daisyui-theme', breaking accessibility.
- <label htmlFor='webui-theme' className='label'>
+ <label htmlFor='daisyui-theme' className='label'>
<span className='label-text font-medium'>Theme</span>
</label>
<select
- id='daisyui-theme'
+ id='daisyui-theme'
name='daisyui-theme'
className='select select-bordered w-full'
value={currentTheme}
onChange={e => {
setCurrentTheme(e.target.value);
handleThemeChange(e);
}}
>📝 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.
| <label htmlFor='webui-theme' className='label'> | |
| <span className='label-text font-medium'>Theme</span> | |
| </label> | |
| <select | |
| id='daisyui-theme' | |
| name='daisyui-theme' | |
| className='select select-bordered w-full' | |
| value={currentTheme} | |
| onChange={e => { | |
| setCurrentTheme(e.target.value); | |
| handleThemeChange(e); | |
| }} | |
| <label htmlFor='daisyui-theme' className='label'> | |
| <span className='label-text font-medium'>Theme</span> | |
| </label> | |
| <select | |
| id='daisyui-theme' | |
| name='daisyui-theme' | |
| className='select select-bordered w-full' | |
| value={currentTheme} | |
| onChange={e => { | |
| setCurrentTheme(e.target.value); | |
| handleThemeChange(e); | |
| }} | |
| > |
🤖 Prompt for AI Agents
In web/src/pages/Settings/index.jsx around lines 334 to 345, the label's htmlFor
('webui-theme') does not match the select's id ('daisyui-theme'), breaking
accessibility; update one of them so they match (e.g., change the select id to
'webui-theme' or change the label htmlFor to 'daisyui-theme') and ensure the
name attribute remains correct and any related code references the chosen id.
| id='dashboardLayout' | ||
| name='dashboardLayout' | ||
| className='select select-bordered w-full' | ||
| value={formData.dashboardLayout || 'process-first'} | ||
| onChange={e => { | ||
| const value = e.target.value; | ||
| setFormData({ | ||
| ...formData, | ||
| dashboardLayout: value, | ||
| }); | ||
| setDashboardLayout(value); | ||
| }} | ||
| > |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Same-tab dashboard layout sync (follow-up).
Calling setDashboardLayout writes localStorage. To reflect changes immediately on Home within the same tab, dispatch a CustomEvent in setDashboardLayout (see Home review) and listen to it in Home.
No changes needed here after updating dashboardManager and Home as suggested.
🏁 Script executed:
#!/bin/bash
# Search for definition and usage of setDashboardLayout
rg -n "setDashboardLayout" -A 3
# Search for CustomEvent dispatch in any file
rg -n "new CustomEvent" -n
# Search in Home component for any event listeners related to dashboardLayout or storage
rg -n "addEventListener" -A 5Length of output: 3424
Same-tab dashboard layout sync
The Settings page change looks correct—no edits needed here. To ensure the Home page picks up layout changes in the same tab, implement the following in your other modules:
-
web/src/utils/dashboardManager.js
After writing tolocalStorage, dispatch a custom event:export const setDashboardLayout = (layout) => { localStorage.setItem(DASHBOARD_LAYOUT_KEY, layout); window.dispatchEvent( new CustomEvent('dashboardLayoutChange', { detail: layout }) ); };
-
web/src/pages/Home/index.jsx
In the existinguseEffect, add a listener for this event alongside the storage listener:useEffect(() => { setDashboardLayout(getDashboardLayout()); const handleStorageChange = (e) => { if (e.key === 'dashboardLayout') { setDashboardLayout(e.newValue || 'process-first'); } }; const handleCustomEvent = (e) => { setDashboardLayout(e.detail || 'process-first'); }; window.addEventListener('storage', handleStorageChange); window.addEventListener('dashboardLayoutChange', handleCustomEvent); return () => { window.removeEventListener('storage', handleStorageChange); window.removeEventListener('dashboardLayoutChange', handleCustomEvent); }; }, []);
No further changes are required in web/src/pages/Settings/index.jsx after these updates.
🤖 Prompt for AI Agents
In web/src/pages/Settings/index.jsx around lines 359-371, the Settings page
correctly updates localStorage but other tabs won't reflect the change in the
same tab; to fix this, update web/src/utils/dashboardManager.js to dispatch a
CustomEvent named 'dashboardLayoutChange' (with the layout in detail)
immediately after writing to localStorage, and update
web/src/pages/Home/index.jsx by adding a listener in the existing useEffect that
listens for 'dashboardLayoutChange' and calls setDashboardLayout with the event
detail (and still keep the existing storage event listener); ensure both
listeners are removed in the cleanup return of the effect.
| }, [shot, chart]); | ||
|
|
||
| return <canvas className="w-full" ref={ref} />; | ||
| return <canvas className='w-full' ref={ref} />; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Clean up Chart.js instance on unmount; remove debug log and dead code.
- Add effect cleanup to destroy the chart to prevent leaks.
- Remove console.log(shot) (Line 110).
- Remove unused variable
start(Line 5).
Diff within the existing effect to add cleanup (outside this exact line, shown for clarity):
useEffect(() => {
- const ct = new Chart(ref.current, chartData);
- setChart(ct);
-}, [ref]);
+ const ct = new Chart(ref.current, chartData);
+ setChart(ct);
+ return () => ct.destroy();
+}, [ref]);Remove debug log and dead code:
- console.log(shot);
+ // console.log(shot);- let start = 0;
+ // unusedCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In web/src/pages/ShotHistory/HistoryChart.jsx around line 128, the Chart.js
instance created for the canvas isn't destroyed on unmount (risking memory
leaks) and there is leftover debug/dead code; add a cleanup to the effect that
calls chart.destroy() (or equivalent) when the component unmounts or when
dependencies change, ensure you keep a reference to the chart instance so it can
be destroyed, remove the console.log(shot) at line 110, and delete the unused
variable start at line 5 along with any other unreachable or unused code paths.
|
* Add Daisy Ui (#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 (#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>
* 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
Complete Migration to Daisy Ui
Summary by CodeRabbit
New Features
UI / New Layouts
Style
Chores