Conversation
…6.0.0 pane-registry@5.0.1)
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The focused fix correctly uses the existing drag helper and includes comprehensive regression coverage.
Review effort: Balanced
Findings: None
What changed in this PR
Enables dragging “Everyone” directly to an ACL access level without dropping the globe icon URL.
Changes:
- Makes the button draggable as
foaf:Agent. - Disables native image dragging.
- Adds regression tests for drag-and-drop and click behavior.
| File | Description |
|---|---|
src/acl/add-agent-buttons.ts |
Configures semantic dragging for “Everyone.” |
test/unit/acl/add-agent-buttons.test.ts |
Tests URI-list, plain-text, and click workflows. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
A perhaps silly question — does "Everyone" in this context equate to World access? to all users known to the Solid system? to something else? Whatever the answer, I strongly advise that it be provided via mouseover help text, at the least. If I wonder at this, others will (or should!)... |
|
Everyone means really everyone. It means that the quoted ACL permissions applies to everyone (world access with the described permissions). |
Dragging the Add Everyone globe directly from the sharing chooser currently drops the SVG URL. The ACL editor then tries to load that image as an RDF access target, which can produce a GitHub Pages CORS preflight error and prevents the permission change. This matches the recent report in the solidcommunity.net channel.
Make the button draggable through the existing
makeDraggablehelper usingfoaf:Agent, and disable native dragging on its child image. Users can drop Everyone directly onto the desired access level; clicking to add Everyone as a Viewer still works. No server CORS or authentication changes are needed.Fixes #448. Fixes #419.
Validation:
https://solidos.github.io/solid-ui/src/icons/noun_98053.svg; the patched button dropshttp://xmlns.com/foaf/0.1/Agent.NODE_OPTIONS=--no-experimental-webstorage npm test -- --maxWorkers=2: 660 passed, 46 skipped, including test type checking. On this Node 25.1.0 workstation, the unmodified test command hits three login-component failures becauselocalStorage.clearis unavailable; disabling Node's experimental web storage lets jsdom provide it. CI uses Node 22/24.npm run build: passed, including source type checking, library/CDN bundles and Storybook.git diff --check: passed.