Maange Access Dialog - #872
SharonStrats wants to merge 6 commits into
Conversation
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The save flow is incomplete, and several rendering, accessibility, and integration issues remain.
Review effort: Lite
Findings: 1
Open (7)
Save handler discards changes and leaves submitting state active · New Globe icon selector targets the wrong custom element · New Component imports utility through package root, causing bundle coupling · New Missing import prevents globe icon custom element registration · New Origin class subjects are missing from access holder rendering · New Search control lacks an accessible label · New Empty subject URI produces an incomplete dialog title · New
What changed in this PR
Adds an access-control sharing dialog with supporting input, combobox, theme, and custom-element updates.
Changes:
- Adds access-control modal rendering, types, exports, and styles.
- Adds input left-icon support and configurable borders.
- Adds combobox border styling and theme color variables.
| File | Description |
|---|---|
src/types/custom-elements.d.ts |
Registers modal element types. |
src/styles/theme.css |
Adds theme color variables. |
src/components/input/Input.ts |
Adds left-icon slot support. |
src/components/input/Input.styles.css |
Styles icons and configurable borders. |
src/components/combobox/Combobox.styles.css |
Adds configurable border styling. |
src/components/access-control-modal/types.ts |
Defines access-control types. |
src/components/access-control-modal/index.ts |
Exports the modal component. |
src/components/access-control-modal/AccessControlModal.ts |
Implements the access-control dialog. |
src/components/access-control-modal/AccessControlModal.styles.css |
Styles the dialog UI. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
aef29ff to
d74eb3d
Compare
…istry@5.0.0-1) (latest: rdflib@2.4.1)
NoelDeMartin
left a comment
There was a problem hiding this comment.
After a quick review I think it's ok, the only comment is that maybe we shouldn't customize the design system components unless we have a good reason to do it.
| @@ -1,4 +1,6 @@ | |||
| :host { | |||
| --solid-ui-input-border-color: var(--solid-ui-color-gray-400); | |||
There was a problem hiding this comment.
You've mentioned in the PR description that you "needed" to change the border color, but why is that? If that's because the design in Figma has a different border for this selector, maybe the Figma is wrong. I don't see a reason to customize the combobox border color. As I always say, it's not impossible to create some UI outside of the design system, but we should have a very good reason to do it.
| accessor label = '' | ||
|
|
||
| @property({ type: Boolean, reflect: true, attribute: 'hide-label' }) | ||
| accessor hideLabel = false |
There was a problem hiding this comment.
Maybe instead of saying "hideLabel", we should call this "srOnlyLabel" or something. Hiding can be interpreted as both for sighted users and screen readers. "srOnly" makes it clear that the label is still visible for screen readers.



Keep in mind this is for the UI structure ticket only, it doesn't work yet.
Also included are some existing web component configuration
Note: I tried to only configure what I thought were essentials, but you may notice the Copy Link button doesn't match the design, to make it smaller I will need to add configuration for font in the solid-ui-button.
Needs solid-logic changes.