Skip to content

fix: Allow any minute value (0-59) in time entry modal#181

Open
EdiWeeks wants to merge 3 commits into
mainfrom
fix/177-decimal-hours-input
Open

fix: Allow any minute value (0-59) in time entry modal#181
EdiWeeks wants to merge 3 commits into
mainfrom
fix/177-decimal-hours-input

Conversation

@EdiWeeks
Copy link
Copy Markdown
Contributor

@EdiWeeks EdiWeeks commented Mar 17, 2026

Summary

  • Removed 15-minute step restriction on the Minutes input in the time entry modal, allowing any value from 0-59
  • Changed minutes max from 45 to 59 so users can log precise times like 1h 45m
  • Inlined the handleHoursChange helper and improved empty-state display when editing entries
image image

Closes #177

Test plan

  • Open the time entry modal and verify the Minutes field accepts any value from 0-59
  • Enter 1h 45m and confirm it saves and displays as 1h 45m
  • Enter 0h 15m and confirm it saves and displays as 15m
  • Enter 24h and confirm minutes is disabled and reset to 0
  • Edit an existing entry and verify hours/minutes populate correctly

🤖 Generated with Claude Code

Previously minutes were restricted to 15-minute increments (step=15,
max=45). Now users can enter any minute value from 0-59, enabling
precise time logging like 1h 45m.

Closes #177

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@EdiWeeks EdiWeeks added the bug Something isn't working label Mar 17, 2026
@EdiWeeks EdiWeeks requested a review from Copilot March 17, 2026 10:58
Copy link
Copy Markdown
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 timesheet time-entry modal to support precise minute entry (0–59) instead of fixed 15-minute increments, improving accuracy when logging/editing durations.

Changes:

  • Removes the 15-minute stepping restriction for the Minutes field.
  • Increases the Minutes max from 45 to 59.
  • Adjusts edit-mode prefill to show empty inputs when hours/minutes are zero, and inlines the “hours >= 24 => minutes = 0” behavior.

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

Comment on lines 121 to 122
const h = Math.floor(entry.hours);
const m = Math.round((entry.hours - h) * 60);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 8c039d8. Now using totalMinutes = Math.round(entry.hours * 60) then deriving h = Math.floor(totalMinutes / 60) and m = totalMinutes % 60 to avoid the rounding-to-60 edge case.

Comment on lines 343 to 349
label="Minutes"
type="number"
min="0"
max="45"
step="15"
max="59"
value={minutes}
onChange={(e) => setMinutes(e.target.value)}
placeholder="0"
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 8c039d8. Both onChange handlers now clamp values to valid ranges (hours 0-24, minutes 0-59). The submit handler also clamps before computing totalHours as a safety net.

- Use totalMinutes approach to avoid Math.round rounding to 60 minutes
- Clamp hours (0-24) and minutes (0-59) in onChange handlers
- Validate/clamp values in handleSubmit before computing totalHours

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@EdiWeeks EdiWeeks requested a review from BenGWeeks March 17, 2026 11:07
@EdiWeeks
Copy link
Copy Markdown
Contributor Author

Tested, Copilot comment addressed. Please approve @BenGWeeks

Copilot AI review requested due to automatic review settings May 1, 2026 10:51
Copy link
Copy Markdown
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 timesheet time entry modal to support precise hour/minute entry (0–59 minutes) instead of 15-minute increments, aligning logged time with expected real-world durations and addressing #177.

Changes:

  • Removed the 15-minute step restriction and increased Minutes max from 45 to 59.
  • Improved edit-mode prefill by deriving hours/minutes from rounded total minutes to avoid the “60 minutes” rounding edge case.
  • Added clamping in onChange handlers and in submit logic (hours 0–24, minutes 0–59; minutes forced to 0 when hours is 24).

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

const h = Math.floor(totalMinutes / 60);
const m = totalMinutes % 60;
setHours(h > 0 ? h.toString() : '');
setMinutes(m > 0 ? m.toString() : '');
@BenGWeeks
Copy link
Copy Markdown
Contributor

Do we still need this @EdiWeeks you think? Prefer to log time to the minute? Might be good for agents so very specific time (they might be very quick). Gneerally previously stuck to 0.25 increments so easier billing though.

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

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Add two digit decimals to hours

3 participants