162 lines
13 KiB
Markdown
162 lines
13 KiB
Markdown
# Domain Pitfalls
|
|
|
|
**Domain:** Kimai heatmap plugin v1.1 -- visualization modes, TomSelect entity pickers, cascading filters, display toggle
|
|
**Researched:** 2026-04-08
|
|
**Confidence:** HIGH (based on direct source analysis of existing plugin code and Kimai's KimaiFormSelect internals)
|
|
|
|
## Critical Pitfalls
|
|
|
|
Mistakes that cause rewrites or major issues.
|
|
|
|
### Pitfall 1: KimaiFormSelect Depends on Kimai's Plugin Container -- Cannot Use It Standalone
|
|
|
|
**What goes wrong:** SEED-001 suggests using Kimai's `KimaiFormSelect.js` with `data-api-url`, `data-related-select`, and `data-empty-url` attributes to get cascading for free. But `KimaiFormSelect._activateApiSelects()` calls `this.getContainer().getPlugin('api')` to make API requests (line 433-435). This requires the select to be inside a form that Kimai's JS plugin system initialized via `activateForm()`. The heatmap widget is a dashboard card, not a Symfony form.
|
|
|
|
**Why it happens:** Kimai's form system (`KimaiFormSelect`, `KimaiFormPlugin`) is tightly coupled to Kimai's JS plugin container and form lifecycle. The cascading logic in `_activateApiSelects` registers a delegated `change` event listener on `document`, but the handler calls `this.getContainer().getPlugin('api')` which requires the KimaiFormSelect instance to have been created by Kimai's app initialization. Our widget's IIFE bundle runs independently.
|
|
|
|
**Consequences:** Adding `data-api-url` and `data-related-select` attributes to `<select>` elements in the widget card will silently fail. No errors, just dead selects that do not cascade.
|
|
|
|
**Prevention:** Roll your own cascade with plain `fetch()` calls to Kimai's API routes or (better) custom controller endpoints. The widget is already fetch-driven. Three more fetch-based selects is consistent with the existing pattern. Use plain `<select>` elements styled with Tabler CSS. Add TomSelect only if the UX demands it.
|
|
|
|
**Detection:** Change the customer picker and observe whether the project picker updates. If it does not, you hit this.
|
|
|
|
### Pitfall 2: Destroying and Recreating SVG Without Cleaning Up Tooltips
|
|
|
|
**What goes wrong:** The current `renderHeatmap()` starts with `container.innerHTML = ''` (line 184) but tooltips are appended to `document.body` (line 267). When switching visualization modes (year -> week -> day), each render creates a new tooltip. The existing cleanup at line 263 (`document.querySelectorAll('.heatmap-tooltip').forEach(el => el.remove())`) only runs inside `renderHeatmap` -- if new modes have their own render functions without this cleanup, tooltips accumulate.
|
|
|
|
**Why it happens:** The tooltip is necessarily outside the SVG container (fixed positioning to escape overflow clipping). Each render function must know to clean up the shared tooltip.
|
|
|
|
**Consequences:** Multiple `.heatmap-tooltip` divs on the page. Stale tooltip positioning. Memory leaks on repeated mode switches.
|
|
|
|
**Prevention:** Extract tooltip into a shared module that all visualization modes import. One tooltip div, created on first use, reused across mode switches. A single `cleanup()` function that any render path calls before rebuilding.
|
|
|
|
**Detection:** Switch modes 5+ times, inspect `document.body` children for orphaned `.heatmap-tooltip` elements.
|
|
|
|
### Pitfall 3: Hour-Level Aggregation Query Performance
|
|
|
|
**What goes wrong:** Day-mode and combined day/hour views need time-of-day data. The current `getDailyAggregation()` groups by `DATE(t.date)` which benefits from Kimai's date index. An hour-level query (`HOUR(t.begin)` or `EXTRACT(HOUR FROM t.begin)`) applies a function to the column, defeating index usage. GROUP BY HOUR over a full year of data scans every row for that user.
|
|
|
|
**Why it happens:** MySQL/MariaDB cannot use an index when a function wraps the indexed column.
|
|
|
|
**Consequences:** Slow queries for users with thousands of entries. For personal use (likely <10K entries) it is noticeable latency (~200-500ms), not catastrophic. But it makes mode switching feel sluggish.
|
|
|
|
**Prevention:**
|
|
1. Limit hour-level queries to a narrow date range (one week or one month, not a full year). The UI for day-mode should show a specific date range anyway.
|
|
2. For the combined matrix, fetch raw timesheet entries for the selected range and aggregate in PHP rather than doing a complex GROUP BY.
|
|
3. Profile with `EXPLAIN` on the actual query against the dev database.
|
|
|
|
**Detection:** Mode switch to day-view takes noticeably longer than year-view load.
|
|
|
|
## Moderate Pitfalls
|
|
|
|
### Pitfall 4: Mode State Lost on Filter Change
|
|
|
|
**What goes wrong:** User switches to week-mode, then changes the project filter. The filter's fetch callback re-renders the visualization, but defaults back to year-mode because the mode selection is not part of the render state. The current `doRender()` function (line 339-344) always calls `renderHeatmap()` -- the year view.
|
|
|
|
**Why it happens:** The current architecture has no shared state object. Mode and filter are independent UI controls that both trigger re-renders, but neither knows about the other's state.
|
|
|
|
**Consequences:** User frustration. Every filter change resets the view mode. Feels broken.
|
|
|
|
**Prevention:** Introduce a state object BEFORE adding the first new mode:
|
|
```typescript
|
|
interface WidgetState {
|
|
mode: 'year' | 'week' | 'day' | 'combined';
|
|
metric: 'hours' | 'count';
|
|
filters: { customerId: number | null; projectId: number | null; activityId: number | null };
|
|
data: HeatmapData | null;
|
|
}
|
|
```
|
|
All UI actions (mode switch, filter change, metric toggle) update state, then call a single `render(state)` dispatcher that picks the right visualization function.
|
|
|
|
**Phase warning:** This refactor MUST happen before adding any new visualization mode. Retrofitting state management after multiple modes exist is painful and error-prone.
|
|
|
|
### Pitfall 5: Cascading API Response Format Assumptions
|
|
|
|
**What goes wrong:** Kimai's `/api/projects?customer=X` returns objects with `parentTitle` (customer name), `id`, `name`, and more. Activities can be "global" (no project parent). If you build your own cascade, you must handle this format correctly -- or your own simplified format if you use custom controller endpoints.
|
|
|
|
**Why it happens:** Kimai's API response shape is designed for `KimaiFormSelect._updateSelect()` which groups by `parentTitle` into optgroups. Rolling your own cascade means either parsing the same format or defining your own.
|
|
|
|
**Prevention:** Use custom controller endpoints (like the existing `getUserProjects()`) that return a simple `[{id, name}]` format. This avoids parsing Kimai's complex API response structure with optgroups and `parentTitle`. Handle edge cases:
|
|
- Customer with no projects -> empty project list
|
|
- Project with no activities -> empty activity list
|
|
- Global activities (no project association) -> include them when no project filter is set
|
|
|
|
### Pitfall 6: Color Scale Domain Mismatch Across Modes
|
|
|
|
**What goes wrong:** Year-mode uses `max(data.days, d => d.hours)` as the scale ceiling (typically 8-12). Week-mode aggregates by day-of-week over a year (totals could be 200+). Day-mode shows hour slots (0-3 range). Reusing one color scale makes week-mode cells all dark and day-mode cells all light.
|
|
|
|
**Why it happens:** Each mode has fundamentally different value ranges. Easy to forget when the scale is set up once in shared code.
|
|
|
|
**Prevention:** Each mode computes its own color scale domain from its own data. Extract scale creation into a factory function that accepts a data array. Never share a global scale instance.
|
|
|
|
### Pitfall 7: TomSelect Bundle Size Duplication
|
|
|
|
**What goes wrong:** Importing `tom-select` in `heatmap.ts` and bundling with esbuild ships a second copy (~50KB min) alongside Kimai's own TomSelect that Webpack Encore already loaded.
|
|
|
|
**Why it happens:** The plugin ships a standalone IIFE bundle independent of Kimai's module system. No shared module resolution between them.
|
|
|
|
**Prevention:** Do NOT bundle TomSelect. Options:
|
|
1. **Best for personal use:** Plain `<select>` elements with Tabler CSS. For <50 projects, native browser select works fine. No TomSelect needed.
|
|
2. **If TomSelect is needed:** Check if `window.TomSelect` exists at runtime (Kimai may expose it globally). Mark as external in esbuild config.
|
|
3. **Fallback:** Accept the duplication if autocomplete search is essential and Kimai does not expose a global.
|
|
|
|
**Recommendation:** Start with plain selects. Add TomSelect only if the list sizes demand it.
|
|
|
|
### Pitfall 8: Kimai API Auth from Widget Context
|
|
|
|
**What goes wrong:** If the cascade pickers call Kimai's standard API routes (`/api/customers`, `/api/projects`), those routes may require API token authentication depending on Kimai configuration.
|
|
|
|
**Why it happens:** Dashboard widgets run in authenticated session context, so session cookies are sent. Kimai's API accepts session auth for browser requests. But some Kimai installations restrict API access to token-only.
|
|
|
|
**Prevention:** Add thin controller endpoints in the plugin itself (`/heatmap/customers`, `/heatmap/projects`, `/heatmap/activities`) that call Kimai's repositories directly, exactly like the existing `getUserProjects()` method. This avoids API auth concerns entirely and lets you shape the response format.
|
|
|
|
**Detection:** 401 responses when cascade pickers try to fetch data. Test by calling `/api/customers` from browser console on the dashboard page.
|
|
|
|
## Minor Pitfalls
|
|
|
|
### Pitfall 9: ResizeObserver vs Window Resize
|
|
|
|
**What goes wrong:** Current code uses `window.addEventListener('resize')`. Different modes have different natural widths (week-mode is much narrower than year-mode). Kimai's sidebar toggle changes container width without a window resize event.
|
|
|
|
**Prevention:** Use `ResizeObserver` on the container element instead. It fires on any container size change regardless of cause.
|
|
|
|
### Pitfall 10: Entry Count Toggle Is Not Just a CSS Swap
|
|
|
|
**What goes wrong:** The hours/count toggle seems trivial -- just show `d.hours` vs `d.count`. But the color scale domain must change (hours: 0-12 vs count: 0-20+), tooltips must show different labels, and stats must recalculate.
|
|
|
|
**Prevention:** Treat the metric toggle as a state change that triggers a full re-render with the appropriate color scale, tooltip format, and stats calculation. The backend already returns both `hours` and `count` in `DayEntry`, so no API changes needed.
|
|
|
|
### Pitfall 11: Week Start Preference Must Propagate to All Modes
|
|
|
|
**What goes wrong:** Year-mode respects `data-week-start` (monday/sunday). Week-mode aggregation (day-of-week buckets) must also respect this -- Monday-first vs Sunday-first changes which column is "day 0". Day-mode hour bucketing is unaffected but the combined day/hour matrix is.
|
|
|
|
**Prevention:** Include `weekStart` in the state object passed to all visualization modes. Test both configurations explicitly in Vitest.
|
|
|
|
### Pitfall 12: Multiple Render Functions Diverging Over Time
|
|
|
|
**What goes wrong:** Starting with one `renderHeatmap()` and adding `renderWeekMode()`, `renderDayMode()`, `renderCombinedMode()` as separate functions. Over time, bug fixes (tooltip positioning, accessibility, theme colors) get applied to one function but not others.
|
|
|
|
**Prevention:** Extract shared concerns (tooltip management, color scale creation, cell click handling, stats rendering) into utility functions that all modes use. Each mode only implements its unique layout logic (grid generation, axis labels, cell positioning).
|
|
|
|
## Phase-Specific Warnings
|
|
|
|
| Phase Topic | Likely Pitfall | Mitigation |
|
|
|-------------|---------------|------------|
|
|
| State management refactor | Pitfall 4 (mode state lost on filter) | Implement state object BEFORE adding any new mode |
|
|
| Entity pickers / cascade | Pitfall 1 (KimaiFormSelect needs form lifecycle) | Roll your own cascade with fetch + plain selects |
|
|
| Entity pickers / cascade | Pitfall 7 (TomSelect bundle duplication) | Start with plain selects, no TomSelect |
|
|
| Cascade API endpoints | Pitfall 5 (API response format), Pitfall 8 (auth) | Own controller endpoints returning simple format |
|
|
| Week/day modes | Pitfall 2 (tooltip cleanup), Pitfall 6 (color scale per mode) | Shared tooltip manager, per-mode scale factory |
|
|
| Day/hour visualization | Pitfall 3 (hour aggregation query perf) | Narrow date range for hour-level queries |
|
|
| Hours/count toggle | Pitfall 10 (not just CSS) | Full re-render with different scale and stats |
|
|
| All new modes | Pitfall 11 (week start), Pitfall 12 (code divergence) | Shared utilities, weekStart in state, test both configs |
|
|
|
|
## Sources
|
|
|
|
- `dev/kimai/assets/js/forms/KimaiFormSelect.js` lines 386-447: cascade logic via `_activateApiSelects`, `getContainer().getPlugin('api')` dependency at line 433
|
|
- `dev/kimai/src/Form/Extension/SelectWithApiDataExtension.php`: wires `data-api-url`, `data-related-select`, `data-empty-url` onto Symfony EntityType form views only
|
|
- `dev/kimai/src/Form/Type/ProjectType.php` lines 124-137: `api_data` option configures activity cascade route
|
|
- `assets/src/heatmap.ts`: tooltip on `document.body` (line 267), `innerHTML` cleanup (line 184), resize handler (lines 392-398), `doRender` always calls year-view (line 341)
|
|
- `src/Service/HeatmapService.php`: daily aggregation via `DATE(t.date)` GROUP BY (lines 24-31)
|
|
- `assets/src/types.ts`: `DayEntry` has both `hours` and `count` fields
|
|
- `.claude/projects/.../memory/project_kimai_lessons.md`: Phase 1-2 lessons on DI, widget system, testing
|