Add total time to chronos timer#90787
Conversation
🦜 Polyglot Parrot! 🦜Squawk! Looks like you added some shiny new English strings. Allow me to parrot them back to you in other tongues: View the translation diffdiff --git a/src/languages/es.ts b/src/languages/es.ts
index c34c9492..4043c7e7 100644
--- a/src/languages/es.ts
+++ b/src/languages/es.ts
@@ -1692,10 +1692,8 @@ const translations: TranslationDeepObject<typeof en> = {
backdropLabel: 'Fondo del Modal',
},
nextStep: {
- /* eslint-disable @typescript-eslint/no-unused-vars */
message: {
[CONST.NEXT_STEP.MESSAGE_KEY.WAITING_TO_ADD_TRANSACTIONS]: (actor, actorType, _eta, _etaType) => {
- // eslint-disable-next-line default-case
switch (actorType) {
case CONST.NEXT_STEP.ACTOR_TYPE.CURRENT_USER:
return `Esperando a que <strong>tú</strong> añadas gastos.`;
@@ -1706,7 +1704,6 @@ const translations: TranslationDeepObject<typeof en> = {
}
},
[CONST.NEXT_STEP.MESSAGE_KEY.WAITING_TO_SUBMIT]: (actor, actorType, _eta, _etaType) => {
- // eslint-disable-next-line default-case
switch (actorType) {
case CONST.NEXT_STEP.ACTOR_TYPE.CURRENT_USER:
return `Esperando a que <strong>tú</strong> envíes los gastos.`;
@@ -1718,7 +1715,6 @@ const translations: TranslationDeepObject<typeof en> = {
},
[CONST.NEXT_STEP.MESSAGE_KEY.NO_FURTHER_ACTION]: (_actor, _actorType, _eta, _etaType) => `¡No se requiere ninguna acción adicional!`,
[CONST.NEXT_STEP.MESSAGE_KEY.WAITING_FOR_SUBMITTER_ACCOUNT]: (actor, actorType, _eta, _etaType) => {
- // eslint-disable-next-line default-case
switch (actorType) {
case CONST.NEXT_STEP.ACTOR_TYPE.CURRENT_USER:
return `Esperando a que <strong>tú</strong> añadas una cuenta bancaria.`;
@@ -1733,7 +1729,6 @@ const translations: TranslationDeepObject<typeof en> = {
if (eta) {
formattedETA = etaType === CONST.NEXT_STEP.ETA_TYPE.DATE_TIME ? ` el ${eta} de cada mes` : ` ${eta}`;
}
- // eslint-disable-next-line default-case
switch (actorType) {
case CONST.NEXT_STEP.ACTOR_TYPE.CURRENT_USER:
return `Esperando a que tus gastos se envíen automáticamente${formattedETA}.`;
@@ -1744,7 +1739,6 @@ const translations: TranslationDeepObject<typeof en> = {
}
},
[CONST.NEXT_STEP.MESSAGE_KEY.WAITING_TO_FIX_ISSUES]: (actor, actorType, _eta, _etaType) => {
- // eslint-disable-next-line default-case
switch (actorType) {
case CONST.NEXT_STEP.ACTOR_TYPE.CURRENT_USER:
return `Esperando a que <strong>tú</strong> soluciones ellos problemas.`;
@@ -1755,7 +1749,6 @@ const translations: TranslationDeepObject<typeof en> = {
}
},
[CONST.NEXT_STEP.MESSAGE_KEY.WAITING_TO_APPROVE]: (actor, actorType, _eta, _etaType) => {
- // eslint-disable-next-line default-case
switch (actorType) {
case CONST.NEXT_STEP.ACTOR_TYPE.CURRENT_USER:
return `Esperando a que <strong>tú</strong> apruebes los gastos.`;
@@ -1766,7 +1759,6 @@ const translations: TranslationDeepObject<typeof en> = {
}
},
[CONST.NEXT_STEP.MESSAGE_KEY.WAITING_TO_EXPORT]: (actor, actorType, _eta, _etaType) => {
- // eslint-disable-next-line default-case
switch (actorType) {
case CONST.NEXT_STEP.ACTOR_TYPE.CURRENT_USER:
return `Esperando a que <strong>tú</strong> exportes este informe.`;
@@ -1777,7 +1769,6 @@ const translations: TranslationDeepObject<typeof en> = {
}
},
[CONST.NEXT_STEP.MESSAGE_KEY.WAITING_TO_PAY]: (actor, actorType, _eta, _etaType) => {
- // eslint-disable-next-line default-case
switch (actorType) {
case CONST.NEXT_STEP.ACTOR_TYPE.CURRENT_USER:
return `Esperando a que <strong>tú</strong> pagues los gastos.`;
@@ -1788,7 +1779,6 @@ const translations: TranslationDeepObject<typeof en> = {
}
},
[CONST.NEXT_STEP.MESSAGE_KEY.WAITING_FOR_POLICY_BANK_ACCOUNT]: (actor, actorType, _eta, _etaType) => {
- // eslint-disable-next-line default-case
switch (actorType) {
case CONST.NEXT_STEP.ACTOR_TYPE.CURRENT_USER:
return `Esperando a que <strong>tú</strong> termines de configurar una cuenta bancaria de empresa.`;
@@ -1808,7 +1798,6 @@ const translations: TranslationDeepObject<typeof en> = {
[CONST.NEXT_STEP.MESSAGE_KEY.SUBMITTING_TO_SELF]: (_actor, _actorType, _eta, _etaType) =>
`¡Ups! Parece que estás enviando el informe a <strong>ti mismo</strong>. Aprobar tus propios informes está <strong>prohibido</strong> por tu espacio de trabajo. Por favor, envía este informe a otra persona o contacta a tu administrador para cambiar la persona a la que lo envías.`,
[CONST.NEXT_STEP.MESSAGE_KEY.REJECTED_REPORT]: (actor, actorType) => {
- // eslint-disable-next-line default-case
switch (actorType) {
case CONST.NEXT_STEP.ACTOR_TYPE.CURRENT_USER:
return `Este informe fue rechazado. Esperando a que <strong>tú</strong> corrijas los problemas y lo vuelvas a enviar manualmente.`;
@@ -2464,7 +2453,6 @@ ${amount} para ${merchant} - ${date}`,
two: 'º',
few: 'º',
other: 'º',
- /* eslint-disable @typescript-eslint/naming-convention */
'1': 'Primero',
'2': 'Segundo',
'3': 'Tercero',
@@ -2475,7 +2463,6 @@ ${amount} para ${merchant} - ${date}`,
'8': 'Octavo',
'9': 'Noveno',
'10': 'Décimo',
- /* eslint-enable @typescript-eslint/naming-convention */
},
},
approverInMultipleWorkflows: 'Este miembro ya pertenece a otro flujo de aprobación. Cualquier actualización aquí se reflejará allí también.',
@@ -6961,7 +6948,6 @@ ${amount} para ${merchant} - ${date}`,
restrictedDescription: 'Sólo las personas en tu espacio de trabajo pueden encontrar esta sala',
privateDescription: 'Sólo las personas que están invitadas a esta sala pueden encontrarla',
publicDescription: 'Cualquier persona puede unirse a esta sala',
- // eslint-disable-next-line @typescript-eslint/naming-convention
public_announceDescription: 'Cualquier persona puede unirse a esta sala',
createRoom: 'Crea una sala de chat',
roomAlreadyExistsError: 'Ya existe una sala con este nombre',
@@ -6981,7 +6967,6 @@ ${amount} para ${merchant} - ${date}`,
restricted: 'Espacio de trabajo',
private: 'Privada',
public: 'Público',
- // eslint-disable-next-line @typescript-eslint/naming-convention
public_announce: 'Anuncio Público',
},
},
@@ -7296,7 +7281,6 @@ ${amount} para ${merchant} - ${date}`,
updatedDefaultTitle: (newDefaultTitle, oldDefaultTitle) => `cambió la fórmula personalizada del nombre del informe a "${newDefaultTitle}" (previamente "${oldDefaultTitle}")`,
updatedOwnership: (oldOwnerEmail, oldOwnerName, policyName) => `asumió la propiedad del espacio de trabajo ${policyName} de ${oldOwnerName} (${oldOwnerEmail})`,
updatedAutoHarvesting: (enabled) => `${enabled ? 'habilitó' : 'deshabilitó'} el envío programado`,
- // eslint-disable-next-line @typescript-eslint/max-params
updatedIndividualBudgetNotification: (
budgetAmount,
budgetFrequency,
diff --git a/src/languages/pl.ts b/src/languages/pl.ts
index 504e4697..485f9eb0 100644
--- a/src/languages/pl.ts
+++ b/src/languages/pl.ts
@@ -8051,7 +8051,7 @@ Dodaj więcej zasad wydatków, żeby chronić płynność finansową firmy.`,
oooEventSummaryFullDay: (summary: string, dayCount: number, date: string) => `${summary} za ${dayCount} ${dayCount === 1 ? 'dzień' : 'dni'} do ${date}`,
oooEventSummaryPartialDay: (summary: string, timePeriod: string, date: string) => `${summary} z ${timePeriod} z dnia ${date}`,
startTimer: 'Startuj licznik',
- stopTimer: (duration: string) => `Zatrzymaj licznik czasu (${duration})`,
+ stopTimer: (duration: string) => `Zatrzymaj licznik (${duration})`,
scheduleOOO: 'Zaplanuj nieobecność',
scheduleOOOTitle: 'Zaplanuj nieobecność',
date: 'Data',
diff --git a/src/languages/zh-hans.ts b/src/languages/zh-hans.ts
index d9352224..fafe83ec 100644
--- a/src/languages/zh-hans.ts
+++ b/src/languages/zh-hans.ts
@@ -7848,7 +7848,7 @@ ${reportName}
oooEventSummaryFullDay: (summary: string, dayCount: number, date: string) => `${summary},共计 ${dayCount} ${dayCount === 1 ? '天' : '天'},截至 ${date}`,
oooEventSummaryPartialDay: (summary: string, timePeriod: string, date: string) => `${summary},时间范围:${timePeriod},日期:${date}`,
startTimer: '开始计时',
- stopTimer: (duration: string) => `停止计时器 (${duration})`,
+ stopTimer: (duration: string) => `停止计时器(${duration})`,
scheduleOOO: '安排外出办公',
scheduleOOOTitle: '安排外出办公',
date: '日期',
Note You can apply these changes to your branch by copying the patch to your clipboard, then running |
| function formatElapsedTime(startTime: string): string { | ||
| const elapsedMs = Date.now() - new Date(`${startTime}Z`).getTime(); | ||
| const totalMinutes = Math.floor(elapsedMs / 60000); | ||
| const hours = Math.floor(totalMinutes / 60); |
There was a problem hiding this comment.
❌ CONSISTENCY-2 (docs)
The value 60000 (milliseconds per minute) is a magic number that reduces readability. While 60 for minutes-per-hour is relatively self-explanatory, 60000 requires the reader to recognize it as a unit conversion.
Extract this to a named constant:
const MS_PER_MINUTE = 60000;
function formatElapsedTime(startTime: string): string {
const elapsedMs = Date.now() - new Date(`${startTime}Z`).getTime();
const totalMinutes = Math.floor(elapsedMs / MS_PER_MINUTE);
const hours = Math.floor(totalMinutes / 60);
const minutes = totalMinutes % 60;
return `${hours}:${String(minutes).padStart(2, '0')}`;
}Reviewed at: a991ec9 | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a991ec9810
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| selector: (reportActions: unknown): string | null => { | ||
| const sorted = getSortedReportActionsForDisplay(reportActions as OnyxEntry<ReportActions>, canPerformWriteAction, false, visibleReportActionsData, report.reportID); | ||
| return isChronosTimerRunningFromVisibleActions(sorted, currentUserAccountID); | ||
| return getTimeOfChronosTimerRunningFromVisibleActions(sorted, currentUserAccountID); |
There was a problem hiding this comment.
Update Chronos utils mock to include renamed selector
This callsite now depends on getTimeOfChronosTimerRunningFromVisibleActions, but the existing mock in tests/ui/ChronosTimerHeaderButton.test.tsx still only provides isChronosTimerRunningFromVisibleActions. In Jest, that leaves this import undefined, so rendering ChronosTimerHeaderButton throws a TypeError when the selector runs. This change introduces a deterministic test break in the Chronos header button UI tests unless the mock export is updated.
Useful? React with 👍 / 👎.
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
tgolen
left a comment
There was a problem hiding this comment.
I think having it update live would be stellar. It would probably be pretty easy to add at some point in the future.
Also, if we're putting more work into these kind of features, I think we should change how the code determines that a timer is running.
Right now, the FE code just looks at the report actions to find the last start/stop. The proper way of doing it is connecting to the chronos timing NVP, and if there is a startTime value, then there is a running timer. That would require BE changes to sync the NVP to the FE.
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
|
🚧 @tgolen has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/tgolen in version: 9.3.76-0 🚀
|
|
I reviewed the changes in this PR and the existing help site articles under No help site changes are required. This PR adds elapsed time display to the Chronos timer's "Stop Timer" button (e.g., "Stop Timer (1:30)"). The existing help site articles cover workspace-level Time Tracking features (enabling time tracking, setting hourly rates, creating time expenses) but do not document the Chronos timer start/stop button in the chat header. Since no existing documentation references this button's label or behavior, no updates are needed. |
|
🚀 Deployed to production by https://github.com/roryabraham in version: 9.3.77-3 🚀
|
Explanation of Change
I always wonder how long my current timer has been running and since now we have the stop timer button, it's the perfect pace to add that information.
I purposely did not make it live update, since I think that would make things more complex and it's not really needed since most people don't remain in the chronos chat all the time. LMK if you disagree and I could add a 1m setInterval call.
Tests
Offline tests
No
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
Same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari