Conversation
|
…-time-format Co-authored-by: Cursor <cursoragent@cursor.com>
|
Merged master in |
Pull request was converted to draft
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
jvsena42
left a comment
There was a problem hiding this comment.
Approved — no HIGH/MEDIUM findings at head e201c7e71.
Checked: The earlier change requests are addressed at head e201c7e: the branch merges cleanly into current master (GitHub reports MERGEABLE, git merge-tree shows no conflicts), DATE_TIME_YEAR_SHORT now reuses DatePattern.DATE_FORMAT instead of duplicating the pattern, and toEpochSecondsOrNull drops pre-1970 instants before toULong(), so they fall back to the raw string. All four rows (Created on, Order expiry, Opened on, Closed on) go through uiDateText, which reads LocalIs24HourFormat. That local is provided in ContentView.kt:454 by rememberIs24HourFormat(), so the screen updates on ACTION_TIME_CHANGED without reopening. channelDateText keeps the old fallback to the raw string when parsing fails. The Opened on value comes from the same activity timestamp/confirmTimestamp that the activity list already formats with uiDateText, so dropping the old runCatching there adds no new crash path. DatePattern.CHANNEL_DETAILS has no remaining references, and the new tests check both clock formats and the locale-dependent month name.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
# Conflicts: # journeys/README.md
jvsena42
left a comment
There was a problem hiding this comment.
Approved — no blocking HIGH/MEDIUM findings at head e67916b74.
Inline: LOW — A CJIT connection meets the precondition but never shows "Opened on"
Checked: Follow-up on 6f4adcf (new journey): Reviewed the one new own commit 6f4adcf (journey lightning-connections/channel-details-time-format.xml plus README rows). Every testTag it names exists at head: HeaderMenu (HomeScreen.kt:1402), DrawerSettings (DrawerMenu.kt:311), Tab-advanced (CustomTabRowWithSpacing.kt:62 from SettingsTab.Advanced), Channels (SettingsScreen.kt:596) and Channel (LightningConnectionsScreen.kt:345), in that navigation order. The journey changes the format through the Settings app rather than settings put, which is correct: rememberIs24HourFormat (DateText.kt:40) only re-reads DateFormat.is24HourFormat on ACTION_TIME_CHANGED, which settings put system time_12_24 never broadcasts, and since the local is provided once at ContentView.kt:460 a settings put change would stay stale even after reopening the screen. Relaunching with am start -n brings back the existing singleTask MainActivity, so the "still on Connection details" check holds. The expected values match DATE_TIME_YEAR_SHORT = MMM d, yyyy, h:mm a / MMM d, yyyy, HH:mm, and with the fix reverted master's hardcoded MMM d, yyyy, HH:mm would fail the first AM/PM assertion. The iOS skip is recorded in the cross-platform table.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@jvsena42 conflicts |
|
In hand — a merge of current master is running on this branch right now (this one pushes to the fork over SSH, since it is a cross-repo PR). Same cause as the rest of the batch: every PR here commits a journey and adds a row to |
# Conflicts: # journeys/README.md
|
@talosmachina any idea how we can avoid this class of issues: I want something basic so we avoid unwanted conflicts of this kind, something simple not elaborate like the changelog fragments solution we have for changelogs. |
|
From the receiving end of about thirty of these today, the cause is narrow: Simplest fix, no tooling: move that metadata into the journey it describes. Each journey XML already has a If a listing is still wanted, it can be generated rather than typed: Middle option if the tables should stay: one README per suite ( For this batch specifically, the offer stands to strip the README rows out of the open PRs and land them in one follow-up, which ends the re-merges immediately: #1275 (comment) |
jvsena42
left a comment
There was a problem hiding this comment.
Approved — no blocking HIGH/MEDIUM findings at head 1a3f76e98.
Checked: Follow-up on 79bb2c8: the journey LOW is fixed. The precondition now requires a connection opened with Transfer to Spending from savings and explains why a CJIT connection never shows "Opened on" (it is read from the on-chain funding transfer), and the tap step picks that connection when several rows share the Channel tag.
# Conflicts: # journeys/README.md
jvsena42 Agreed on the diagnosis, and the suites table is now gone from both PRs — the count column was stale the moment any PR added a journey, and the Notes column had already moved: its setup information is the Nothing was dropped silently. On iOS the per-suite divergence notes ("adapted", "iOS-only") moved into Two corrections to the plan as written:
Worth noting the change argued for itself while landing: merging master into #1250 conflicted in exactly that table, for the third time in one sitting. Per-suite READMEs and stripping the README rows from the other open PRs are both worth doing and are independent of these two — happy to take the offer on the batch. |
There was a problem hiding this comment.
Verdict: ✅ Approve
Reaudit: diff 1 file.
Counterpart synonymdev/bitkit-ios#710: not compared.
Findings:
N/A
Audit:
Already done in comment.
Coverage:
QA: journeys and manual tests await all reviewers to approve, author can run it now via comment: @ovi-reviewer test
Reviewed by gpt-5.6-sol-high via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest · audit (author or owner)
Fixes #1110
Related to #1275
This PR routes the Lightning connection timestamps through the shared
uiDateTextentry point, so they follow the device's 12/24-hour setting.Description
Channel details built its own formatter from
DatePattern.CHANNEL_DETAILS, the hardcoded 24-hour patternMMM d, yyyy, HH:mm. A connection created at 8:58 PM therefore read20:58on a device set to a 12-hour clock, while the activity list one screen away read8:58 PM. #1111 addeduiDateTextand left this screen for a follow-up.uiDateText, which readsLocalIs24HourFormatand updates while the screen is open.DatePattern.DATE_FORMATfor theMMM d, yyyydate, so the date shape this screen shares with iOS stays the same.DatePattern.CHANNEL_DETAILSand the screen's own date formatting helpers, now unused.Out of Scope
,date/time separator is kept; Figma and iOS use-(Nov 13, 2022 - 14:38). Aligning it is a follow-up.LightningConnectionDetailView.swiftalso hardcodesHH:mm, so iOS has the same 12-hour bug. Needs a matching bitkit-ios fix.ui/screens/subscriptions/SubscriptionsScreen.kt: the first-period end date still uses a literalMMM d, yyyy, HH:mmand ignores the 12/24-hour setting. Not part of Channel details time uses different time format to system #1110.adb shell settings put system time_12_24does not refresh an open screen, because the app listens forACTION_TIME_CHANGED, which only the system Settings app broadcasts. Existing behaviour, unchanged.Design
Preview
Device set to 12-hour time. Connection 6 is the Blocktank order connection from the earlier captures, since closed, so it still shows Created on and Opened on:
Device set to 24-hour time, after the fix:
Flow on the fix build: 12-hour, then system Settings to 24-hour and back to the still-open screen, which updates live, then system Settings to Automatic and back, AM/PM again.
channel-date-format-flow.mp4
Earlier comparison captures from the original PR:
QA Notes
Manual Tests
Sep 15, 2026, 2:44 PM, matching the status bar. On a CJIT connection only Created on is shown, since Opened on comes from the on-chain transfer that funded the channel.Sep 15, 2026, 14:44without reopening.regression:Connection whose order timestamp is malformed: the row shows the raw value instead of being empty.A new journey covers steps 1 and 2 and was run on a Pixel 9 emulator. It is committed here at
journeys/lightning-connections/channel-details-time-format.xml, with its rows added tojourneys/README.md(suites table, and the cross-platform table as not ported — iOSLightningConnectionDetailView.swifthardcodesMMM d, yyyy - HH:mm):channel-details-time-format.xml
Automated Checks
DateTimeExtTest.kt: ISO-8601 parsing over the plain and millisecond forms, unparseable and empty input, pre-1970 rejection, epoch start, the short-month style in both clock formats, and its month name underLocale.GERMANY.just compile,just test,just lintpass; detekt reports no findings in the changed files.