EPMRPP-108273 || Add new icons: Configuration, GroupBy, PinFilled, PinOutline #152
Conversation
siarheirazuvalau
requested review from
AmsterGet,
allaprischepa and
maria-hambardzumian
as code owners
WalkthroughAdds a new SCSS module for an interactive icon grid, refactors the icon stories to dynamically render all icons with click/keyboard-to-copy behavior and a brief copy notification, and exports four additional SVG icon components from the icons index. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Grid as IconsGridComponent
participant Clipboard as Clipboard API
User->>Grid: Click or press Enter/Space on icon button
activate Grid
Grid->>Clipboard: navigator.clipboard.writeText(iconName)
activate Clipboard
Clipboard-->>Grid: Promise resolves (success)
deactivate Clipboard
Grid->>Grid: setCopiedName(iconName) & show copy-notification
Grid-->>User: Visual feedback (notification + label)
deactivate Grid
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/components/icons/icons.stories.tsx (2)
22-24: Consider adding error handling and user feedback.The clipboard operation can fail silently in some contexts. Consider adding a try-catch block and visual feedback (e.g., a toast notification) to inform users whether the copy succeeded.
Example implementation:
const handleCopyName = (name: string) => { - navigator.clipboard.writeText(name); + navigator.clipboard.writeText(name) + .then(() => { + // Could add toast notification: "Copied!" + }) + .catch((err) => { + console.error('Failed to copy:', err); + }); };
28-28: Consider improving type safety for icon components.The
Object.entries(icons)approach typesIconasany, which loses type safety. Consider adding a type assertion or helper to ensure all values are valid React components.Example with type assertion:
{Object.entries(icons).map(([name, Icon]) => { const IconComponent = Icon as React.ComponentType; return ( <div key={name} className={styles['icon-item']} onClick={() => handleCopyName(name)} title={`Click to copy: ${name}`} > <BaseIconButton> <IconComponent /> </BaseIconButton> <div className={styles['icon-name']}>{name}</div> </div> ); })}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
src/components/icons/svg/configuration.svgis excluded by!**/*.svgsrc/components/icons/svg/groupBy.svgis excluded by!**/*.svgsrc/components/icons/svg/pinFilled.svgis excluded by!**/*.svgsrc/components/icons/svg/pinOutline.svgis excluded by!**/*.svg
📒 Files selected for processing (3)
src/components/icons/icons.stories.module.scss(1 hunks)src/components/icons/icons.stories.tsx(2 hunks)src/components/icons/index.ts(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/icons/icons.stories.tsx (1)
src/components/baseIconButton/baseIconButton.tsx (1)
BaseIconButton(15-31)
🔇 Additional comments (5)
src/components/icons/index.ts (1)
16-16: LGTM! New icon exports are properly integrated.The four new icon exports (ConfigurationIcon, GroupByIcon, PinFilledIcon, PinOutlineIcon) follow the established pattern and maintain alphabetical ordering.
Also applies to: 33-33, 44-45
src/components/icons/icons.stories.module.scss (3)
17-23: LGTM! Grid layout is well-designed.The responsive grid with
auto-fillandminmaxprovides a flexible layout that adapts to the dynamic icon count.
25-45: LGTM! Interactive styling enhances UX.The hover and active states with smooth transitions provide good visual feedback for the click-to-copy interaction.
47-57: LGTM! Text styling is appropriate.The use of
user-select: textallows manual copying of icon names, and the ellipsis handling prevents layout issues with longer names.src/components/icons/icons.stories.tsx (1)
4-5: LGTM! Wildcard import enables automatic icon inclusion.Using
import * as iconsautomatically includes newly added icons in the gallery without requiring manual updates to the stories file.
…r copying the icon name
2 participants
Changes
Visuals
Summary by CodeRabbit
New Features
Accessibility