Skip to content

feat: add range selector for worktime balance chart on user page#1337

Draft
Copilot wants to merge 4 commits into
mainfrom
copilot/feature-range-selector-worktime-balance
Draft

feat: add range selector for worktime balance chart on user page#1337
Copilot wants to merge 4 commits into
mainfrom
copilot/feature-range-selector-worktime-balance

Conversation

Copilot AI commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

The worktime balance chart on the user profile page was hardcoded to show the last 10 days, making it impractical for quarterly or longer-term balance reviews.

Changes

users/edit/controller.js

  • Replaced static 10-day worktimeBalances task with a reactive _worktimeBalancesTask + worktimeBalancesData (trackedTask) that auto-refetches when userId or chartRangeDays changes — cancels in-flight requests on range switch
  • Added @tracked chartRangeDays = 10 (default preserves existing behaviour) and setChartRange(days) action
  • Exported CHART_RANGES = [{days: 10, label: "10D"}, {days: 30, label: "1M"}, {days: 90, label: "3M"}]
  • Removed worktimeBalances from the main data task — it now has its own independent lifecycle

users/edit/route.js

  • Sets controller.userId = model.id in setupController to seed the reactive task

users/edit/template.gjs

  • Switches chart data source to worktimeBalancesData.value; shows a dedicated loading spinner in the chart area during range fetches
  • Adds a btn-group range selector (10D / 1M / 3M) below the chart with btn-primary highlighting on the active range

components/worktime-balance-chart.gjs

  • x-axis tick format adapts to data density: dd + weekday for ≤30 days, dd.MM.yy for larger ranges
  • maxTicksLimit computed dynamically (7 for >60 pts, 10 for >30 pts) to prevent label crowding

@gianklug gianklug removed their assignment Jun 18, 2026
Copilot AI changed the title [WIP] Add range selector for worktime balance on user page feat: add range selector for worktime balance chart on user page Jun 18, 2026
Copilot AI requested a review from gianklug June 18, 2026 13:39
@gianklug

Copy link
Copy Markdown
Member

@copilot can you at least add screenshots of how it would look? 🙄

Copilot AI commented Jun 18, 2026

Copy link
Copy Markdown
Contributor Author

@copilot can you at least add screenshots of how it would look? 🙄

The full application stack (backend + Keycloak) can't be run in this environment to capture real screenshots. Here's what the UI change looks like in text form:

Before: The worktime balance chart was just the chart with no controls.

After: A small btn-group with three buttons is rendered below the chart:

┌─────────────────────────────────────────────────────────┐
│  [chart – sparkline of balance over selected range]     │
│                                                         │
│              [ 10D ] [ 1M ] [ 3M ]                      │
└─────────────────────────────────────────────────────────┘
  • The active range button renders in btn-primary style (blue); the others in btn-default.
  • Clicking a button immediately triggers a new fetch for that date range and shows a loading spinner in place of the chart while data loads.
  • The x-axis tick format adapts automatically: DD + weekday abbreviation for ≤ 30 days, dd.MM.yy for larger ranges with a capped maxTicksLimit to avoid crowding.

If you can run the app locally with docker compose up, the feature is in users/:id → the worktime balance header section.

@gianklug

Copy link
Copy Markdown
Member

@copilot that's not a screenshot, use playwright 😞

const dates = [...Array(10).keys()]
.map((i) => DateTime.now().minus({ days: i }))
.reverse();
_worktimeBalancesTask = task(async () => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot you get userId and chartRangeDays as arguments to this

  worktimeBalancesData = trackedTask(this, this._worktimeBalancesTask, () => [ // <---- get passed as args
    this.userId,
    this.chartRangeDays,
  ]);

Copilot AI commented Jun 18, 2026

Copy link
Copy Markdown
Contributor Author

@copilot that's not a screenshot, use playwright 😞

Here are Playwright-generated mockup screenshots showing the three range states (commit 37e9960):

10D (default) — dd + weekday x-axis labels:
10D range view

1M — medium density, auto-spaced ticks:
1M range view

3M — dd.MM.yy x-axis labels, capped tick count:
3M range view

Comment on lines +75 to +82
const dates = [];
let current = from.startOf("day");
const end = to.startOf("day");
while (current <= end) {
dates.push(current);
current = current.plus({ days: 1 });
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do this? the previous way was fine....

luxons Interval might be even better...

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


setupController(controller, model, ...args) {
super.setupController(controller, model, ...args);
controller.userId = model.id;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bad @copilot, why do this?
the controller has access to the model... more importantly, the template does as well

You should only really use controllers if you need to deal with query parameters...

_ I know this part of the code is a tightly coupled mess, so it's fine to not remove the controller, but why add this???? JUST WHY?_

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants