-
Notifications
You must be signed in to change notification settings - Fork 111
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
Infra proxy edit mode - added some UI improvements #4888
Conversation
Deploy preview for chef-automate processing. Building with commit 9b844d4 https://app.netlify.com/sites/chef-automate/deploys/606bcd800e4e490007a6da8a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me
@@ -36,7 +36,7 @@ <h2 slot="title">Create Environment</h2> | |||
</div> | |||
<div class="button-bar-box"> | |||
<chef-button tertiary (click)="closeCreateModal()">Cancel</chef-button> | |||
<chef-button [disabled]="!detailsFormGroup?.valid || conflictError" primary id="create-button-object-modal" (click)="createEnvironment()"> | |||
<chef-button [disabled]="!detailsFormGroup?.valid || conflictError || nameExist" primary id="create-button-object-modal" (click)="createEnvironment()"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
excellent - thank you for checking against existing names
...nts/automate-ui/src/app/modules/infra-proxy/infra-search-bar/infra-search-bar.component.scss
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this @vinay033!
Many changes have been made to the styling of similar files in #4875. Please have a look at this PR and lets plan to merge #4875 before this one.
Change Request
In the Create Environment Modal, when a user adds a "space" in front of the desired name, it is seen as a new name. This should be trimmed in some fashion when checking for same name.
To reproduce
Navigate to an organization > environments > click "Create Environment"
Make a new environment with any name (ex. "mytest")
Make a second new environment using same name plus a space (ex. " mytest")
The warning goes away and you are allowed to create. Thankfully, the backend does not let a user create this name, but we should handle it on the front end as well.
...s/automate-ui/src/app/modules/infra-proxy/data-bags-details/data-bags-details.component.scss
Outdated
Show resolved
Hide resolved
eaa44a0
to
9464c51
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job Vinay! π
9464c51
to
ea6ebd7
Compare
@vinay033 I rebased and corrected the merge conflicts and will merge as soon as tests pass π |
Signed-off-by: Vinay Sharma <vsharma@chef.io>
Signed-off-by: Vinay Sharma <vsharma@chef.io>
Signed-off-by: Vinay Sharma <vsharma@chef.io>
Signed-off-by: seajamied <jdegnan@chef.io>
ea6ebd7
to
9b844d4
Compare
Signed-off-by: Vinay Sharma vsharma@chef.io
π© Description: What code changed, and why?
we need to fix the below-mentioned points
βοΈ Related Resources
#4873
π Definition of Done
I have added some changes for the UI improvements.
π How to Build and Test the Change
To add data https://github.com/chef/automate/blob/master/dev-docs/adding-data/adding_test_data.md#adding-data-to-infra-views
β Checklist
π· Screenshots, if applicable