feat(ui): surface helper state and actions - #670
feat(ui): surface helper state and actions#670Alex Shabalin (alex-sparus) wants to merge 1 commit into
Conversation
This stack of pull requests is managed by jj-ryu. |
🎨 Storybook previewUpdated for cc4aa49
|
📋 PR Overview
🔬 Coverage
|
db02c9d to
c39e869
Compare
41b4d09 to
70e5e87
Compare
c39e869 to
34cf6b7
Compare
70e5e87 to
5113963
Compare
34cf6b7 to
ba85203
Compare
5113963 to
0261bfa
Compare
ba85203 to
bdb2c00
Compare
0261bfa to
f85c545
Compare
bdb2c00 to
527369e
Compare
f85c545 to
bef7da4
Compare
527369e to
07dfb5b
Compare
bef7da4 to
9230f43
Compare
07dfb5b to
0f17cea
Compare
9230f43 to
a1a17a5
Compare
| // The condition is exactly that: the standing decision is `granted` and the row | ||
| // is not. It covers a registration waiting for approval in Login Items, a copy | ||
| // of nixmac running from a folder macOS will not register, and a register that | ||
| // failed — and it deliberately cannot tell them apart, because the row carries |
There was a problem hiding this comment.
Is there any way to change this, it doesn't seem very user-friendly.
There was a problem hiding this comment.
Hmm, to change what? This is just a missing required permissions banner - just like full-disk access or any other permissions that the app requires and the user might disable at some point
| onClick: () => void; | ||
| }; | ||
|
|
||
| /** |
There was a problem hiding this comment.
Some of these (I suspect AI-generated) comments are overly verbose and potentially fragile (for example, specific classnames in the comments, over-explaining specifics of how the UX works) and should be cut back, seems particularly the case in the frontend code.
There was a problem hiding this comment.
Fair. Kept only the regression-preventing facts (the design-system button resize/animation gotchas, the live-vs-snapshot rule) and dropped the classname walkthroughs and UX narration.
The buttons in the UI really need some refactoring, I have it noted for fixing later - there are several copies by now with various fixes
| try { | ||
| const report = await client.darwin.helperDisable(); | ||
| setNotice({ tone: "info", message: report.detail }); | ||
| // deprecated(orpc): replace with client/orpc from @/lib/orpc |
There was a problem hiding this comment.
We should probably start addressing these in new code.
There was a problem hiding this comment.
100%, the two new-code sites now use the oRPC client; the remaining marked sites in these files predate this PR
| variant: "secondary", | ||
| }; | ||
| } | ||
| if (perm.id === "privileged-helper") { |
There was a problem hiding this comment.
I see a const for this value elsewhere that should probably be used as long as there's not an enum.
There was a problem hiding this comment.
Added a const
| * The one permission nixmac installs itself instead of asking macOS for, and so | ||
| * the one row whose state nixmac keeps working on after the probe returns. | ||
| */ | ||
| export const HELPER_PERMISSION_ID = "privileged-helper"; |
There was a problem hiding this comment.
Back-reference to another comment -- there are several places this could/should be used but isn't, I believe.
| import { computeRepairPlan, type RepairInputs } from "./lib"; | ||
|
|
||
| /** What the backend reports while macOS holds the registration for approval. */ | ||
| const APPROVE_IN_LOGIN_ITEMS = |
There was a problem hiding this comment.
Generally unless it's otherwise important I'd make a less-tight assertion so that there's options for varying the text without having to duplicate the change int he test code.
There was a problem hiding this comment.
The sentence is now declared once in utils/test-fixtures, kept the exact assertion
| RestartSetupConfirmation: () => null, | ||
| })); | ||
|
|
||
| const APPROVE_IN_LOGIN_ITEMS = |
There was a problem hiding this comment.
This is duplicative of repair.test.tsx.
There was a problem hiding this comment.
Fixed
0f17cea to
74e888d
Compare
a1a17a5 to
9727da9
Compare
74e888d to
8b6f076
Compare
9727da9 to
e4795b0
Compare
8b6f076 to
74c824f
Compare
e4795b0 to
05fdc6f
Compare
05fdc6f to
a673e07
Compare
Problem: none of the helper machinery is visible or drivable — no way to grant or disable, no sign that macOS is waiting in Login Items or that an upgrade is waiting out a running activation — and the repair banner evaluates once per launch, so it keeps warning about a helper the convergence loop fixed seconds later. Solution: orpc endpoints for status, grant, and disable; one permission row produced whole from each reconciliation report, with replace_row letting the convergence loop publish updates without running a second reconciliation; the repair banner follows live helper state instead of the launch snapshot. The UI never re-words or re-classifies a report — rows render the backend sentence verbatim, and the TS test fixtures mirror the backend vocabulary exactly.
74c824f to
7de50ac
Compare
a673e07 to
cc4aa49
Compare
2 participants
Summary
Problem: none of the helper machinery is visible or drivable — no way to grant or disable, no sign that macOS is waiting in Login Items or that an upgrade is waiting out a running activation — and the repair banner evaluates once per launch, so it keeps warning about a helper the convergence loop fixed seconds later.
Solution: orpc endpoints for status, grant, and disable; one permission row produced whole from each reconciliation report, with
replace_rowletting the convergence loop publish updates without running a second reconciliation; the repair banner follows live helper state instead of the launch snapshot. The UI never re-words or re-classifies a report — rows render the backend sentence verbatim, and the TS test fixtures mirror the backend vocabulary exactly.Stacked on #669; completes the helper-redesign stack.
Test Plan
cargo testgreen (row/report mapping, permissions state)bunx tsc --noEmit && bun run test:unit(panel + repair suites; fixtures assert the backend sentences verbatim)Docs
Prior review
Nearly all of this PR matches content from #636:
repair.tsx/lib.ts/types.ts/permissions_state.rs/orpcbyte-identical,permissions-panel.tsxdiffers by 2 lines, test fixtures updated to the new backend sentences. Caveat: Scott's #636 approval predates the convergence-loop and banner additions that landed on that branch afterwards, so those parts (mostly in #669'shelper_permission.rs) were never formally approved.