✅ Deploy Preview for anu-vue ready!

Name	Link
🔨 Latest commit	1ea18d7
🔍 Latest deploy log	https://app.netlify.com/sites/anu-vue/deploys/636f504b6db2d0000af1e4b3
😎 Deploy Preview	https://deploy-preview-56--anu-vue.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

Hi,

Thanks so much for your continuous contribution. I really appreciate it.

I don't get enough time to review medium to big like PRs in one go so I will break my review into multiple comments so we can gradually update the PR.

BTW, your design sense is nice ❤️

Design

• Let's add a light variant in docs and make it the default variant
• Use i-bx-x for close icon in closable chips (makes it minimal)
• Chips aren't clickable except closable type and click event, so what's your opinion on removing states?
• Use outline icon if available

Docs

• We use - for icons instead of the colon. i-bx:bxs-user => i-bx-bxs-user
• API section is missing
• succes => success

Questions

• How well does it work with avatar?
  

If you want to make any updates after reading design section of contributing guide, feel free to discuss 😇

P.S. Will review your code later, good night buddy

Oh, you also added support for v-model, that's really nice 😊

I really liked the way you extracted closeChip in the function even if it was a single-line statement. This makes component markup cleaner 😍

Docs

• Add demo with useGroupModel (don't use same content as shown in below image)
  

• Update chip variant demo content

  content

   <template>
     <div class="flex gap-x-2">
       <AChip>Primary</AChip>
       <AChip variant="fill">
         Primary
       </AChip>
       <AChip variant="outline">
         Primary
       </AChip>
       <AChip variant="text">
         Primary
       </AChip>
     </div>
   </template>

• Update chip icon demo content

  Content

   <template>
     <div class="flex flex-wrap gap-2">
       <AChip icon="i-bx-user">
         <span>User</span>
       </AChip>
       <AChip
         color="success"
         icon="i-bx-bxs-battery-charging"
       >
         <span>Charging</span>
       </AChip>
       <AChip
         color="info"
         icon="i-bx-wrench"
       >
         <span>Settings</span>
       </AChip>
       <AChip
         color="warning"
         icon="i-bx-car"
       >
         <span>Traffic</span>
       </AChip>
       <AChip
         color="danger"
         icon="i-bx-trash"
       >
         <span>Remove</span>
       </AChip>
     </div>
   </template>
   

Design

• Just like the state, chip don't have cursor-pointer if it isn't clickable. Can you dynamically add cursor-pointer & states based on the click event attachment? (closable chips won't have this as only the close icon is clickable)

Code/API

• Do we need 1 div & 2 span wrapper for chip? Isn't 1 div wrapper enough?
• As per our Theme Preset rule, we should better move rounded-full and other UI related classes in preset and only keep structural classes like flex items-center in the component. (gap-x- as well will go in preset)
• Instead of class={'rounded-full'}, you can write class="rounded-full"
• emit events:
  • click:append-icon when appended icon is clicked
  • click:close when close icon is clicked

BTW, what do you prefer should I make changes myself when I get time or do you want to perform the changes I review?

Hi, thank you for great feedback. 🙏 It is very valuable for me.
I'm happy to perform the changes myself based on your review.

How well does it work with avatar?

<AChip class="gap-x-2 py-2">
  <AAvatar
    class="text-xs"
    src="/images/demo/portrait-1.jpg"
  />
  <span>Olivia</span>
</AChip>

if you adjust the gap and padding it looks like this

@jd-solanki How about naming event listeners close and click-append-icon?
I'm asking because when I have an event listener whose name contains a colon, for some reason I can't track via "attrs" if the listener is attached or not. Which would be useful for changing cursor to a pointer when hovering over an icon.

is event name click:appendIcon fine or do you want to change it to click:append-icon?

Thanks for the changes 😇

Oh, i forgot to add suggestion for changed highlighted lines

is event name click:appendIcon fine or do you want to change it to click:append-icon?

My testing shows that whether we use camelCase or snake_case, we can use attrs to see if the event listener is attached. However, as soon as we add a colon to the name, the listener does not appear in attrs, whether it is attached or not.

oh, we will use camelCase no worries. Moreover, this is default by this eslint rule 😊