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

Role editor: hierarchical validation and tabs #49546

Open
wants to merge 7 commits into
base: bl-nero/slidetabs
Choose a base branch
from

Conversation

bl-nero
Copy link
Contributor

@bl-nero bl-nero commented Nov 28, 2024

This PR moves the role editor UI to a two-level tab hierarchy. It also uses a new approach to validation, which is necessary, because the design requires errors to be visible on multiple levels of the hierarchy. New validation rules are also added to bring the UI up to reasonable parity with the backend requirements.

To turn on the new UI, go to developer tools -> Application -> Local storage and add a grv_teleport_use_new_role_editor key set to true. This will be lifted once UI is good to be released.

Demo

Requires #49545
Contributes to #46612

@bl-nero bl-nero added the no-changelog Indicates that a PR does not require a changelog entry label Nov 28, 2024
@bl-nero bl-nero marked this pull request as ready for review November 28, 2024 15:18
};

export type KubernetesResourceKind =
Copy link
Contributor

Choose a reason for hiding this comment

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

is there some backend equivalent that we are keeping this in line with? Could we maybe add a comment linking to it?

<H2>{isCreating ? 'Create a New Role' : role?.metadata.name}</H2>
<Flex alignItems="center" mb={3} gap={2}>
<Box flex="1">
<H2>{isCreating ? 'Create a New Role' : role?.metadata.name}</H2>
Copy link
Contributor

Choose a reason for hiding this comment

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

Mentioning "Edit Role" is not necessary for editing? Edit Role {role?.metadata.name}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, this may be clearer.

isProcessing={isProcessing}
onChange={setStandardModel}
/>
<div id={standardEditorId}>
Copy link
Contributor

Choose a reason for hiding this comment

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

why is <div id={standardEditorId} needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are used to associate the tab switcher tabs with the switched panels. I'm adding a comment.

};
export type WindowsDesktopSpecValidationResult = RuleSetValidationResult<
typeof windowsDesktopSpecValidationRules
>;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: all the rules functions in this file would have a better home in a separate rule.ts file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I agree. The only reason I didn't make it is that I'm constantly in progress with more PRs, so after the version cut, I'm planning to split both the StandardEditor file, as well as this one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(Oh, wait. This is actually new code. It can be easily extracted.)

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

Successfully merging this pull request may close these issues.

3 participants