Icon changes #2359
Icon changes #2359
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
npm warn config production Use WalkthroughThis pull request introduces updates to icon components across various files. Key changes include a redesigned Changes
Sequence DiagramsequenceDiagram
participant UI as User Interface
participant Icons as Icon Components
participant Styles as CSS Styles
UI->>Icons: Request Default Profile Icon
Icons-->>UI: Render New SVG Icon
UI->>Icons: Request Classification Icons
Icons-->>UI: Render Specific Project Icons
UI->>Styles: Apply Classification Item Styling
Styles-->>UI: Provide Consistent Layout
Possibly related PRs
Suggested labels
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
public/assets/images/icons/headerIcons/DefaultProfileImageIcon.tsx (1)
1-45: Make the component more reusable with propsConsider adding size props to make the component more flexible.
+interface DefaultProfileImageIconProps { + width?: string | number; + height?: string | number; +} -const DefaultProfileImageIcon = () => { +const DefaultProfileImageIcon = ({ width = 48, height = 48 }: DefaultProfileImageIconProps) => { return ( <svg - width="48" - height="48" + width={width} + height={height} viewBox="0 0 48 48"src/features/projectsV2/ProjectListControls/microComponents/ClassificationDropDown.tsx (2)
26-42: Optimize icon configuration with useMemoThe
classificationItemIconsobject is recreated on every render. Consider using useMemo to optimize performance.-const classificationItemIcons = { +const classificationItemIcons = useMemo(() => ({ 'large-scale-planting': ( <TreePlanting color="black" width={'20'} height={'20'} /> ), // ... other icons mangroves: <Mangroves color="black" width={'20'} height={'20'} />, -}; +}), []);
26-42: Extract icon size to a constantThe icon size '20' is repeated across all icons. Consider extracting it to a constant.
+const CLASSIFICATION_ICON_SIZE = '20'; + const classificationItemIcons = { 'large-scale-planting': ( - <TreePlanting color="black" width={'20'} height={'20'} /> + <TreePlanting color="black" width={CLASSIFICATION_ICON_SIZE} height={CLASSIFICATION_ICON_SIZE} /> ), // ... other icons
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
public/assets/images/icons/headerIcons/DefaultProfileImageIcon.tsx(1 hunks)public/assets/images/icons/myForestMapIcons/PointMarkerIconsSymbol.tsx(1 hunks)src/features/projectsV2/ProjectListControls/microComponents/ClassificationDropDown.tsx(2 hunks)src/features/projectsV2/ProjectListControls/styles/ProjectListControls.module.scss(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
src/features/projectsV2/ProjectListControls/styles/ProjectListControls.module.scss (1)
239-244: LGTM! Clean and consistent stylingThe new classificationItem class follows the existing style patterns and uses appropriate flex properties for layout.
| <svg | ||
| width="48" | ||
| height="48" | ||
| viewBox="0 0 48 48" | ||
| fill="none" | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| > |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add accessibility attributes to SVG
The SVG element should include accessibility attributes for better screen reader support.
<svg
width="48"
height="48"
viewBox="0 0 48 48"
fill="none"
xmlns="http://www.w3.org/2000/svg"
+ role="img"
+ aria-label="Default Profile Image"
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <svg | |
| width="48" | |
| height="48" | |
| viewBox="0 0 48 48" | |
| fill="none" | |
| xmlns="http://www.w3.org/2000/svg" | |
| > | |
| <svg | |
| width="48" | |
| height="48" | |
| viewBox="0 0 48 48" | |
| fill="none" | |
| xmlns="http://www.w3.org/2000/svg" | |
| role="img" | |
| aria-label="Default Profile Image" | |
| > |
| // Todo - move to a common location | ||
| import type { IconProps } from '../../../../../src/features/common/types/common'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Move IconProps type to a shared types file
The TODO comment suggests moving types to a common location. This should be addressed.
-// Todo - move to a common location
-import type { IconProps } from '../../../../../src/features/common/types/common';
+import type { IconProps } from '@/types/common';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Todo - move to a common location | |
| import type { IconProps } from '../../../../../src/features/common/types/common'; | |
| import type { IconProps } from '@/types/common'; |
| export const RegisteredTreeIcon = () => { | ||
| return ( | ||
| <svg | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| xmlSpace="preserve" | ||
| id="Layer_1" | ||
| x={0} | ||
| y={0} | ||
| viewBox="0 0 42 49" | ||
| > | ||
| <style>{'.st1{fill:#007a49}'}</style> | ||
| <path | ||
| d="M35.3 6.6c7.9 7.7 7.9 20.3 0 28L21.6 48c-.3.3-.8.3-1.2 0L6.7 34.6c-7.9-7.7-7.9-20.3 0-28 7.9-7.8 20.7-7.8 28.6 0z" | ||
| style={{ | ||
| fill: '#fff', | ||
| stroke: '#007a49', | ||
| strokeWidth: 1.5, | ||
| }} | ||
| /> | ||
| <path | ||
| d="M36.6 15.4c-.4-.1-.8-.2-1.3-.3-1-.2-1.9.2-2.8.6-1.1.4-2.1 1.1-3.1 1.8-.9.6-1.8 1.3-2.8 1.9-.5.3-1.1.5-1.7.7-.5.2-1 .1-1.5-.1-.2-.1-.3 0-.3.1-.9 1.4-1.6 2.9-1.9 4.6-.1.5-.4.8-.9 1-.5.1-.9 0-1.3-.3-1.3-1.4-2.6-2.9-3.9-4.3-.2-.2-.2-.4 0-.6l.4-.4c.6-.5 1.4-.5 2 .1.6.7 1.2 1.4 1.8 2 .1.1.1.2.2.3 2.2-4 5.3-7 9-9.4-1.3.4-2.5 1.2-3.6 2.1-.9.8-1.9 1.8-3 2.7 0-.7 0-1.5.2-2.2 1-4.1 5.3-6.3 9.3-4.7.8.3 1.6.8 2.3 1.4 1 .9 2 1.9 2.9 3 0-.1 0-.1 0 0z" | ||
| className="st1" | ||
| /> | ||
| <path | ||
| d="M28.1 19c1.1 3.3 0 7.9-3.8 10.4-3.7 2.4-8.6 1.7-11.5-1.6-3-3.3-3.1-8.3-.4-11.8 2.8-3.6 7.3-4.2 10.3-2.9-.3.5-.5 1-.8 1.4-.7-.1-1.4-.3-2.1-.3-3.9-.1-7.1 2.7-7.6 6.6-.5 3.8 2.3 7.6 6.1 8.1 4.1.6 7.9-2.2 8.4-6.4.1-.7 0-1.5-.1-2.2 0-.3 0-.4.2-.5.5-.2.9-.5 1.3-.8z" | ||
| className="st1" | ||
| /> | ||
| </svg> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Inconsistent props pattern in RegisteredTreeIcon
Unlike other icon components, RegisteredTreeIcon doesn't accept standard IconProps. Consider making it consistent with other icons.
-export const RegisteredTreeIcon = () => {
+export const RegisteredTreeIcon = ({ color, width, height }: IconProps) => {
return (
<svg
xmlns="http://www.w3.org/2000/svg"
xmlSpace="preserve"
- id="Layer_1"
x={0}
y={0}
+ width={width}
+ height={height}
viewBox="0 0 42 49"
>
- <style>{'.st1{fill:#007a49}'}</style>
+ <style>{`.st1{fill:${color}}`}</style>Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
@rupamkairi , please also change the icon in the edit profile form (see attached screenshot).
public/assets/images/icons/myForestMapIcons/PointMarkerIconsSymbol.tsx
Outdated
Show resolved
Hide resolved
public/assets/images/icons/headerIcons/DefaultProfileImageIcon.tsx
Outdated
Show resolved
Hide resolved
src/features/projectsV2/ProjectListControls/microComponents/ClassificationDropDown.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
public/assets/images/icons/headerIcons/DefaultProfileImageIcon.tsx (1)
12-21: Use consistent color prop.The
colorprop is not being used in thefillandstrokeattributes of the rectangles. For consistency, these should use the passed color prop.- <rect x="2" y="2" width="44" height="44" rx="22" fill="#DFF4E8" /> + <rect x="2" y="2" width="44" height="44" rx="22" fill={color} /> <rect x="2" y="2" width="44" height="44" rx="22" - stroke="white" + stroke={color} strokeWidth="2.15138" />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
public/assets/images/icons/headerIcons/DefaultProfileImageIcon.tsx(1 hunks)public/assets/images/icons/myForestMapIcons/PointMarkerIconsSymbol.tsx(1 hunks)src/features/projectsV2/ProjectListControls/microComponents/ClassificationDropDown.tsx(2 hunks)src/features/projectsV2/ProjectListControls/styles/ProjectListControls.module.scss(1 hunks)src/features/user/Profile/ProfileCard/index.tsx(2 hunks)src/features/user/Settings/EditProfile/EditProfileForm.tsx(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/features/user/Settings/EditProfile/EditProfileForm.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/features/projectsV2/ProjectListControls/styles/ProjectListControls.module.scss
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (6)
src/features/user/Profile/ProfileCard/index.tsx (1)
7-8: LGTM! Icon changes look good.The replacement of
DefaultUserProfileImagewithDefaultProfileImageIconis consistent with the PR objectives.Also applies to: 29-29
public/assets/images/icons/headerIcons/DefaultProfileImageIcon.tsx (2)
1-2: Move IconProps type to a common location.The TODO comment suggests moving types to a common location. This should be addressed.
-// Todo - move to a common location -import type { IconProps } from '../../../../../src/features/common/types/common'; +import type { IconProps } from '@/types/common';
5-11: Add accessibility attributes to SVG.The SVG element should include accessibility attributes for better screen reader support.
<svg width={width} height={height} viewBox="0 0 48 48" fill="none" xmlns="http://www.w3.org/2000/svg" + role="img" + aria-label="Default Profile Image" >src/features/projectsV2/ProjectListControls/microComponents/ClassificationDropDown.tsx (1)
26-34: Consider CSS-based styling for icons.Based on past feedback:
- Move the hardcoded width/height values to the icon components' default props
- Use CSS for icon colors to match the text color (rgb(51,51,51)) from the
.filterButtonclassconst classificationItemIcons = { - 'large-scale-planting': <TreePlanting width={'20'} height={'20'} />, - agroforestry: <Agroforestry width={'20'} height={'20'} />, - 'natural-regeneration': <NaturalRegeneration width={'20'} height={'20'} />, - 'managed-regeneration': <ManagedRegeneration width={'20'} height={'20'} />, - 'urban-planting': <UrbanRestoration width={'20'} height={'20'} />, - 'other-planting': <OtherPlanting width={'20'} height={'20'} />, - mangroves: <Mangroves width={'20'} height={'20'} />, + 'large-scale-planting': <TreePlanting />, + agroforestry: <Agroforestry />, + 'natural-regeneration': <NaturalRegeneration />, + 'managed-regeneration': <ManagedRegeneration />, + 'urban-planting': <UrbanRestoration />, + 'other-planting': <OtherPlanting />, + mangroves: <Mangroves />, };Add to your CSS:
.filterButton svg { width: 20px; height: 20px; color: rgb(51,51,51); }public/assets/images/icons/myForestMapIcons/PointMarkerIconsSymbol.tsx (2)
1-272: Consider architectural improvements.Based on past feedback:
- Consider merging with existing
PointMarkerIconsto avoid duplication- If kept separate, move to
/icons/projectv2as it's specific to the project list pageAdditionally, for consistency across all icon components:
- Add accessibility attributes to all SVGs
- Consider using CSS variables for colors instead of props
- Set default width/height in the components
167-196: Make RegisteredTreeIcon consistent with other icons.Unlike other icon components,
RegisteredTreeIcondoesn't accept standardIconProps. Consider making it consistent with other icons.-export const RegisteredTreeIcon = () => { +export const RegisteredTreeIcon = ({ color, width, height }: IconProps) => { return ( <svg xmlns="http://www.w3.org/2000/svg" xmlSpace="preserve" - id="Layer_1" x={0} y={0} + width={width} + height={height} viewBox="0 0 42 49" > - <style>{'.st1{fill:#007a49}'}</style> + <style>{`.st1{fill:${color}}`}</style>
|
@rupamkairi , could you remove the frame around this instance of the icon (path /profile) |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
public/assets/images/icons/projectV2/PointMarkerIconsSymbol.tsx (1)
1-2: Address the TODO comment to improve code organization.Moving the
IconPropstype to a common location would improve maintainability and reduce import complexity. Consider creating a dedicated types directory at the root level.-// Todo - move to a common location -import type { IconProps } from '../../../../../src/features/common/types/common'; +import type { IconProps } from '@/types/common';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
public/assets/images/icons/headerIcons/DefaultProfileImageIcon.tsx(1 hunks)public/assets/images/icons/projectV2/PointMarkerIconsSymbol.tsx(1 hunks)src/features/projectsV2/ProjectListControls/microComponents/ClassificationDropDown.tsx(2 hunks)src/features/user/Settings/EditProfile/EditProfileForm.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/features/user/Settings/EditProfile/EditProfileForm.tsx
- src/features/projectsV2/ProjectListControls/microComponents/ClassificationDropDown.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
public/assets/images/icons/headerIcons/DefaultProfileImageIcon.tsx (1)
3-29: Add accessibility attributes and improve responsiveness.Previous review comments about accessibility and fixed dimensions are still valid.
Apply these improvements:
- <svg viewBox="0 0 48 48" fill="none" xmlns="http://www.w3.org/2000/svg"> + <svg + viewBox="0 0 48 48" + fill="none" + xmlns="http://www.w3.org/2000/svg" + role="img" + aria-label="Default Profile Image" + >Also, consider making the icon more responsive by accepting width and height props:
-const DefaultProfileImageIcon = () => { +const DefaultProfileImageIcon = ({ width, height }: { width?: number; height?: number }) => { return ( <svg + width={width} + height={height} viewBox="0 0 48 48" fill="none" xmlns="http://www.w3.org/2000/svg"
| x={0} | ||
| y={0} | ||
| viewBox="0 0 42 49" | ||
| > | ||
| <style>{'.st1{fill:#007a49}'}</style> | ||
| <path | ||
| d="M35.3 6.6c7.9 7.7 7.9 20.3 0 28L21.6 48c-.3.3-.8.3-1.2 0L6.7 34.6c-7.9-7.7-7.9-20.3 0-28 7.9-7.8 20.7-7.8 28.6 0z" | ||
| style={{ | ||
| fill: '#fff', | ||
| stroke: '#007a49', | ||
| strokeWidth: 1.5, | ||
| }} | ||
| /> | ||
| <path | ||
| d="M36.6 15.4c-.4-.1-.8-.2-1.3-.3-1-.2-1.9.2-2.8.6-1.1.4-2.1 1.1-3.1 1.8-.9.6-1.8 1.3-2.8 1.9-.5.3-1.1.5-1.7.7-.5.2-1 .1-1.5-.1-.2-.1-.3 0-.3.1-.9 1.4-1.6 2.9-1.9 4.6-.1.5-.4.8-.9 1-.5.1-.9 0-1.3-.3-1.3-1.4-2.6-2.9-3.9-4.3-.2-.2-.2-.4 0-.6l.4-.4c.6-.5 1.4-.5 2 .1.6.7 1.2 1.4 1.8 2 .1.1.1.2.2.3 2.2-4 5.3-7 9-9.4-1.3.4-2.5 1.2-3.6 2.1-.9.8-1.9 1.8-3 2.7 0-.7 0-1.5.2-2.2 1-4.1 5.3-6.3 9.3-4.7.8.3 1.6.8 2.3 1.4 1 .9 2 1.9 2.9 3 0-.1 0-.1 0 0z" | ||
| className="st1" | ||
| /> | ||
| <path | ||
| d="M28.1 19c1.1 3.3 0 7.9-3.8 10.4-3.7 2.4-8.6 1.7-11.5-1.6-3-3.3-3.1-8.3-.4-11.8 2.8-3.6 7.3-4.2 10.3-2.9-.3.5-.5 1-.8 1.4-.7-.1-1.4-.3-2.1-.3-3.9-.1-7.1 2.7-7.6 6.6-.5 3.8 2.3 7.6 6.1 8.1 4.1.6 7.9-2.2 8.4-6.4.1-.7 0-1.5-.1-2.2 0-.3 0-.4.2-.5.5-.2.9-.5 1.3-.8z" | ||
| className="st1" | ||
| /> | ||
| </svg> | ||
| ); | ||
| }; | ||
|
|
||
| export const TreePlanting = ({ | ||
| color = 'defaultColor', | ||
| width, | ||
| height, | ||
| }: IconProps) => { | ||
| return ( | ||
| <svg | ||
| viewBox="4 4 32 32" | ||
| width={width} | ||
| height={height} | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| > | ||
| <g clipPath="url(#clip0_1568_11606)"> | ||
| <path | ||
| d="M21.9965 23.6068C21.9224 23.6768 21.8647 23.7263 21.8111 23.7757C18.7291 26.8317 20.9005 24.6406 17.8144 27.6925C17.6743 27.8326 17.6372 27.9396 17.7114 28.1332C18.1482 29.2906 18.0039 30.4067 17.3777 31.4446C17.0769 31.943 16.6772 32.3837 16.3105 32.8409C16.158 33.0344 15.9355 33.0962 15.7089 32.9932C15.4246 32.8656 15.1321 32.7338 14.8807 32.5484C13.4551 31.4899 12.1943 30.2585 11.193 28.784C10.6904 28.0426 10.7274 27.8367 11.3949 27.2559C12.1737 26.5764 13.0389 26.0739 14.0896 25.9586C14.6788 25.8927 15.2474 25.9833 15.7954 26.1933C15.9438 26.251 16.0385 26.2386 16.1539 26.1192C19.2318 23.0096 17.0604 25.1554 20.1424 22.05C20.1877 22.0047 20.2289 21.947 20.2619 21.9099C19.6727 21.218 19.0835 20.5384 18.5149 19.8506C18.2965 19.587 18.1152 19.2987 17.9133 19.0227C17.8309 18.9115 17.8226 18.8292 17.9339 18.718C19.3678 17.2929 20.7769 15.8431 22.2355 14.4428C23.8342 12.9107 25.779 12.4411 27.9298 12.8489C28.6797 12.9889 29.4008 13.2649 30.1301 13.4914C30.229 13.5202 30.3402 13.6191 30.3814 13.7138C30.8388 14.7805 31.1354 15.8884 31.1766 17.0499C31.2426 18.8745 30.6451 20.4519 29.3637 21.7534C27.9834 23.1579 26.5783 24.5418 25.1898 25.938C25.0703 26.0574 24.9797 26.078 24.8478 25.9709C23.999 25.2749 23.1461 24.5829 22.2932 23.8869C22.1943 23.8045 22.1078 23.7139 21.9924 23.6068H21.9965ZM15.9273 31.494C16.7225 30.7403 17.0686 29.8754 16.7637 28.8375C16.4671 27.8284 15.7625 27.223 14.7283 27.0583C13.7971 26.91 13.0431 27.2724 12.4085 27.9726C13.7518 28.9899 14.9096 30.1514 15.9314 31.494H15.9273Z" | ||
| fill={color} | ||
| /> | ||
| </g> | ||
| <defs> | ||
| <clipPath id="clip0_1568_11606"> | ||
| <rect | ||
| height="20.3338" | ||
| width="20.3338" | ||
| fill={color} | ||
| transform="translate(10.8516 12.7087)" | ||
| /> | ||
| </clipPath> | ||
| </defs> | ||
| </svg> | ||
| ); | ||
| }; | ||
|
|
||
| export const UrbanRestoration = ({ | ||
| color = 'defaultColor', | ||
| width, | ||
| height, | ||
| }: IconProps) => { | ||
| return ( | ||
| <svg | ||
| viewBox="4 4 32 32" | ||
| width={width} | ||
| height={height} | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| > | ||
| <g clipPath="url(#clip0_3579_1345)"> | ||
| <path | ||
| d="M25.6536 22.2729C25.6536 22.1286 25.6536 22.0059 25.6536 21.8833C25.6536 21.1401 25.6536 20.3897 25.6536 19.6466C25.6536 19.56 25.6262 19.5095 25.551 19.4662C23.1915 18.0088 20.832 16.5514 18.4657 15.0939C18.3904 15.0434 18.3768 15.0001 18.3904 14.9136C18.8897 12.4532 20.791 10.534 23.1368 10.1011C26.2281 9.53838 29.1826 11.5874 29.9144 14.7981C29.9554 14.9641 29.9828 15.1372 30.0033 15.3032C30.017 15.4042 30.058 15.4403 30.1469 15.4619C31.4532 15.7866 32.3559 16.6163 32.7936 17.9583C33.5049 20.1228 32.1713 22.4821 30.0238 22.8717C28.8475 23.0809 27.8216 22.7491 26.9393 21.8977C26.8573 21.8183 26.8026 21.8111 26.7068 21.8616C26.3717 22.042 26.0161 22.1719 25.6399 22.2729H25.6536Z" | ||
| fill={color} | ||
| /> | ||
| <path | ||
| d="M15.3202 30.0002H11.3945C11.3945 29.9569 11.3945 29.9209 11.3945 29.8776C11.3945 27.2369 11.3945 24.6034 11.3945 21.9627C11.3945 21.8833 11.4219 21.84 11.4834 21.7968C13.4463 20.5846 15.4159 19.3725 17.3788 18.1532C17.4608 18.1027 17.5224 18.0955 17.6045 18.1532C19.5673 19.3653 21.5233 20.5774 23.4861 21.7895C23.5613 21.8328 23.5887 21.8833 23.5887 21.9771C23.5887 24.6034 23.5887 27.2297 23.5887 29.8559C23.5887 29.8992 23.5887 29.9425 23.5887 29.993H19.663C19.663 29.9209 19.663 29.8559 19.663 29.791C19.663 28.0666 19.663 26.3422 19.663 24.625C19.663 23.4779 18.8902 22.4966 17.8507 22.3307C16.4965 22.1142 15.327 23.1676 15.327 24.6178C15.327 26.3566 15.327 28.1027 15.327 29.8415C15.327 29.892 15.327 29.9425 15.327 30.0002H15.3202Z" | ||
| fill={color} | ||
| /> | ||
| <path | ||
| d="M10.0059 21.8904C10.0059 21.8399 10.0059 21.8111 10.0059 21.775C10.0059 21.1978 10.0059 20.6134 10.0059 20.0362C10.0059 19.964 10.0264 19.9207 10.0879 19.8774C11.7156 18.8745 13.3365 17.8717 14.9642 16.8688C15.7781 16.3637 16.5919 15.8659 17.4058 15.3608C17.4742 15.3175 17.5152 15.3175 17.5836 15.3608C20.0115 16.8616 22.4326 18.3623 24.8605 19.8558C24.9562 19.9135 24.9836 19.9784 24.9836 20.0867C24.9836 20.6494 24.9836 21.2122 24.9836 21.775C24.9836 21.8111 24.9836 21.8399 24.9836 21.8976C24.9357 21.8688 24.9015 21.8543 24.8673 21.8327C22.4394 20.332 20.0115 18.8313 17.5836 17.3305C17.5221 17.2945 17.4742 17.2945 17.4126 17.3305C14.9847 18.8168 12.5637 20.3175 10.1426 21.8111C10.1016 21.8399 10.0606 21.8616 10.0059 21.8904Z" | ||
| fill={color} | ||
| /> | ||
| <path | ||
| d="M24.2656 23.1387C24.9427 23.1243 25.5993 23.0088 26.2421 22.7852V29.9929H24.2656V23.1315V23.1387Z" | ||
| fill={color} | ||
| /> | ||
| </g> | ||
| <defs> | ||
| <clipPath id="clip0_3579_1345"> | ||
| <rect | ||
| width="23" | ||
| height="20" | ||
| fill={color} | ||
| transform="translate(10 10)" | ||
| /> | ||
| </clipPath> | ||
| </defs> | ||
| </svg> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve consistency and accessibility across icon components.
-
Color prop defaults are inconsistent:
- Most components use 'defaultColor'
- OtherPlanting uses 'currentColor'
- RegisteredTreeIcon has hardcoded colors
-
RegisteredTreeIcon doesn't follow the same pattern as other components.
-
SVGs lack accessibility attributes.
Consider the following improvements:
- Standardize color defaults:
-color = 'defaultColor',
+color = 'currentColor',- Update RegisteredTreeIcon to match other components:
-export const RegisteredTreeIcon = () => {
+export const RegisteredTreeIcon = ({
+ color = 'currentColor',
+ width,
+ height,
+}: IconProps) => {
return (
<svg
+ width={width}
+ height={height}
xmlns="http://www.w3.org/2000/svg"
xmlSpace="preserve"
viewBox="0 0 42 49"
>
- <style>{'.st1{fill:#007a49}'}</style>
<path
d="M35.3 6.6c7.9 7.7 7.9 20.3 0 28L21.6 48c-.3.3-.8.3-1.2 0L6.7 34.6c-7.9-7.7-7.9-20.3 0-28 7.9-7.8 20.7-7.8 28.6 0z"
style={{
fill: '#fff',
- stroke: '#007a49',
+ stroke: color,
strokeWidth: 1.5,
}}
/>- Add accessibility attributes to all SVGs:
<svg
+ role="img"
+ aria-label="[Icon Name]"
viewBox="4 4 32 32"
width={width}
height={height}
xmlns="http://www.w3.org/2000/svg"
>Committable suggestion skipped: line range outside the PR's diff.
That might require removing the background color (a gray background color is there.). |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
public/assets/images/icons/headerIcons/DefaultProfileImageIcon.tsx (3)
3-21: Make the icon size adaptable.The SVG should adapt to its container size while maintaining aspect ratio. Consider accepting width and height as props.
-const DefaultProfileImageIcon = () => { +interface DefaultProfileImageIconProps { + width?: number | string; + height?: number | string; +} + +const DefaultProfileImageIcon: React.FC<DefaultProfileImageIconProps> = ({ + width = '100%', + height = '100%' +}) => { return ( <svg + width={width} + height={height} viewBox="0 0 48 48" fill="none" xmlns="http://www.w3.org/2000/svg"
4-5: Consider using theme colors.The background and border colors are hardcoded. Consider using theme variables for better maintainability and consistency.
- <rect width="48" height="48" rx="24" fill="#DFF4E8" /> - <rect width="48" height="48" rx="24" stroke="white" strokeWidth="2" /> + <rect width="48" height="48" rx="24" fill="var(--profile-icon-bg, #DFF4E8)" /> + <rect width="48" height="48" rx="24" stroke="var(--profile-icon-border, white)" strokeWidth="2" />
8-9: Consider using theme color for the icon.The path color is hardcoded. Consider using a theme variable for better maintainability.
- fill="#219653" + fill="var(--profile-icon-color, #219653)"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
public/assets/images/icons/headerIcons/DefaultProfileImageIcon.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
public/assets/images/icons/headerIcons/DefaultProfileImageIcon.tsx (1)
3-3: Add accessibility attributes to SVG.The SVG element should include accessibility attributes for better screen reader support.
- <svg viewBox="0 0 48 48" fill="none" xmlns="http://www.w3.org/2000/svg"> + <svg + viewBox="0 0 48 48" + fill="none" + xmlns="http://www.w3.org/2000/svg" + role="img" + aria-label="Default Profile Image" + >
| <rect | ||
| width="19" | ||
| height="23" | ||
| fill="white" | ||
| transform="translate(14 12)" | ||
| /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Adjust clipping path to remove white space.
The clipping path dimensions and translation create unnecessary white space around the icon. Consider adjusting these values to fill the container better.
<rect
- width="19"
- height="23"
+ width="21"
+ height="25"
fill="white"
- transform="translate(14 12)"
+ transform="translate(13.5 11.5)"
/>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <rect | |
| width="19" | |
| height="23" | |
| fill="white" | |
| transform="translate(14 12)" | |
| /> | |
| <rect | |
| width="21" | |
| height="25" | |
| fill="white" | |
| transform="translate(13.5 11.5)" | |
| /> |
public/assets/images/icons/projectV2/PointMarkerIconsSymbol.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
public/assets/images/icons/headerIcons/DefaultProfileImageIcon.tsx(1 hunks)public/assets/images/icons/projectV2/PointMarkerIconsSymbol.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (3)
public/assets/images/icons/projectV2/PointMarkerIconsSymbol.tsx (2)
1-2: Move IconProps type to a common location.The TODO comment indicates that the IconProps type should be moved to a common location. This is a good practice for better code organization and reusability.
Would you like me to help create a new file for common types and move the IconProps type there?
167-196: 🛠️ Refactor suggestionUpdate RegisteredTreeIcon to match component pattern.
The RegisteredTreeIcon component:
- Doesn't accept standard props (color, width, height)
- Uses hardcoded colors
- Uses inline styles
Apply this diff to align with other components:
-export const RegisteredTreeIcon = () => { +export const RegisteredTreeIcon = ({ + color = 'currentColor', + width, + height, +}: IconProps) => { return ( <svg + width={width} + height={height} xmlns="http://www.w3.org/2000/svg" xmlSpace="preserve" - id="Layer_1" - x={0} - y={0} viewBox="0 0 42 49" > - <style>{'.st1{fill:#007a49}'}</style> <path d="M35.3 6.6c7.9 7.7 7.9 20.3 0 28L21.6 48c-.3.3-.8.3-1.2 0L6.7 34.6c-7.9-7.7-7.9-20.3 0-28 7.9-7.8 20.7-7.8 28.6 0z" style={{ fill: '#fff', - stroke: '#007a49', + stroke: color, strokeWidth: 1.5, }} /> <path d="M36.6 15.4c-.4-.1-.8-.2-1.3-.3-1-.2-1.9.2-2.8.6-1.1.4-2.1 1.1-3.1 1.8-.9.6-1.8 1.3-2.8 1.9-.5.3-1.1.5-1.7.7-.5.2-1 .1-1.5-.1-.2-.1-.3 0-.3.1-.9 1.4-1.6 2.9-1.9 4.6-.1.5-.4.8-.9 1-.5.1-.9 0-1.3-.3-1.3-1.4-2.6-2.9-3.9-4.3-.2-.2-.2-.4 0-.6l.4-.4c.6-.5 1.4-.5 2 .1.6.7 1.2 1.4 1.8 2 .1.1.1.2.2.3 2.2-4 5.3-7 9-9.4-1.3.4-2.5 1.2-3.6 2.1-.9.8-1.9 1.8-3 2.7 0-.7 0-1.5.2-2.2 1-4.1 5.3-6.3 9.3-4.7.8.3 1.6.8 2.3 1.4 1 .9 2 1.9 2.9 3 0-.1 0-.1 0 0z" - className="st1" + fill={color} /> <path d="M28.1 19c1.1 3.3 0 7.9-3.8 10.4-3.7 2.4-8.6 1.7-11.5-1.6-3-3.3-3.1-8.3-.4-11.8 2.8-3.6 7.3-4.2 10.3-2.9-.3.5-.5 1-.8 1.4-.7-.1-1.4-.3-2.1-.3-3.9-.1-7.1 2.7-7.6 6.6-.5 3.8 2.3 7.6 6.1 8.1 4.1.6 7.9-2.2 8.4-6.4.1-.7 0-1.5-.1-2.2 0-.3 0-.4.2-.5.5-.2.9-.5 1.3-.8z" - className="st1" + fill={color} /> </svg> ); };Likely invalid or redundant comment.
public/assets/images/icons/headerIcons/DefaultProfileImageIcon.tsx (1)
3-21: 🛠️ Refactor suggestionImprove icon component implementation.
Several improvements are needed:
- Add accessibility attributes for screen readers
- Accept standard props for customization
- Adjust clipping path to remove unnecessary white space
Apply these changes:
-const DefaultProfileImageIcon = () => { +interface IconProps { + width?: number | string; + height?: number | string; +} + +const DefaultProfileImageIcon = ({ width, height }: IconProps) => { return ( <svg + role="img" + aria-label="Default Profile Image" viewBox="0 0 48 48" + width={width} + height={height} fill="none" xmlns="http://www.w3.org/2000/svg" > <rect width="48" height="48" rx="24" fill="#DFF4E8" /> <g clipPath="url(#clip0_65_29)"> <path d="M26.0541 24.7993C28.5227 23.8038 30.2559 21.4379 30.2559 18.6582C30.2559 14.9865 27.2227 12 23.4936 12C19.7645 12 16.7445 14.9865 16.7445 18.6582C16.7445 21.4379 18.4777 23.8167 20.9463 24.7993C17.1384 25.8595 14.2628 29.1304 14.0002 33.1771C13.9739 33.6425 14.1446 34.1079 14.4598 34.457C14.788 34.8061 15.2607 35 15.7466 35H31.2407C31.7134 35 32.1992 34.8061 32.5275 34.457C32.8558 34.1209 33.0265 33.6425 32.9871 33.1771C32.7245 29.1304 29.8357 25.8465 26.041 24.7993H26.0541Z" fill="#219653" /> </g> <defs> <clipPath id="clip0_65_29"> <rect - width="19" - height="23" + width="21" + height="25" fill="white" - transform="translate(14 12)" + transform="translate(13.5 11.5)" /> </clipPath> </defs> </svg> ); };Likely invalid or redundant comment.
This covers Icons changes proposed in the following tasks.
https://process.hub.plant-for-the-planet.org/browse/ET-206
https://process.hub.plant-for-the-planet.org/browse/ET-207
For reference
