Add the options panel to role editor #49801
Conversation
bl-nero
added
no-changelog
Can you add a story for this UI? will make it easy for reviewers to play around without spinning the dev UI and setting up all the dependencies. |
Also tightens some constraints about standard role editor conformance.
bl-nero
force-pushed
the
bl-nero/role-editor-8
branch
from
4d4eaa6 to
0bacc2f
Compare
bl-nero
force-pushed
the
bl-nero/role-editor-9
branch
from
7b4bd84 to
a1b1c2e
Compare
|
@flyinghermit It's already there, just go to https://localhost:9002/?path=/story/teleport-roles-role-editor--new-role and go to the Options panel. |
|
Thanks for the link, that is helpful. Can you also put that link to storybook in your PR description? Its hard to guess otherwise when the PR itself does not touch on the story :) |
| <H4 | ||
| css={` | ||
| grid-column: 1/3; | ||
| border-top: ${theme.borders[1]} | ||
| ${theme.colors.interactive.tonal.neutral[0]}; | ||
| padding-top: ${theme.space[3]}px; | ||
| `} | ||
| > | ||
| SSH | ||
| </H4> |
There was a problem hiding this comment.
Seeing this H4 grid repeated with only the text values (SSH,Database, Desktop etc) being unique, I think you will gain by programmatically creating each row instead of with hand. Given the scope of our role option, this will likely grow a larger list and I think will be cleaner that way.
| desktopClipboard: true, | ||
| createDesktopUser: false, | ||
| desktopDirectorySharing: true, | ||
| }, |
There was a problem hiding this comment.
The SAML IdP option is not included this time?
There was a problem hiding this comment.
We are supporting an arbitrary subset of options for now, so no, it's not included.
| <Box | ||
| border={1} | ||
| borderColor={theme.colors.interactive.tonal.neutral[0]} | ||
| borderRadius={3} | ||
| p={3} | ||
| css={` | ||
| display: grid; | ||
| grid-template-columns: 1fr 1fr; | ||
| align-items: baseline; | ||
| row-gap: ${theme.space[3]}px; | ||
| `} | ||
| > |
There was a problem hiding this comment.
There is a lot of mixing of Styled components, style prop, css attributes, and then standard css in the css prop? Is there a particular reason you used all 4 instead of picking one? I know genreally the css attributes are used for 1 offs or what not, but this seems to be all over the place.
I think the preferred way is a styled component (at least, I've received comments as such), but I don't care which way you do it here as long as its consistent.
There was a problem hiding this comment.
Oh, this one. Yes, that was just me being lazy, I forgot to refactor it before submitting.
| grid-column: 1/3; | ||
| border-top: ${theme.borders[1]} | ||
| ${theme.colors.interactive.tonal.neutral[0]}; | ||
| padding-top: ${theme.space[3]}px; | ||
| `} |
There was a problem hiding this comment.
This style seems to match the other H4 in the component, lets move it to a single styled component so we don't risk missing style updates if we need to change it
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
* Add admin rule tab to the role editor Also tightens some constraints about standard role editor conformance. * Add a way to delete an admin rule * Add the options panel to role editor * Review * Review
To turn on the new UI, go to developer tools -> Application -> Local storage and add a
grv_teleport_use_new_role_editorkey set totrue. This will be lifted once UI is good to be released.Figma
Story: https://localhost:9002/?path=/story/teleport-roles-role-editor--new-role
Deviations from the design:
Requires #49764
Contributes to #46612