-
Notifications
You must be signed in to change notification settings - Fork 15
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
fix(KFLUXUI-260): It's unclear to users from representation in UI that components must be unique per workspace #93
base: main
Are you sure you want to change the base?
Conversation
…for component name
…ed while creating application
src/components/ImportForm/ComponentSection/ComponentSection.tsx
Outdated
Show resolved
Hide resolved
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #93 +/- ##
==========================================
+ Coverage 80.11% 80.14% +0.02%
==========================================
Files 570 570
Lines 21452 21542 +90
Branches 5050 5078 +28
==========================================
+ Hits 17187 17265 +78
- Misses 4241 4252 +11
- Partials 24 25 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
LGTM
<HelperText> | ||
<HelperTextItem> | ||
Must be unique within tenant namespace{' '} | ||
<span className="component-section-helper-text-namespace-highlighted">{namespace}</span> | ||
</HelperTextItem> | ||
</HelperText> |
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.
use input field prop helperText to set the helper text,
.component-section-helper-text-namespace-highlighted{ | ||
font-weight: 800; | ||
} |
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.
missing new line
.component-section-helper-text-namespace-highlighted{ | |
font-weight: 800; | |
} | |
.component-section { | |
&__component-name { | |
&--helper-text { | |
font-weight: var(--pf-v5-global--FontWeight--bold); | |
} | |
} | |
} |
if ( | ||
error?.json?.reason === 'AlreadyExists' && | ||
error?.json?.details?.kind === 'components' | ||
) | ||
formikHelpers.setStatus({ | ||
submitError: `Component "${error.json?.details?.name}" already exists in this namespace. Edit the name to be unique and try again.`, | ||
}); | ||
else formikHelpers.setStatus({ submitError: error.message }); |
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.
This seems redundant, you added helper text indicating name should be unique in namespace. We should just show the error message returned by the api
Fixes
https://issues.redhat.com/browse/KFLUXUI-260)
Description
Helper text is added for user conveying component names should be unique within tenant's namespace.
Helper Text:** Must be unique within tenant namespace **
When creating application, if component name is duplicate then error message is updated to following message:
"Component already exists in this namespace. Edit the name to be unique and try again."
Unique default component name suggested after adding a URL while creating an application.
If same component name exists within namespace, then suffix "-[Ramndom Number]" is added.
Example: Component "sample-component" already exists within namespace, so suggested default component added is
"sample-component-50". Screenshot is also attached.
Type of change
Screen shots / Gifs for design review
3.Unique Default Component name is suggested