refactor: replace neoicon with nuxt icon #11450
Conversation
WalkthroughThis pull request replaces the legacy Changes
Sequence Diagram(s)sequenceDiagram
participant UI as UI Component
participant KIcon as KIcon Component
participant IconLib as Icon Library
UI->>KIcon: Request icon render (name, size)
KIcon->>IconLib: Map size to CSS class using sizeMap
IconLib-->>KIcon: Return icon styling/makeup
KIcon-->>UI: Render icon with computed classes
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (21)
🪧 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 (
|
✅ Deploy Preview for koda-canary ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
|
|
👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎ This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
Deploying koda-art-prod with
|
| Latest commit: |
8b9f0f1
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://ae4c6598.kodaart-production.pages.dev |
| Branch Preview URL: | https://refactor--replace-neoicon.kodaart-production.pages.dev |
preschian
requested review from
vikiival and
Jarsen136
and removed request for
a team
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
components/navbar/MobileExpandableSection.vue (1)
52-59: Consider updating iconFamily propThe component still defines an
iconFamilyprop with a default value of "fasr", but it's no longer used since the new Icon component incorporates the icon family directly in the name (e.g., "i-mdi:"). Consider removing this unused prop.defineProps({ title: { type: String, default: '', }, icon: { type: String, default: '', }, - iconFamily: { - type: String, - default: 'fasr', - }, noPadding: { type: Boolean, default: false, }, })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (15)
components/ColorModeButton.vue(1 hunks)components/ColorModeSwitch.vue(4 hunks)components/Navbar.vue(4 hunks)components/TheFooter.vue(5 hunks)components/common/ConnectWallet/WalletAssetIdentity.vue(1 hunks)components/common/ConnectWallet/WalletAssetMenu.vue(4 hunks)components/landing/FarcasterBanner.vue(1 hunks)components/navbar/ExploreDropdown.vue(1 hunks)components/navbar/MobileExpandableSection.vue(2 hunks)components/navbar/ProfileDropdown.vue(4 hunks)components/navbar/ShoppingCartButton.vue(2 hunks)composables/useTheme.ts(2 hunks)libs/ui/src/components/NeoModalHead/NeoModalHead.vue(2 hunks)nuxt.config.ts(1 hunks)package.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- components/common/ConnectWallet/WalletAssetIdentity.vue
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: shard (1, 10)
- GitHub Check: Cloudflare Pages: koda-art-prod
🔇 Additional comments (36)
package.json (1)
120-120: Dependency added for Nuxt Icon.This adds the
@nuxt/icondependency (version 1.10.3) to support the transition from NeoIcon to Nuxt Icon component across the application.nuxt.config.ts (1)
279-279: Module added for Nuxt Icon integration.The
@nuxt/iconmodule has been added to the Nuxt configuration, which is consistent with the dependency added in package.json. This enables the use of the Icon component throughout the application.composables/useTheme.ts (3)
21-21: Updated icon naming pattern for system mode.The icon identifier has been changed from 'computer-classic' to 'i-mdi:desktop-classic' to follow the Nuxt Icon naming convention, which uses prefixes to specify the icon library.
32-32: Updated icon naming pattern for light mode.The icon identifier has been changed from 'sun' to 'i-mdi:weather-sunny' to follow the Nuxt Icon naming convention.
37-37: Updated icon naming pattern for dark mode.The icon identifier has been changed from 'moon' to 'i-mdi:moon-waning-crescent' to follow the Nuxt Icon naming convention.
components/ColorModeButton.vue (1)
9-9: Replaced NeoIcon with Nuxt Icon component.The
NeoIconcomponent has been replaced with theIconcomponent from @nuxt/icon, and the property has been updated from:iconto:nameto match the new component's API.The removal of the NeoIcon import from the script section and corresponding change to the Icon component is consistent with the refactoring goal stated in the PR.
components/landing/FarcasterBanner.vue (2)
5-5: Clean UI improvement with flexbox alignment.Adding
items-centerto the flex container ensures proper vertical alignment of the icon and text, improving the visual balance of the banner.
13-16: Successful replacement of NeoIcon with Nuxt Icon component.The refactoring replaces the NeoIcon component with the Nuxt Icon component, correctly using the Material Design Icons format with the
i-mdi:prefix. The sizing and spacing withtext-2xl ml-2look appropriate.libs/ui/src/components/NeoModalHead/NeoModalHead.vue (3)
7-7: Improved button layout with flexbox properties.Adding
flex items-center gap-2creates better spacing and alignment between the icon and text in the button, enhancing the visual appearance and accessibility.
11-14: Successfully migrated from NeoIcon to Nuxt Icon.The replacement of NeoIcon with the Nuxt Icon component is correctly implemented with the proper Material Design icon name
i-mdi:chevron-leftand appropriate sizing class.
26-26: Clean import statement update.The import statement has been properly updated to remove the NeoIcon import, keeping only the necessary NeoCommonHead component.
components/navbar/ShoppingCartButton.vue (2)
11-14: Successful icon component replacement.The NeoIcon for the cart has been correctly replaced with the Nuxt Icon component using the appropriate Material Design icon name and size class.
22-26: Consistent icon component migration.The second icon replacement maintains consistency with the overall refactoring approach, properly using the Material Design icon format and preserving the existing functionality.
components/navbar/ExploreDropdown.vue (1)
14-18: Replaced inline SVG with standardized Icon component.This change simplifies the template by removing the inline SVG in favor of the more maintainable Nuxt Icon component. This improves code consistency across the codebase while maintaining the same visual appearance.
components/TheFooter.vue (3)
35-38: Icon component replacement looks goodThe NeoIcon component has been successfully replaced with the new Icon component with appropriate naming convention.
115-119: Good conversion to MDI icon formatThe sparkle icon has been appropriately converted to the Material Design Icons format
i-mdi:star-four-points. Nice job maintaining visual consistency while updating the component.
35-148:❓ Verification inconclusive
Verify icon consistency across dark/light modes
All icons have been successfully replaced with the Nuxt Icon component. However, it's worth verifying that these icons maintain proper visibility across both dark and light color modes.
🏁 Script executed:
#!/bin/bash # Check for any icon-specific styling or theme adaptations in CSS rg -A 3 "\.text-k-grey|\.dark|@apply dark:|\.light" --glob "*.css" --glob "*.scss"Length of output: 2654
Icon Styling Consistency Verification for Dark/Light Modes
Based on the grep results, the repository contains theme-specific styling (e.g., in
assets/styles/global.scss,libs/ui/src/scss/variable.scss, andlibs/ui/src/scss/tailwind.scss) that distinguishes between dark and light modes. The<Icon>components incomponents/TheFooter.vueare using thetext-k-greyclass, which is expected to be adjusted appropriately via the project's Tailwind setup or CSS variables.Please ensure that:
- The
text-k-greyutility provides sufficient contrast in both themes.- A quick manual visual check is performed in dark and light modes to confirm that the icons remain clearly visible and consistent.
components/navbar/MobileExpandableSection.vue (2)
9-13: Dynamic icon binding maintained correctlyGood job maintaining the dynamic binding for the icon property through
:name="icon".
15-18: Fixed icon styling and class consistencyThe icon styling with
text-2xlclass is consistently applied, and theicon--rightclass is preserved.components/common/ConnectWallet/WalletAssetMenu.vue (3)
15-18: Consistent text coloring maintainedGood job preserving the
text-k-greyclass while updating to the Icon component.
37-40: Consistent sizing for menu iconsGood job applying the
text-xlclass consistently to both the earth and cog-outline icons in the footer menu items.Also applies to: 68-71
79-79: Import statement properly updatedThe import statement has been correctly updated to remove the NeoIcon import while retaining the necessary components.
components/navbar/ProfileDropdown.vue (4)
27-31: Profile icon implementation simplifiedThe replacement of the image tag with an Icon component simplifies the implementation by removing the need for the
profileIconcomputed property that likely handled dark/light mode variations.
39-40: Inline icon usage is consistentThe inline Icon component usage for the earth icon is consistent in both dropdown menus.
Also applies to: 71-71
89-89: Import statement correctly updatedThe import statement has been properly updated to remove the NeoIcon component.
27-31:❓ Verification inconclusive
Verify accessibility of icon replacement
The replacement of an image with an icon might impact accessibility. Ensure that the Icon component provides appropriate aria attributes or that the parent element has descriptive text/labels.
🏁 Script executed:
#!/bin/bash # Check if the Icon component handles accessibility concerns rg -A 5 "Icon =" --glob "*.ts" --glob "*.vue" --glob "*.js" | grep -E "aria|role|label|title" # Check if there are existing aria attributes on icons elsewhere that should be applied here rg "<Icon.*aria" --glob "*.vue"Length of output: 347
Accessibility Concerns on Icon Replacement in ProfileDropdown.vue
The recent search didn’t reveal any usage of aria attributes, roles, or labels for the Icon component. Please manually verify that the Icon component either implements its own accessibility features (e.g., default aria-label/role) or that a descriptive alternative (such as a proper aria-label on a surrounding element) is provided. In case the Icon component lacks default accessibility support, address this by adding the appropriate attributes to ensure the icon is accessible.
- Location:
components/navbar/ProfileDropdown.vue, lines 27–31- Action: Verify the Icon component’s implementation (or its usage) for proper accessibility attributes.
components/Navbar.vue (5)
109-112: Icon replacement implementation looks goodThe replacement of NeoIcon with the new Icon component using the
i-mdi:firenaming convention is appropriate. The styling classes (text-k-primary text-xl ml-1) properly maintain the visual appearance.
188-188: Icon property updated correctlyThe
iconproperty has been properly updated to use the new icon format (i-mdi:earth), maintaining consistent icon replacement approach.
209-212: User profile icon replacement properly implementedThe NeoIcon for the user profile has been correctly replaced with the Icon component using the Material Design Icons format. The styling with
text-xl ml-1maintains the appropriate visual appearance.
214-217: Chevron icon replacement implemented properlyThe right chevron icon has been correctly replaced with the Icon component using MDI format. The class
text-2xl icon--rightensures proper sizing and positioning.
249-249: Import dependency updated correctlyThe import statement has been properly updated to remove the NeoIcon dependency while retaining NeoButton, which is still in use.
components/ColorModeSwitch.vue (5)
3-8: Mobile view icon replacement looks goodThe NeoIcon component for mobile view has been properly replaced with the Icon component, maintaining the dynamic binding to the current mode's icon. The click handler for switching modes has been preserved.
19-22: Dropdown trigger icon replacement implemented correctlyThe icon in the dropdown trigger has been properly replaced with the Icon component using the same dynamic binding pattern. The
text-xlclass ensures proper sizing in the UI.
34-37: Dropdown item icons replacement looks goodThe icons in dropdown items have been properly replaced with the Icon component, maintaining the same binding pattern to display different icons for each option.
45-45: Imports updated correctlyThe import statement has been properly updated to remove the NeoIcon dependency while retaining the necessary NeoDropdown and NeoDropdownItem components.
1-60:❓ Verification inconclusive
Verify icon implementation consistency across theme modes
The changes look good, but it's important to ensure that the icon definitions in the theme options (referenced by
currentMode.iconandoption.icon) have been updated to use the newi-mdi:prefix format. These icon values need to match the new Icon component's naming convention.
🏁 Script executed:
#!/bin/bash # Check if theme options have been updated with the new icon format echo "Looking for theme-related files that might contain icon definitions..." fd -t f -e ts -e js -e vue theme --exclude node_modules echo -e "\nChecking for theme option definitions with icon properties:" rg -A 10 "themeOptions|colorOptions" --type ts --type vue --type jsLength of output: 465
Icon Prefix Consistency Verification – Manual Check Required
The code changes in
ColorModeSwitch.vuelook correct from a UI standpoint. However, please verify that all theme icon definitions have been updated to the newi-mdi:prefix. In particular:
Theme Options Consistency: Ensure that the icon values in theme option objects (e.g., those defined in
composables/useTheme.ts) are now using thei-mdi:prefix.Search Method for Vue Files: Note that the previous search encountered an error with the
--type vueflag. When searching through Vue files, consider using a command like:fd -e vue | xargs grep "i-mdi:"This will help confirm that the new prefix is consistently applied across all relevant files.
Please manually verify these points to ensure consistency with the new Icon component naming convention.
|
@exezbcz ? |
|
gm these are off center |
@exezbcz Updated. In this PR, I'm using MDI icons. If you prefer another icon set, please let us know. List of available icons: https://icones.js.org/collection/mdi |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
pages/how-to-icon.vue (2)
36-39: Consider a more robust approach for custom size handlingThe current approach of splitting the size string with
size.split('-')[1]works for the current examples but might be fragile if more complex size patterns are used in the future.- :size="size.split('-')[1]" + :size="size.replace('fa-', '')"
45-61: Enhance documentation for size compatibilityThe script section clearly defines the supported sizes, but consider adding a comment to explain the mapping between
NeoIconsizes andKIconsizes to help future developers understand the compatibility.const sizes = [ 'default', 'small', 'medium', 'large', '2x', '3x', ] +// Map between NeoIcon's fa-* sizes and KIcon's numeric sizes const customSize = [ 'fa-2x', 'fa-3x', ]components/KIcon.vue (2)
19-26: Consider documenting size values with CSS class equivalentsThe size mapping is well-structured, but adding documentation about the actual pixel sizes or rem values these classes correspond to would improve maintainability.
const sizeMap = { - 'default': 'size-5', - 'small': 'size-4', - 'medium': 'size-6', - 'large': 'size-8', - '2x': 'size-10', // fa-2x - '3x': 'size-14', // fa-3x + // Maps to Tailwind CSS size classes + 'default': 'size-5', // 1.25rem (20px) + 'small': 'size-4', // 1rem (16px) + 'medium': 'size-6', // 1.5rem (24px) + 'large': 'size-8', // 2rem (32px) + '2x': 'size-10', // 2.5rem (40px) - equivalent to fa-2x + '3x': 'size-14', // 3.5rem (56px) - equivalent to fa-3x }
1-27: Consider adding support for custom numeric sizesThe component currently only supports a fixed set of predefined sizes. Consider adding support for custom sizes to make the migration from NeoIcon smoother and provide more flexibility.
Here's a suggested implementation that would support both predefined sizes and custom numeric values:
<template> <Icon :name="name" - :class="sizeMap[size]" + :class="getSizeClass(size)" /> </template> <script setup lang="ts"> withDefaults( defineProps<{ name: string - size?: 'default' | 'small' | 'medium' | 'large' | '2x' | '3x' + size?: string | number }>(), { size: 'default', }, ) const sizeMap = { 'default': 'size-5', 'small': 'size-4', 'medium': 'size-6', 'large': 'size-8', '2x': 'size-10', // fa-2x '3x': 'size-14', // fa-3x } +// Handle both predefined sizes and custom numeric values +const getSizeClass = (size: string | number): string => { + if (size in sizeMap) { + return sizeMap[size as keyof typeof sizeMap] + } + + // Handle numeric values or strings that can be converted to numbers + const numericSize = Number(size) + if (!isNaN(numericSize)) { + return `size-${numericSize}` + } + + // Handle 'fa-X' format + if (typeof size === 'string' && size.startsWith('fa-')) { + const faSize = size.replace('fa-', '') + return sizeMap[faSize as keyof typeof sizeMap] || `size-${faSize}` + } + + // Default fallback + return sizeMap.default +} </script>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
components/ColorModeButton.vue(1 hunks)components/ColorModeSwitch.vue(4 hunks)components/KIcon.vue(1 hunks)components/Navbar.vue(4 hunks)components/TheFooter.vue(5 hunks)components/base/MediaItem.vue(2 hunks)components/common/ConnectWallet/WalletAssetIdentity.vue(1 hunks)components/common/ConnectWallet/WalletAssetMenu.vue(4 hunks)components/create/CreateLanding.vue(4 hunks)components/landing/FarcasterBanner.vue(1 hunks)components/navbar/ExploreDropdown.vue(1 hunks)components/navbar/MobileExpandableSection.vue(2 hunks)components/navbar/ProfileDropdown.vue(4 hunks)components/navbar/ShoppingCartButton.vue(2 hunks)components/shared/DropUpload.vue(4 hunks)libs/ui/src/components/NeoModalHead/NeoModalHead.vue(2 hunks)pages/how-to-icon.vue(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (14)
- components/landing/FarcasterBanner.vue
- components/common/ConnectWallet/WalletAssetIdentity.vue
- components/navbar/ProfileDropdown.vue
- libs/ui/src/components/NeoModalHead/NeoModalHead.vue
- components/ColorModeButton.vue
- components/navbar/ExploreDropdown.vue
- components/navbar/MobileExpandableSection.vue
- components/navbar/ShoppingCartButton.vue
- components/create/CreateLanding.vue
- components/ColorModeSwitch.vue
- components/common/ConnectWallet/WalletAssetMenu.vue
- components/shared/DropUpload.vue
- components/base/MediaItem.vue
- components/Navbar.vue
⏰ Context from checks skipped due to timeout of 90000ms (16)
- GitHub Check: Redirect rules
- GitHub Check: Header rules
- GitHub Check: Pages changed
- GitHub Check: shard (10, 10)
- GitHub Check: shard (9, 10)
- GitHub Check: shard (8, 10)
- GitHub Check: shard (3, 10)
- GitHub Check: shard (6, 10)
- GitHub Check: shard (7, 10)
- GitHub Check: shard (2, 10)
- GitHub Check: shard (4, 10)
- GitHub Check: shard (5, 10)
- GitHub Check: shard (1, 10)
- GitHub Check: build
- GitHub Check: build-cf
- GitHub Check: Cloudflare Pages: koda-art-prod
🔇 Additional comments (8)
pages/how-to-icon.vue (1)
1-43: Great implementation of an icon comparison page!This is a well-structured demo page that visually compares the legacy
NeoIconwith the newKIconcomponent across different sizes. This will be helpful for developers to understand the visual differences and ensure consistency during migration.components/TheFooter.vue (5)
35-38: Successful icon migration from NeoIcon to KIconThis change correctly implements the migration from
NeoIconto the newKIconcomponent, with proper attribute renaming fromicontonameand updating the icon identifier to use the MDI format.
69-72: Consistent implementation of the icon migrationThe icon migration has been consistently applied throughout the file, maintaining proper formatting and class attributes.
104-107: Icon migration maintains visual consistencyThe icon change from
"arrow-up-right"to"i-mdi:arrow-top-right"maintains the same visual intent while adapting to the new MDI naming convention.
145-148: Consistent implementation throughout social links sectionThe migration pattern has been consistently applied to the social links section, maintaining the same styling and behavior.
115-119:❓ Verification inconclusive
Verify visual appearance of the star icon
The icon has been changed from a "sparkle" to a "star-four-points". Please verify that this new icon maintains the same visual intent and aesthetic as the previous one.
🏁 Script executed:
#!/bin/bash # Check if there are other instances of the sparkle/star icon for consistency rg -i "sparkle|star" --type vueLength of output: 61
Action required: Confirm visual consistency of updated star icon
The current use of
i-mdi:star-four-pointsappears to replace the previous "sparkle" icon. However, the initial grep command did not correctly search Vue files, as noted by the error regarding the file type. Please verify manually—or rerun the search with a corrected command (e.g., usingrg -i 'sparkle|star' -g '*.vue')—that the new icon is used consistently and maintains the intended visual and aesthetic style.components/KIcon.vue (2)
1-6: Good implementation of the Icon wrapper componentThe component template is clean and correctly passes the required props to the underlying Nuxt Icon component.
8-17: Well-defined props with TypeScript typingThe props are well-defined with proper TypeScript typing and default values. The use of
withDefaultswithdefinePropsis the correct pattern for Vue 3 composition API.
Oops, I accidentally removed the fix in the last commit 😅 Should be fixed now |
|
|
looks good, thank you! |
Thank you for your contribution to the Koda - Generative Art Marketplace.
👇 __ Let's make a quick check before the contribution.
PR Type
Context
Summary by CodeRabbit
New Features
KIconcomponent for improved icon rendering.Bug Fixes
NeoIconwithKIcon.Style