Skip to content

Feat/access input - #882

Open
SharonStrats wants to merge 4 commits into
stagingfrom
feat/access-input
Open

SharonStrats wants to merge 4 commits into
stagingfrom
feat/access-input

Conversation

@SharonStrats

@SharonStrats SharonStrats commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

This was for the input ticket, the add at the top. I used the solid-ui-combobox.
This relies on solid-logic SolidOS/solid-logic#336

Some notes:

  • I didn't want to configure the combobox right now, I thought I would leave it for discussion. Something you will find is the ^ on the end of the input for instance which is not in the design.
  • the font from the design for the placeholder was so light I couldn't read it so I left it.
  • In the design it says to use commas to add multiples, but that isn't how it actually works now with the search, you pick one and it adds it below when you select it. You can also copy in a url and it will load it and present it in the search so you can select it. If the profile can't be loaded it says not found.
  • I created a new role called 'No Access' instead of 'Restricted' as I had recommended in a previous comment on an issue I believe. I felt like it is more explicit. In order to keep things consistent on the roles across all, in solid-logic it's called 'No Access'. In the shared with area, I just rename it to say 'Remove'. And in this top section I do not display the Role since it really doesn't make sense when you are adding.

Here is a screenshot
Screenshot 2026-10-05 at 11 33 00 AM

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Search options, multi-value selection, asynchronous input handling, and existing tests have unresolved regressions.

Review effort: Balanced
Findings: 3 High severity · 2 Medium severity

Open (5)
What changed in this PR

Adds directory-backed principal selection and pending access grants to the access-control modal.

Changes:

  • Adds searchable principal and grant comboboxes.
  • Introduces pending-grant chips and label resolution.
  • Updates styling, theme colors, and screen-reader label bindings.
File Description
src/​styles/​theme.css Adds gray theme tokens.
src/​components/​select/​Select.stories.ts Fixes label property binding.
src/​components/​input/​Input.stories.ts Fixes label property binding.
src/​components/​combobox/​Combobox.stories.ts Fixes label property bindings.
src/​components/​access-control-modal/​AccessControlModal.ts Implements access search and pending grants.
src/​components/​access-control-modal/​AccessControlModal.styles.css Styles the revised modal interface.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/components/access-control-modal/AccessControlModal.ts Outdated
Comment thread src/components/access-control-modal/AccessControlModal.ts
Comment thread src/components/access-control-modal/AccessControlModal.ts Outdated
Comment thread src/components/access-control-modal/AccessControlModal.ts Outdated
Comment thread src/components/access-control-modal/AccessControlModal.ts Outdated
@SharonStrats SharonStrats linked an issue Oct 5, 2026 that may be closed by this pull request
@timea-solid

Copy link
Copy Markdown
Member

Hei your decisions make colplete sense to me. I like tge No access and how the combo box works, actually design wide itnis more consistent and better overall.

@timea-solid timea-solid left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job and thanks dmfir addint tests too!

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: In review

Development

Successfully merging this pull request may close these issues.

Sharing pane - the Enter name and link input and roles

3 participants