Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Role Diff Visual to Role editor #51483

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add Role Diff Visual to Role editor #51483

wants to merge 2 commits into from

Conversation

avatus
Copy link
Contributor

@avatus avatus commented Jan 24, 2025

This will enable a role diff viewer from Teleport Policy if the client has Teleport policy enabled

Screen.Recording.2025-01-24.at.4.31.23.PM.mov

TODO
Add a few more tests, however, most of the work is from the Diff component which we get pulled in from "over the wire" from TAG so, testing won't work too well there. There are already tests added to test the RoleDiffComponent and role editor conditional rendering so this PR is mostly just using existing work.

This will enable a role diff viewer from Teleport Policy if the client
has Teleport policy enabled
@avatus avatus added the no-changelog Indicates that a PR does not require a changelog entry label Jan 24, 2025
@avatus avatus requested review from ryanclark and bl-nero January 24, 2025 22:34
@avatus avatus removed the request for review from flyinghermit January 24, 2025 22:36
@@ -50,8 +50,9 @@ import { State, useRoles } from './useRoles';

// RoleDiffProps are an optional set of props to render the role diff visualizer.
type RoleDiffProps = {
RoleDiffComponent: ComponentType;
updateRoleDiff: (role: Role) => Promise<void>;
RoleDiffComponent: React.ReactElement;
Copy link
Contributor

Choose a reason for hiding this comment

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

As this is now an element, normally the prop doesn't start with a capital letter

For example, react-error-boundary's props

image

(Also I think this should be a ReactNode - react-error-boundary and React.PropsWithChildren use ReactNode)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-changelog Indicates that a PR does not require a changelog entry size/sm ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants