-
-
Notifications
You must be signed in to change notification settings - Fork 39
feat: add minimal flat Cloud shape component #94
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
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for awesomeshapes ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
@shreyashpatel5506 Deploy preview fails |
Yes, the dependency conflict exists on main as well. |
|
If everything looks good, please review. |
|
@ashutosh1919 please check my pr |
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.
Pull request overview
This PR attempts to add a new Cloud shape component to the react-awesome-shapes library. However, the implementation does not follow the established patterns used by all other shapes in the codebase, creating significant API inconsistencies and maintainability concerns.
Changes:
- Added a new Cloud shape component using direct SVG rendering (instead of the BaseShape wrapper pattern)
- Exported Cloud component from library index
- Added Cloud to the shapes demo constants and display
- Included unrelated changes to footer component, configuration files, and build setup
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/lib/shapes/cloudshape.tsx | New Cloud component with non-standard implementation pattern |
| src/lib/index.tsx | Exports Cloud using default export (inconsistent with other shapes) |
| src/constants/shapes.constant.ts | Adds Cloud to scope and example (with invalid gradient color) |
| src/components/demo/shapesdemos.tsx | Adds Cloud to demo display |
| src/components/layout/footer.tsx | Unrelated: Updates deprecated Themed.a to Link with security attributes |
| .npmrc | Unrelated: Adds legacy-peer-deps configuration |
| .vscode/settings.json | Unrelated: Adds VS Code PR settings |
| .github/workflows/lint-check.yml | Unrelated: Updates Node version and install command |
| .eslintrc.json | Unrelated: Adds ESLint rule for sx prop |
| package.json | Unrelated: Adds Babel plugin dependency |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import React from 'react'; | ||
|
|
||
| interface CloudProps { | ||
| width?: number | string; | ||
| height?: number | string; | ||
| color?: string; | ||
| zIndex?: number; | ||
| } | ||
|
|
||
| const Cloud: React.FC<CloudProps> = ({ | ||
| width = '220px', | ||
| height = '120px', | ||
| color = '#E5E7EB', | ||
| zIndex = 1 | ||
| }) => { | ||
| return ( | ||
| <svg | ||
| aria-label="Minimal flat cloud shape" | ||
| height={height} | ||
| role="img" | ||
| style={{ zIndex }} | ||
| viewBox="0 0 220 120" | ||
| width={width} | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| > | ||
| <path | ||
| d=" | ||
| M60 90 | ||
| H150 | ||
| C175 90 190 75 190 60 | ||
| C190 45 175 35 160 38 | ||
| C155 22 138 15 122 22 | ||
| C112 8 90 10 82 25 | ||
| C65 25 50 38 50 55 | ||
| C35 58 30 70 30 80 | ||
| C30 88 40 90 60 90 | ||
| Z | ||
| " | ||
| fill={color} | ||
| /> | ||
| </svg> | ||
| ); | ||
| }; | ||
|
|
||
| export default Cloud; |
Copilot
AI
Jan 18, 2026
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.
The Cloud component does not follow the established pattern used by other shapes in this codebase. All other shapes (Circle, Star, Heart, Arrow, Message, etc.) use the BaseShape wrapper component with styled-components and clip-path. This inconsistency creates maintenance issues and API incompatibility.
The Cloud component should:
- Use BaseShape wrapper for consistent positioning and responsive behavior
- Accept the standard ShapeProps or ShapeWithSize interface properties (top, left, right, bottom, position, className, breakpoints, zIndex as string)
- Use styled-components with clip-path or background for the shape rendering
- Follow the same pattern as other shapes for consistency
| width?: number | string; | ||
| height?: number | string; | ||
| color?: string; | ||
| zIndex?: number; | ||
| } | ||
|
|
||
| const Cloud: React.FC<CloudProps> = ({ | ||
| width = '220px', | ||
| height = '120px', | ||
| color = '#E5E7EB', | ||
| zIndex = 1 | ||
| }) => { | ||
| return ( | ||
| <svg | ||
| aria-label="Minimal flat cloud shape" | ||
| height={height} | ||
| role="img" | ||
| style={{ zIndex }} | ||
| viewBox="0 0 220 120" | ||
| width={width} |
Copilot
AI
Jan 18, 2026
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.
The CloudProps interface has inconsistent type signatures compared to other shapes in the codebase. The zIndex should be typed as string (not number), and width/height should support responsive arrays (string | string[]) to be consistent with the BaseShape pattern used by all other shapes. This breaks API consistency across the shape library.
| width?: number | string; | |
| height?: number | string; | |
| color?: string; | |
| zIndex?: number; | |
| } | |
| const Cloud: React.FC<CloudProps> = ({ | |
| width = '220px', | |
| height = '120px', | |
| color = '#E5E7EB', | |
| zIndex = 1 | |
| }) => { | |
| return ( | |
| <svg | |
| aria-label="Minimal flat cloud shape" | |
| height={height} | |
| role="img" | |
| style={{ zIndex }} | |
| viewBox="0 0 220 120" | |
| width={width} | |
| width?: string | string[]; | |
| height?: string | string[]; | |
| color?: string; | |
| zIndex?: string; | |
| } | |
| const resolveResponsiveValue = ( | |
| value?: string | string[] | |
| ): string | undefined => { | |
| if (Array.isArray(value)) { | |
| return value[0]; | |
| } | |
| return value; | |
| }; | |
| const Cloud: React.FC<CloudProps> = ({ | |
| width = '220px', | |
| height = '120px', | |
| color = '#E5E7EB', | |
| zIndex = '1' | |
| }) => { | |
| return ( | |
| <svg | |
| aria-label="Minimal flat cloud shape" | |
| height={resolveResponsiveValue(height)} | |
| role="img" | |
| style={{ zIndex }} | |
| viewBox="0 0 220 120" | |
| width={resolveResponsiveValue(width)} |
| export { SquareDonut } from './shapes/squareDonut'; | ||
| export { Arrow } from './shapes/arrow'; | ||
| export { Message } from './shapes/message'; | ||
| export { default as Cloud } from './shapes/cloudshape'; |
Copilot
AI
Jan 18, 2026
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.
The export pattern for Cloud is inconsistent with all other shapes in the library. All other shapes use named exports (e.g., export { Circle }, export { Star }), but Cloud uses a default export. This breaks the consistent import pattern established throughout the codebase and should be changed to a named export for consistency.
| export { default as Cloud } from './shapes/cloudshape'; | |
| export { Cloud } from './shapes/cloudshape'; |
| export const cloudExample = `<Cloud | ||
| width="220px" | ||
| height="120px" | ||
| color="linear-gradient(135deg, #c7d2fe, #a5b4fc)" |
Copilot
AI
Jan 18, 2026
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.
The color value uses a CSS linear-gradient which is incompatible with the SVG fill attribute. SVG fill only accepts solid colors (like hex, rgb, or named colors), not CSS gradient syntax. The gradient will not render as expected. Use a solid color value like "#c7d2fe" instead, or implement gradient support using SVG gradient elements (linearGradient/radialGradient) if gradient rendering is desired.
| @@ -0,0 +1 @@ | |||
| legacy-peer-deps=true No newline at end of file | |||
Copilot
AI
Jan 18, 2026
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.
These configuration changes (.npmrc, .vscode/settings.json, lint-check.yml, .eslintrc.json, package.json) appear unrelated to adding the Cloud shape component. While they may address existing project setup issues, they should ideally be in a separate PR focused on project configuration and build improvements. This makes it harder to review and understand what changes are actually needed for the Cloud shape feature.
| legacy-peer-deps=true |
✨ What’s added
aria-label🧪 Testing
📌 Notes
(framework-related). This PR does not introduce or modify dependencies
and focuses only on adding the Cloud shape component.