Skip to content

Update dashboard message to include interval label for total time spent#1094

Open
alextheradu wants to merge 1 commit intohackclub:mainfrom
alextheradu:main
Open

Update dashboard message to include interval label for total time spent#1094
alextheradu wants to merge 1 commit intohackclub:mainfrom
alextheradu:main

Conversation

@alextheradu
Copy link
Contributor

Enhance the projects dashboard message by incorporating an interval label (e.g., "this week") to provide clearer context for the total time spent.

Copilot AI review requested due to automatic review settings March 19, 2026 19:58
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates the Projects dashboard “total time spent” message to include the currently selected interval label, giving users clearer context about what the aggregate time represents.

Changes:

  • Prefixes the “you’ve spent …” activity summary with the interval_label (e.g., “This week”).
  • Applies capitalization styling to the interval label in the message.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 19, 2026

Greptile Summary

This PR is a small UI enhancement to app/javascript/pages/Projects/Index.svelte that prefixes the total time summary sentence with the currently selected interval label (e.g. "This Week, you've spent 4h 30m coding across active projects."). The interval_label prop was already being passed from My::ProjectRepoMappingsController via the human_interval_name helper and typed in the component — this change simply renders it in the template.

Key observations:

  • The capitalize Tailwind class on the <span> is redundant for pre-defined intervals (which are already Title Case from the backend), and introduces a minor visual quirk for custom date ranges where human_interval_name returns "#{from} to #{to}" — causing "to" to render as "To".
  • The {:else} branch (no activity) is left unchanged and still shows a generic "You haven't logged any time for this interval yet." without mentioning the interval — creating a slight inconsistency with the updated {:if} branch.
  • Per the repository's guidelines, tests should accompany new/changed functionality. The existing controller test (project_repo_mappings_controller_test.rb) does not assert the interval_label prop value, and the system test (projects_test.rb) does not verify the label is rendered on screen. A simple assert_equal on the prop and/or an assert_text in the system test would provide adequate coverage.

Confidence Score: 4/5

  • Safe to merge — the change is cosmetic and uses an already-available, server-validated prop with no new data fetching or security surface.
  • The diff touches a single template line and introduces no new logic, API calls, or state. The two noted issues (capitalize on custom date ranges and else-branch inconsistency) are stylistic rather than functional bugs. Missing test coverage is a process concern per repo guidelines but does not block correctness.
  • No files require special attention beyond the single changed template line.

Important Files Changed

Filename Overview
app/javascript/pages/Projects/Index.svelte Adds interval_label to the summary sentence in the projects dashboard. Minor style issues: capitalize CSS can produce "To" in custom date ranges, and the no-activity else branch is left without the interval label, creating a small inconsistency.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[User visits /my/projects] --> B[ProjectRepoMappingsController#index]
    B --> C{interval param present?}
    C -->|"Standard key (e.g. this_week)"| D["human_interval_name → humanize_range\nreturns e.g. 'This Week'"]
    C -->|"custom + from/to params"| E["human_interval_name\nreturns '2024-01-01 to 2024-01-31'"]
    C -->|"No interval"| F["human_interval_name\nreturns 'All Time'"]
    D --> G[interval_label prop passed to Svelte]
    E --> G
    F --> G
    G --> H{projects_data.has_activity?}
    H -->|"true"| I["Renders: interval_label + capitalize CSS\n'This Week, you've spent X coding...'"]
    H -->|"false"| J["Renders: 'You haven't logged any time\nfor this interval yet.' (no label shown)"]
Loading

Comments Outside Diff (1)

  1. app/javascript/pages/Projects/Index.svelte, line 227-229 (link)

    P2 Inconsistency: else branch still omits interval label

    The if branch now prominently surfaces the interval label (e.g., "This Week, you've spent…"), but the else branch still shows the generic message "You haven't logged any time for this interval yet." without identifying the interval. This inconsistency means a user who has no activity for a specific interval (e.g., "Yesterday") won't see which period they are looking at.

    Consider applying the same treatment:

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: app/javascript/pages/Projects/Index.svelte
    Line: 227-229
    
    Comment:
    **Inconsistency: else branch still omits interval label**
    
    The `if` branch now prominently surfaces the interval label (e.g., "This Week, you've spent…"), but the `else` branch still shows the generic message "You haven't logged any time for this interval yet." without identifying the interval. This inconsistency means a user who has no activity for a specific interval (e.g., "Yesterday") won't see which period they are looking at.
    
    Consider applying the same treatment:
    
    
    
    How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
This is a comment left during a code review.
Path: app/javascript/pages/Projects/Index.svelte
Line: 227-229

Comment:
**Inconsistency: else branch still omits interval label**

The `if` branch now prominently surfaces the interval label (e.g., "This Week, you've spent…"), but the `else` branch still shows the generic message "You haven't logged any time for this interval yet." without identifying the interval. This inconsistency means a user who has no activity for a specific interval (e.g., "Yesterday") won't see which period they are looking at.

Consider applying the same treatment:

```suggestion
          <span class="capitalize">{interval_label}</span>, you haven't logged any time yet.
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: app/javascript/pages/Projects/Index.svelte
Line: 222

Comment:
**CSS `capitalize` unintentionally capitalises prepositions in custom date ranges**

`text-transform: capitalize` uppercases the first letter of **every word**. For standard interval labels (`"This Week"`, `"Last 7 Days"`, etc.) the labels are already Title Case from the backend so the class has no visible effect. However, when a user selects a custom date range, `human_interval_name` returns `"#{from} to #{to}"` (e.g. `"2024-01-01 to 2024-01-31"`), and `capitalize` turns "to" into "To", yielding the slightly awkward display **"2024-01-01 To 2024-01-31"**.

Since all pre-defined labels (`human_name` in `RANGES`) and the "All Time" fallback are already properly cased, the `capitalize` class is redundant for them and introduces this edge-case visual glitch for custom ranges. Consider removing the class, or handling the custom date range sentence separately.

How can I resolve this? If you propose a fix, please make it concise.

Last reviewed commit: "Update dashboard mes..."

<p class="text-lg text-surface-content">
{#if projects_data.has_activity}
You've spent
<span class="capitalize">{interval_label}</span>, you've spent
Copy link
Contributor

Choose a reason for hiding this comment

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

P2 CSS capitalize unintentionally capitalises prepositions in custom date ranges

text-transform: capitalize uppercases the first letter of every word. For standard interval labels ("This Week", "Last 7 Days", etc.) the labels are already Title Case from the backend so the class has no visible effect. However, when a user selects a custom date range, human_interval_name returns "#{from} to #{to}" (e.g. "2024-01-01 to 2024-01-31"), and capitalize turns "to" into "To", yielding the slightly awkward display "2024-01-01 To 2024-01-31".

Since all pre-defined labels (human_name in RANGES) and the "All Time" fallback are already properly cased, the capitalize class is redundant for them and introduces this edge-case visual glitch for custom ranges. Consider removing the class, or handling the custom date range sentence separately.

Prompt To Fix With AI
This is a comment left during a code review.
Path: app/javascript/pages/Projects/Index.svelte
Line: 222

Comment:
**CSS `capitalize` unintentionally capitalises prepositions in custom date ranges**

`text-transform: capitalize` uppercases the first letter of **every word**. For standard interval labels (`"This Week"`, `"Last 7 Days"`, etc.) the labels are already Title Case from the backend so the class has no visible effect. However, when a user selects a custom date range, `human_interval_name` returns `"#{from} to #{to}"` (e.g. `"2024-01-01 to 2024-01-31"`), and `capitalize` turns "to" into "To", yielding the slightly awkward display **"2024-01-01 To 2024-01-31"**.

Since all pre-defined labels (`human_name` in `RANGES`) and the "All Time" fallback are already properly cased, the `capitalize` class is redundant for them and introduces this edge-case visual glitch for custom ranges. Consider removing the class, or handling the custom date range sentence separately.

How can I resolve this? If you propose a fix, please make it concise.

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.

2 participants