Limited collaborator UI #2960
Limited collaborator UI #2960
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
the div wrapping the radios was messing up the way RadioFieldDyn passes checked state to its child radios because they're expected to be direct children
david-crespo
force-pushed
the
limited_collaborator
branch
from
6e0af82 to
4b3731d
Compare
|
This should be a regular modal rather than a side modal, since it's a single input / action. Spacing seems inconsistent here, not sure why. I can take a quick look at the design now. |
Okay; just for clarity, does the "Add user" modal remain a side modal, because you have to enter the name and select the role, or is it conceptually a single input and should be a regular modal? |
|
I could go either way, my initial designs had it as a side modal but that's because I had the ability to add multiple users in one go. It's sort of a fuzzy rule based on length. Also, I could see the edit role being part of a side modal but only if it was part of a broader edit user / user view modal. 
Let's use a regular modal for both for now. |
| control={control} | ||
| disabled={isSubmitting} | ||
| > | ||
| <RadioFieldDyn name="presetId" control={control} disabled={isSubmitting}> |
There was a problem hiding this comment.
these were the only other RadioFieldDyn and they were using label="" as a hack around the fact there was a default value if label wasn't passed. That's silly, so I got rid of the default value and stopped passing a label here.
| return ( | ||
| <div> | ||
| <div className="mb-2"> | ||
| <div className="mb-3"> |
There was a problem hiding this comment.
The fact that the only existing callers don't pass a label means that I can change the spacing here without consequence. Looks fine. I considered mb-4 but it was slightly too much. I think the real issue here is that the label and the values have the same treatment, so there's no sense of hierarchy. A problem for another time.
|
Final copy: 
|
Now that oxidecomputer/omicron#9299 is in Omicron, we want to use the new permissions in the web console as well. This PR bumps Omicron (will want to bump again before merging this) and also redesigns the access forms to use radio buttons, which allow us to show all options on the screen, with descriptions of what each role enables.