Adjustable auto-close #549
Conversation
Klakurka
added
bug
WalkthroughDocumentation and React UI now support tri-state autoClose (boolean | number | string) with a 2s default. PaymentDialog uses getAutoCloseDelay to compute numeric delays and manages timers; PayButton prop type and default updated. New util, constant, test and re-export added. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant PB as PayButton
participant PD as PaymentDialog
participant Util as getAutoCloseDelay / Constants
participant Timer as setTimeout
U->>PB: click -> open dialog (props include autoClose)
PB->>PD: render with autoClose
PD->>PD: success -> onSuccess()
PD->>Util: getAutoCloseDelay(autoClose)
Util-->>PD: delay (ms) or undefined
alt delay is number
PD->>Timer: setTimeout(handleWidgetClose, delay)
Timer-->>PD: timeout fires
PD->>PD: handleWidgetClose() (clears timer, closes)
else undefined
PD-->>PD: no auto-close scheduled
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Biome (2.1.2)react/lib/components/PaymentDialog/PaymentDialog.tsxbiome: error while loading shared libraries: libpthread.so.0: cannot open shared object file: No such file or directory react/lib/tests/util/autoClose.test.tsbiome: error while loading shared libraries: libpthread.so.0: cannot open shared object file: No such file or directory react/lib/util/autoClose.tsbiome: error while loading shared libraries: libpthread.so.0: cannot open shared object file: No such file or directory
✨ Finishing Touches
🧪 Generate unit tests
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
docs/zh-tw/README.md (1)
1016-1036: Clarify that decimal seconds are allowed; consider showing a decimal in examples.Docs say “數字（單位：秒）” but the PR allows decimals. Suggest explicitly stating “可為小數” and use a 1.5s example for HTML/JS/React to make it obvious.
-?> 此參數是可選的。預設為 true（2 秒）。可接受 true、false，或數字（單位：秒）以自訂延遲。 +?> 此參數是可選的。預設為 true（2 秒）。可接受 true、false，或數字（單位：秒，可為小數）以自訂延遲。 @@ -auto-close="1" +auto-close="1.5" @@ -autoClose: 1 +autoClose: 1.5 @@ -autoClose = 1 +autoClose = 1.5docs/zh-cn/README.md (1)
1018-1038: 在文档中明确支持小数秒，并用小数示例。当前写法为“数字（秒）”，建议补充“可为小数”，并将示例改为 1.5 秒以消除歧义。
-?> 此参数为可选参数。默认值为 true（2 秒）。可能的值为 true、false，或数字（单位：秒）以自定义延迟。 +?> 此参数为可选参数。默认值为 true（2 秒）。可能的值为 true、false，或数字（单位：秒，可为小数）以自定义延迟。 @@ -auto-close="1" +auto-close="1.5" @@ -autoClose: 1 +autoClose: 1.5 @@ -autoClose = 1 +autoClose = 1.5docs/README.md (1)
1022-1042: Call out decimal support; minor markdownlint nits nearby.
- Please note decimals are allowed and show a 1.5s example.
- markdownlint flagged heading/emphasis issues around these lines; worth a quick pass so docs lint stays green.
-?> This parameter is optional. Default is true (2 seconds). Possible values are true, false, or a number (seconds) to set a custom delay. +?> This parameter is optional. Default is true (2 seconds). Possible values are true, false, or a number (seconds, decimals allowed) to set a custom delay. @@ -auto-close="1" +auto-close="1.5" @@ -autoClose: 1 +autoClose: 1.5 @@ -autoClose = 1 +autoClose = 1.5react/lib/components/PayButton/PayButton.tsx (1)
373-374: Default changed to autoClose: true — highlight as a behavioral change.This alters existing defaults (previously false). Recommend noting in CHANGELOG/release notes and validating key flows that relied on staying open after success.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
docs/README.md(1 hunks)docs/zh-cn/README.md(1 hunks)docs/zh-tw/README.md(1 hunks)react/lib/components/PayButton/PayButton.tsx(2 hunks)react/lib/components/PaymentDialog/PaymentDialog.tsx(4 hunks)react/lib/util/constants.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
react/lib/components/PaymentDialog/PaymentDialog.tsx (2)
react/lib/util/types.ts (1)
Transaction(10-21)react/lib/util/constants.ts (1)
AUTO_CLOSE_DEFAULT_MS(12-12)
🪛 markdownlint-cli2 (0.17.2)
docs/README.md
1022-1022: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
1022-1022: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
1022-1022: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
1028-1028: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
1028-1028: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
1034-1034: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
1034-1034: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run build
🔇 Additional comments (4)
react/lib/components/PayButton/PayButton.tsx (1)
54-55: Prop type expansion: LGTM.Accepting boolean | number | string aligns with HTML attribute parsing and docs.
react/lib/components/PaymentDialog/PaymentDialog.tsx (3)
131-144: Delay parser: LGTM.Covers booleans, numeric strings, and decimals; 0/negative disables. Sensible defaults.
153-155: Re-opening dialog on success when closed — confirm desired UX.If a payment lands after the user has closed the dialog, this re-opens it. Confirm this is intentional and consistent with hideToasts flows.
6-6: Resolved:AUTO_CLOSE_DEFAULT_MSis re-exported byreact/lib/util/index.tsviaexport * from './constants', so importing it from../../utilis valid.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
react/lib/components/PaymentDialog/PaymentDialog.tsx (1)
133-141: Duplicate onClose avoided; timer now cleared on manual close and unmount.This resolves the prior double-invocation risk and manages the timeout lifecycle correctly.
Also applies to: 144-144, 155-159, 162-164
🧹 Nitpick comments (6)
react/lib/tests/util/autoClose.test.ts (2)
1-12: Prefer importing from the util barrel to exercise public API.Importing via the barrel catches regressions in re-exports and reduces path churn.
-import { getAutoCloseDelay } from '../../util/autoClose'; -import { AUTO_CLOSE_DEFAULT_MS } from '../../util/constants'; +import { getAutoCloseDelay, AUTO_CLOSE_DEFAULT_MS } from '../../util';
33-43: Add edge-case tests for numeric strings and large values (optional).To lock behavior, consider tests for: scientific/hex notation strings ('1e3', '0x10'), Infinity/NaN strings, very small positives ('0.0004'), and very large delays (clamping).
If you adopt the parsing/clamping change suggested in util/autoClose.ts, I can follow up with exact test cases.
react/lib/components/PaymentDialog/PaymentDialog.tsx (2)
63-64: Reuse the shared AutoCloseValue type.Keeps the prop type in sync with the util and avoids repeating the union.
-import { Currency, CurrencyObject, Transaction, isPropsTrue, isValidCashAddress, isValidXecAddress, getAutoCloseDelay } from '../../util'; +import { Currency, CurrencyObject, Transaction, isPropsTrue, isValidCashAddress, isValidXecAddress, getAutoCloseDelay } from '../../util'; +import type { AutoCloseValue } from '../../util'; @@ - autoClose?: boolean | number | string; + autoClose?: AutoCloseValue;Also applies to: 103-105
131-132: Nit: remove or move this comment.The comment says “Compute auto-close delay” but the computation happens in handleSuccess. Consider deleting or relocating for clarity.
react/lib/util/autoClose.ts (2)
1-26: Harden parsing and clamp delay to setTimeout limits (optional).Current logic accepts hex/scientific strings ('0x10', '1e3') and can return extremely large delays. Restrict to decimal strings, ensure a minimum of 1 ms when >0 seconds are provided, and clamp to 2^31-1 ms.
-import { AUTO_CLOSE_DEFAULT_MS } from './constants'; +import { AUTO_CLOSE_DEFAULT_MS, MAX_SET_TIMEOUT_MS } from './constants'; export type AutoCloseValue = boolean | number | string | undefined; /** * Determine auto-close delay (ms) from a variety of allowed input types. * Rules: * - undefined -> default enabled delay * - boolean -> true: default delay; false: disabled (undefined) * - number -> >0 seconds: round to nearest ms; <=0: disabled - * - string -> 'true': default; 'false': disabled; numeric string parsed like number; other strings: default + * - string -> 'true': default; 'false': disabled; decimal numeric string parsed like number; other strings: default */ export function getAutoCloseDelay(value: AutoCloseValue): number | undefined { if (value === undefined) return AUTO_CLOSE_DEFAULT_MS; if (typeof value === 'boolean') return value ? AUTO_CLOSE_DEFAULT_MS : undefined; - if (typeof value === 'number') return value > 0 ? Math.round(value * 1000) : undefined; + if (typeof value === 'number') return value > 0 ? clampMs(Math.round(value * 1000)) : undefined; if (typeof value === 'string') { - const trimmed = value.trim().toLowerCase(); - if (trimmed === 'true') return AUTO_CLOSE_DEFAULT_MS; - if (trimmed === 'false') return undefined; - const num = +trimmed; - if (!isNaN(num)) return num > 0 ? Math.round(num * 1000) : undefined; - return AUTO_CLOSE_DEFAULT_MS; + const trimmed = value.trim(); + const lowered = trimmed.toLowerCase(); + if (lowered === 'true') return AUTO_CLOSE_DEFAULT_MS; + if (lowered === 'false') return undefined; + const num = parseDecimalSeconds(trimmed); + if (num !== undefined) return num > 0 ? clampMs(Math.round(num * 1000)) : undefined; + return AUTO_CLOSE_DEFAULT_MS; } return undefined; } + +function clampMs(ms: number): number { + // enforce 1..MAX_SET_TIMEOUT_MS + return Math.min(Math.max(1, ms), MAX_SET_TIMEOUT_MS); +} + +function parseDecimalSeconds(s: string): number | undefined { + // Accept only plain decimal strings (e.g., "2", "2.5", ".5"), reject hex/exponential/Infinity. + if (!/^[+]?(\d+(\.\d+)?|\.\d+)$/.test(s)) return undefined; + const n = Number(s); + return Number.isFinite(n) ? n : undefined; +}Add this constant to keep the clamp self-documented (in react/lib/util/constants.ts):
+export const MAX_SET_TIMEOUT_MS = 2147483647; // 2^31 - 1
3-3: Exported AutoCloseValue type is useful; consider reusing it in component props.Recommend updating PaymentDialog and PayButton prop typings to import this type.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
react/lib/components/PaymentDialog/PaymentDialog.tsx(4 hunks)react/lib/tests/util/autoClose.test.ts(1 hunks)react/lib/util/autoClose.ts(1 hunks)react/lib/util/index.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
react/lib/tests/util/autoClose.test.ts (2)
react/lib/util/autoClose.ts (1)
getAutoCloseDelay(13-26)react/lib/util/constants.ts (1)
AUTO_CLOSE_DEFAULT_MS(12-12)
react/lib/util/autoClose.ts (1)
react/lib/util/constants.ts (1)
AUTO_CLOSE_DEFAULT_MS(12-12)
react/lib/components/PaymentDialog/PaymentDialog.tsx (2)
react/lib/util/types.ts (1)
Transaction(10-21)react/lib/util/autoClose.ts (1)
getAutoCloseDelay(13-26)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run build
🔇 Additional comments (2)
react/lib/util/index.ts (1)
14-14: Barrel re-export for autoClose looks good.Publicly exposing the new utilities via the util index keeps imports consistent across the codebase.
react/lib/components/PaymentDialog/PaymentDialog.tsx (1)
264-274: No action needed: autoClose defaults align across PaymentDialog, PayButton, and documentation (true → 2 s).
Related to #548
Description
Updated the auto-close parameter to allow entering a valid positive number (decimals are fine).
Test plan
Create a button with a custom auto-close duration and see that it meets expectations.
Remarks
This branch also makes auto-close = true by default which is what it was intended to be, also reducing the default (previously only) auto-close timer from 3s -> 2s.
Summary by CodeRabbit
New Features
Documentation
Tests