Skip to content

Set timestamps in a chapter of a study#19728

Closed
nstillman-te wants to merge 1 commit intolichess-org:masterfrom
TechEmpower:inline-clock-editing-15009
Closed

Set timestamps in a chapter of a study#19728
nstillman-te wants to merge 1 commit intolichess-org:masterfrom
TechEmpower:inline-clock-editing-15009

Conversation

@nstillman-te
Copy link

@nstillman-te nstillman-te commented Mar 4, 2026

clock-edit-black

clock-edit-white

Closes #15009

Implements the feature requested in Feature request: Set timestamps in a chapter of a study: contributors can add and edit clock times at mainline positions in study chapters via the UI, without editing PGN by hand.

What

  • Any user who can edit the chapter can set or edit the clock time for the side that just moved, on mainline only.
  • At the initial position (no move selected), neither clock is editable or shown as active.
  • Relay chapters are unchanged (no clock editing).
  • Enter - or -- to clear the clock at that move. Changes are saved only when the chapter is in REC (recording) mode; an alert explains this if they try to save without REC.

How

  • Backend: setClock in StudySocket and StudyApi (Contribute, no relay). Accepts either a clock value or clear (Option[Clock]: Some(clock) to set, None to clear). Writes to the node (or $unset when clearing) and updates denorm via ChapterRepo.setClockAndDenorm.
  • Frontend: Click the clock (or --:-- placeholder) at the current position to open an inline field. Accepted formats: H:MM:SS, MM:SS, M:SS, SS, or SS.t. Parse/format in ui/analyse/src/view/clockParse.ts. Invalid input shows an error state; value is sent on Enter or blur when valid. Clear on Enter or blur when the input is - or --. Tooltip on the whole editable clock and on the input when editing.
  • i18n: Clock strings in translation/source/study.xml: addOrEditClockTime, clockTimeFormat, clockTimePlaceholder, turnOnRecToSaveClockTimes.
  • Tests: ui/analyse/tests/clocks.test.ts for parseTimeToCentis and formatClockFromCentis.

Related


AI-assisted contribution: This PR was developed with Cursor (agent mode). The GitHub issue above was used as context for the requested behavior. Implementation was refined through manual iteration and edge-case testing. The changes have been manually tested (set/edit/clear clock, REC on/off, mainline-only).

Note on "Allow edits from maintainers": This PR is from a fork created under an organization (TechEmpower). GitHub does not allow the "Allow edits from maintainers" option for such forks, so we can't enable it here. If this is a blocker for review or merge, please let us know and we can reopen the same changes from a personal fork where the option can be turned on.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Please allow edits from maintainers on your pull request. See how to enable them here

@ornicar
Copy link
Collaborator

ornicar commented Mar 5, 2026

Allow edits from maintainers

We'll need that. This is a lot of code and it will need a lot of change if it's going to make it to production.

Copy link
Collaborator

@ornicar ornicar left a comment

Choose a reason for hiding this comment

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

I'll continue the review when I'm able to make changes myself

applyWho(api.deleteComment(studyId, position.ref, Comment.Id(id)))

case "setClock" =>
logger.info(s"setClock received studyId=$studyId o=$o")
Copy link
Collaborator

Choose a reason for hiding this comment

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

debug

border: 1px solid $c-border;
border-radius: 3px;
background: $c-bg-box;
box-sizing: border-box;
Copy link
Collaborator

Choose a reason for hiding this comment

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

superfluous

}

.analyse__clock-input--error {
border-color: var(--c-bad, #c23) !important;
Copy link
Collaborator

Choose a reason for hiding this comment

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

just $c-bad

).map(c => (defined(c) && c < 0 ? undefined : c));
import { parseTimeToCentis, formatClockFromCentis } from './clockParse';

const isClearClockValue = (s: string): boolean => s.trim() === '-' || s.trim() === '--';
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it should be empty string instead. That's what people will try to do.

Copy link
Author

Choose a reason for hiding this comment

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

Addressed in #19743

@nstillman-te
Copy link
Author

Closing in favor of #19743 (personal fork — allow maintainer edits).

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.

Feature request: Set timestamps in a chapter of a study

2 participants