feat: add icon pack #114
Merged
feat: add icon pack #114
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Lukas.J.Han <lukas.j.han@gmail.com>
Signed-off-by: Lukas.J.Han <lukas.j.han@gmail.com>
Signed-off-by: Lukas.J.Han <lukas.j.han@gmail.com>
Reviewer's Guide by SourceryThis pull request introduces a new icon library, No diagrams generated as the changes look simple and do not need a visual representation. File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @lukasjhan - I've reviewed your changes - here's some feedback:
Overall Comments:
- The storybook example includes a copy to clipboard feature, which is a nice touch for developers using the library.
Here's what I looked at during the review
- 🟡 General issues: 3 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| xmlns="http://www.w3.org/2000/svg" | ||
| className={className} | ||
| > | ||
| <g clip-path="url(#clip0_887_44026)"> |
There was a problem hiding this comment.
suggestion: Convert SVG attribute names to camelCase for JSX compatibility.
Many SVG attributes such as "clip-path", "fill-rule", "stroke-width", and others should use camelCase (e.g. clipPath, fillRule, strokeWidth) to avoid potential issues and align with React's naming conventions.
Suggested change
| <g clip-path="url(#clip0_887_44026)"> | |
| <g clipPath="url(#clip0_887_44026)"> |
Comment on lines
+19
to
+22
| fill-rule="evenodd" | ||
| clip-rule="evenodd" | ||
| d="M11.0194 1.6001C10.656 1.6001 10.3382 1.84505 10.2458 2.1965L9.73932 4.1209L8.02834 4.84214L6.76933 3.72233C6.45276 3.44076 5.97155 3.45483 5.67197 3.75441L3.75197 5.67441C3.46209 5.96429 3.43825 6.42638 3.69675 6.74456L4.7935 8.09446L4.09021 9.82359L2.21584 10.261C1.85378 10.3455 1.59766 10.6683 1.59766 11.0401V12.9601C1.59766 13.3208 1.83901 13.6369 2.18697 13.7319L4.11668 14.2587L4.83925 15.9728L3.72049 17.2277C3.43833 17.5443 3.45214 18.026 3.75197 18.3258L5.67197 20.2458C5.96217 20.536 6.4249 20.5595 6.74304 20.3002L8.09332 19.1998L9.78179 19.8945L10.262 21.796C10.3517 22.1512 10.6713 22.4001 11.0377 22.4001H12.9577C13.3238 22.4001 13.6433 22.1515 13.7332 21.7965L14.2149 19.8943L15.9552 19.1736C16.1327 19.3208 16.3451 19.5029 16.5539 19.6846C16.7343 19.8417 16.902 19.9897 17.0247 20.0985C17.0859 20.1529 17.1359 20.1973 17.1704 20.2281L17.2235 20.2757C17.5399 20.5594 18.0229 20.5463 18.3233 20.2458L20.2433 18.3258C20.5378 18.0314 20.5571 17.5604 20.2879 17.2428L19.1788 15.9346L19.8938 14.2081L21.799 13.7124C22.1515 13.6207 22.3975 13.3025 22.3976 12.9382L22.3977 11.0401C22.3977 10.6742 22.1494 10.3549 21.7948 10.2648L19.9009 9.78316L19.187 8.05939L20.2885 6.75663C20.5571 6.439 20.5375 5.96854 20.2433 5.67441L18.3233 3.75441C18.0281 3.45914 17.5554 3.44064 17.2379 3.71192L15.9438 4.81786L14.1657 4.08663L13.6693 2.19684C13.5769 1.84523 13.2591 1.6001 12.8955 1.6001H11.0194ZM11.1865 4.90878L11.6361 3.2001H12.2786L12.7194 4.8783C12.7831 5.12088 12.9569 5.31953 13.1889 5.41492L15.7875 6.48361C16.0651 6.59778 16.3833 6.54693 16.6115 6.35191L17.715 5.40885L18.5917 6.28546L17.6505 7.39857C17.4572 7.62719 17.4077 7.9446 17.5222 8.2212L18.576 10.7658C18.6726 10.9989 18.8735 11.1728 19.118 11.235L20.7976 11.6621L20.7976 12.3197L19.1056 12.76C18.8629 12.8231 18.6638 12.9964 18.5679 13.2281L17.5138 15.7734C17.3991 16.0504 17.4489 16.3682 17.6427 16.5968L18.591 17.7154L17.7241 18.5823L17.6044 18.4778C17.248 18.1676 16.7976 17.7828 16.5478 17.6041C16.3231 17.4436 16.0316 17.4102 15.7765 17.5159L13.2319 18.5696C12.9985 18.6663 12.8245 18.8675 12.7625 19.1124L12.335 20.8001H11.6607L11.2344 19.112C11.1724 18.8663 10.9975 18.6645 10.7631 18.5681L8.26153 17.5389C7.99009 17.4273 7.67931 17.4732 7.45178 17.6586L6.2924 18.6035L5.4175 17.7286L6.36994 16.6602C6.57539 16.4297 6.62989 16.1016 6.50996 15.8171L5.4399 13.2786C5.34449 13.0523 5.15038 12.8823 4.91341 12.8176L3.19766 12.3492V11.6749L4.85708 11.2876C5.10996 11.2286 5.31847 11.0505 5.4163 10.81L6.45394 8.2588C6.56393 7.98838 6.51788 7.67951 6.3338 7.45293L5.39351 6.29561L6.26979 5.41933L7.34222 6.37319C7.57267 6.57816 7.90043 6.6324 8.18464 6.5126L10.7235 5.44236C10.9523 5.34595 11.1233 5.14883 11.1865 4.90878ZM11.9992 8.2002C9.90049 8.2002 8.19922 9.90147 8.19922 12.0002C8.19922 14.0989 9.90049 15.8002 11.9992 15.8002C14.0979 15.8002 15.7992 14.0989 15.7992 12.0002C15.7992 9.90147 14.0979 8.2002 11.9992 8.2002ZM9.79922 12.0002C9.79922 10.7851 10.7841 9.8002 11.9992 9.8002C13.2143 9.8002 14.1992 10.7851 14.1992 12.0002C14.1992 13.2153 13.2143 14.2002 11.9992 14.2002C10.7841 14.2002 9.79922 13.2153 9.79922 12.0002Z" | ||
| fill={color} |
There was a problem hiding this comment.
suggestion: Rename hyphenated SVG attributes to camelCase
To align with React’s recommended attribute naming, consider renaming 'fill-rule' to 'fillRule' and 'clip-rule' to 'clipRule'. This change applies to the SVG elements in all icons.
Suggested implementation:
fillRule="evenodd" clipRule="evenodd"
Comment on lines
+10
to
+16
| <svg | ||
| width={width} | ||
| height={height} | ||
| viewBox="0 0 24 24" | ||
| fill="none" | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| className={className} |
There was a problem hiding this comment.
suggestion: Enhance SVG accessibility attributes
If these icons are purely decorative, adding an attribute like 'aria-hidden="true"' can improve accessibility. Otherwise, consider including proper roles and accessible labels.
Suggested change
| <svg | |
| width={width} | |
| height={height} | |
| viewBox="0 0 24 24" | |
| fill="none" | |
| xmlns="http://www.w3.org/2000/svg" | |
| className={className} | |
| <svg aria-hidden="true" | |
| width={width} | |
| height={height} | |
| viewBox="0 0 24 24" | |
| fill="none" | |
| xmlns="http://www.w3.org/2000/svg" | |
| className={className} |
| overflow: 'hidden', | ||
| cursor: 'pointer', | ||
| }} | ||
| onClick={() => { |
There was a problem hiding this comment.
issue (complexity): Consider extracting the inline notification logic into a reusable hook to improve code maintainability and readability by reducing the complexity of the onClick handler.
Consider extracting the inline notification logic into a reusable hook to clean up the onClick handler. For example, you could create a small hook like this:
import { useCallback } from 'react';
export function useNotification() {
const showNotification = useCallback((message: string, duration = 2000) => {
const notification = document.createElement('div');
notification.textContent = message;
Object.assign(notification.style, {
position: 'fixed',
bottom: '20px',
left: '50%',
transform: 'translateX(-50%)',
padding: '10px 20px',
backgroundColor: '#333',
color: 'white',
borderRadius: '5px',
zIndex: '9999',
});
document.body.appendChild(notification);
setTimeout(() => {
document.body.removeChild(notification);
}, duration);
}, []);
return { showNotification };
}Then integrate it in your component:
import React from 'react';
import * as Icons from '../../packages/icon/lib';
import { useNotification } from './path/to/useNotification';
export const Default: React.FC<{ size?: number }> = ({ size = 24 }) => {
const { showNotification } = useNotification();
const iconComponents = Object.entries(Icons).filter(([name]) =>
name.endsWith('Icon'),
);
return (
<div
style={{
display: 'flex',
flexWrap: 'wrap',
gap: '20px',
fontFamily: 'sans-serif',
}}
>
{iconComponents.map(([name, Icon]: [string, any]) => (
<div
key={name}
style={{
display: 'flex',
flexDirection: 'column',
alignItems: 'center',
justifyContent: 'center',
width: '130px',
height: '90px',
padding: '10px',
border: '1px solid #eee',
borderRadius: '5px',
boxShadow: '0 2px 4px rgba(0,0,0,0.05)',
transition: 'transform 0.2s, box-shadow 0.2s',
gap: '10px',
overflow: 'hidden',
cursor: 'pointer',
}}
onClick={() => {
navigator.clipboard.writeText(name);
showNotification(`${name} copied!`);
}}
onMouseOver={(e) => {
e.currentTarget.style.transform = 'translateY(-2px)';
e.currentTarget.style.boxShadow =
'0 4px 8px rgba(0,0,0,0.1)';
}}
onMouseOut={(e) => {
e.currentTarget.style.transform = 'translateY(0)';
e.currentTarget.style.boxShadow =
'0 2px 4px rgba(0,0,0,0.05)';
}}
>
<Icon width={size} height={size} />
<div
style={{
textAlign: 'center',
fontSize: '12px',
whiteSpace: 'nowrap',
overflow: 'hidden',
textOverflow: 'ellipsis',
width: '100%',
}}
>
{name.replace('Icon', '')}
</div>
</div>
))}
</div>
);
};This keeps functionality intact while making the code more maintainable and easier to read.
Signed-off-by: Lukas.J.Han <lukas.j.han@gmail.com>
Signed-off-by: Lukas.J.Han <lukas.j.han@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
It closes #4.
Summary by Sourcery
Adds an icon library with a variety of icons and a storybook to showcase them.
New Features: