Swipe a recent document away, and say when it was opened - #528
Conversation
ca39104 to
982f294
Compare
679a402 to
6d2525c
Compare
982f294 to
f007dab
Compare
6d2525c to
b3bddb7
Compare
f007dab to
ce40dac
Compare
b3bddb7 to
5c5abbb
Compare
ce40dac to
509ca76
Compare
Two things the list was missing. Each row now carries the time it was last opened, which is the thing that tells two similarly named documents apart, and a recent document can be swiped off the list with an undo. Undo puts the entry back at the index it came from rather than at the front, so the list ends up looking exactly as it did - that is the point of an undo, and it is why RecentDocumentList grows an insert() next to add() instead of reusing it. Nothing releases the uri permission on the way out. An undo needs it back, and prune() reclaims it on the next launch anyway - which is what reconciling against the stored lists, rather than bookkeeping every removal, was for. Only the recently opened documents can be swiped. A document inside a granted folder is simply there, and the app has no business removing anything from disk; the adapter tells the two apart by whether the row has a timestamp on it. A "dark documents" switch was built here too and taken back out: with the html odrcore generates, algorithmic darkening changes nothing visible, so the setting would have looked broken. Making it work needs a dark stylesheet from the core, not a toggle here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TiSZ4FeptMUsFjAuCewzQZ
5c5abbb to
0881305
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0881305c85
ℹ️ 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".
|
|
||
| // a document inside a folder is not ours to forget - it is simply there. those carry no | ||
| // subtitle, which is what tells the two apart here. | ||
| return item is LandingItem.Document && item.subtitle != null |
There was a problem hiding this comment.
When a user upgrades with existing recent_documents.json entries that were written before lastOpenedAt was populated, those entries are still rendered in the Recent section but lastOpenedLabel(0) passes a null subtitle. This check then treats them as non-removable, so swipe-to-remove silently does not work for exactly those legacy recent documents until each one is opened again. Use explicit row provenance/state instead of the optional display subtitle to decide whether a document is a recent item.
Useful? React with 👍 / 👎.
| entries.add(index.coerceIn(0, entries.size), entry) | ||
|
|
||
| return entries |
There was a problem hiding this comment.
If the recent list is already at MAX_ENTRIES, the user can swipe one item away, open another document while the undo snackbar is still available, and then tap Undo. At that point current is full again, but insert() adds the restored entry and returns 33 items because it does not apply the same cap/eviction rule as add(), breaking the class invariant that the recent list cannot grow without bound and leaving extra persisted URI grants referenced.
Useful? React with 👍 / 👎.
Two things the list was missing. Each row now carries the time it was last opened, which is the thing that tells two similarly named documents apart, and a recent document can be swiped off the list with an undo. Undo puts the entry back at the index it came from rather than at the front, so the list ends up looking exactly as it did - that is the point of an undo, and it is why RecentDocumentList grows an insert() next to add() instead of reusing it. Nothing releases the uri permission on the way out. An undo needs it back, and prune() reclaims it on the next launch anyway - which is what reconciling against the stored lists, rather than bookkeeping every removal, was for. Only the recently opened documents can be swiped. A document inside a granted folder is simply there, and the app has no business removing anything from disk; the adapter tells the two apart by whether the row has a timestamp on it. A "dark documents" switch was built here too and taken back out: with the html odrcore generates, algorithmic darkening changes nothing visible, so the setting would have looked broken. Making it work needs a dark stylesheet from the core, not a toggle here. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 participant
🤖 Generated with Claude Code
Last of six, stacked. Base:
landing/05-folder-browsing(#527).Two things the list was missing: each row now carries when it was last opened — the thing that tells two similarly named documents apart — and a recent document can be swiped off with an undo.
RecentDocumentListgrows aninsert()next toadd()rather than reusing it.prune()reclaims it on the next launch anyway — which is what reconciling against the stored lists, rather than bookkeeping every removal, was for.A "dark documents" toggle was built here and taken back out
It is the one item from the plan that is not shipping. With the html odrcore generates, algorithmic darkening changes nothing visible — verified on device with the setting on, in night mode, including after forcing an activity recreation to rule out timing. The switch would have looked broken. Making it work needs a dark stylesheet from the core, not a toggle here.
PageView.toggleDarkModekeeps the parameter as the hook.Verification
20 instrumented tests, unit tests, lint and both flavours green. On device: swiped the middle row away, confirmed it left the list and the json, tapped Undo, confirmed it came back between the other two rather than on top.