-
Notifications
You must be signed in to change notification settings - Fork 43
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
👻 Adds colors to drawer tags & removes boilerplate #1481
👻 Adds colors to drawer tags & removes boilerplate #1481
Conversation
24f218a
to
e0c71ec
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1481 +/- ##
=======================================
Coverage 40.16% 40.16%
=======================================
Files 144 144
Lines 4551 4551
Branches 1096 1096
=======================================
Hits 1828 1828
Misses 2626 2626
Partials 97 97
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.
I like the structure of the change. Only a few comments.
const getRandomColor = () => { | ||
const letters = "0123456789ABCDEF"; | ||
let color = "#"; | ||
for (let i = 0; i < 6; i++) { | ||
color += letters[Math.floor(Math.random() * 16)]; | ||
} | ||
return color; | ||
}; |
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.
There is an alternative that looks like it can generate some better colors (like keeping super dark and super light colors out): https://gist.github.com/bendc/76c48ce53299e6078a76
...s/applications/components/application-detail-drawer/application-detail-drawer-assessment.tsx
Outdated
Show resolved
Hide resolved
client/src/app/components/labels-from-items/labels-from-items.tsx
Outdated
Show resolved
Hide resolved
client/src/app/components/labels-from-items/labels-from-items.tsx
Outdated
Show resolved
Hide resolved
e0c71ec
to
69cca76
Compare
Signed-off-by: ibolton336 <ibolton@redhat.com>
69cca76
to
dcf3056
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.
Looks great, just one last change and one nit-pick
}): JSX.Element { | ||
const { t } = useTranslation(); | ||
|
||
if (items?.length === 0) { |
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.
If items
is undefined, this will reduce to undefined === 0
which is false
.
Try if (items?.length ?? 0 === 0) {
to force a 0 if items
is undefined.
client/src/app/pages/archetypes/components/archetype-detail-drawer.tsx
Outdated
Show resolved
Hide resolved
Signed-off-by: Ian Bolton <ibolton@redhat.com>
Signed-off-by: Ian Bolton <ibolton@redhat.com>
Follow up to: #1470