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

Merged
merged 28 commits into from
Dec 5, 2024
Merged

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

Adds a model-level validation capability to our validation library.
They will be later required in the SlideTabs component
Add support for tooltips and status icons
@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.)

Base automatically changed from bl-nero/slidetabs to master December 5, 2024 11:46
@bl-nero bl-nero enabled auto-merge December 5, 2024 13:06
@bl-nero bl-nero added this pull request to the merge queue Dec 5, 2024
Merged via the queue into master with commit e757b69 Dec 5, 2024
40 checks passed
@bl-nero bl-nero deleted the bl-nero/role-editor-7 branch December 5, 2024 13:22
@public-teleport-github-review-bot

@bl-nero See the table below for backport results.

Branch Result
branch/v17 Failed

bl-nero added a commit that referenced this pull request Dec 6, 2024
* Validation

Adds a model-level validation capability to our validation library.

* Move tooltips to the design package

They will be later required in the SlideTabs component

* SlideTabs improvements

Add support for tooltips and status icons

* Role editor: hierarchical validation and tabs

* Review

* Also, rename the tooltip directory

* Fix an import

* Review: status-related props, extract StatusIcon

* Update after changing the SlideTabs interface

* Also, rename the tooltip component

* review

* review

* REview

* Never return undefined from useValidation()
github-merge-queue bot pushed a commit that referenced this pull request Dec 9, 2024
* Validation

Adds a model-level validation capability to our validation library.

* Move tooltips to the design package

They will be later required in the SlideTabs component

* SlideTabs improvements

Add support for tooltips and status icons

* Role editor: hierarchical validation and tabs

* Review

* Also, rename the tooltip directory

* Fix an import

* Review: status-related props, extract StatusIcon

* Update after changing the SlideTabs interface

* Also, rename the tooltip component

* review

* review

* REview

* Never return undefined from useValidation()
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