Feature/f 39 implement generic accordion component #12
Conversation
There was a problem hiding this comment.
Overall good job! You should generally adapt to the code guidelines we set, I guess for the first time it is usually like that, so you code according to your own style, which is totally fine.
Few things I also noticed:
- Generally the text and the icon size is too small inside the cards
- I think the flex container inside the accordtion should have
justify-content: flex-startandalign-items: flex-start - The content inside the accordion when expanded overflows its container
Have a look at all comments and I hope the communication regarding the colors and theming comes soon.
| @@ -0,0 +1,38 @@ | |||
| 'use client'; | |||
There was a problem hiding this comment.
no need of client component here, you do not use any client side APIs
There was a problem hiding this comment.
as soon as remove the 'use client' the accordions as well as card doesn't get rendered.
There was a problem hiding this comment.
This is actually an open bug in NextUI, I had the same problem for the table. Seems like keeping 'use client' is neccesary for now: nextui-org/nextui#1403 (comment)
| @@ -1,9 +1,66 @@ | |||
| import { Chart } from '@/components/Charts/Chart'; | |||
There was a problem hiding this comment.
generally do not remove other components when adding yours here
There was a problem hiding this comment.
I removed that import because I was getting an "unused vars" error. Since the comments mentioned that page.tsx is intended for testing purposes, I decided to remove it.
… content, adjust operations
|
Can you also center the title of an accordion |
Thank you for the feedback, but i like them in this alignment. In Center they won't look good. |
I meant centering vertically, sorry for not clarifying |
✅ Deploy Preview for wfp-hungermap ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
ArmanpreetGhotra
deleted the
feature/f-39-implement-generic-accordion-component
branch
For testing, i added some icons under "Images" folder as svg and added some arguments for testing. i changed the accordion layout whenever there are more than 2 components.
Next todos would be:
right now there is image icon on some accordions. Plan is as soon as user hover over the info icon, information model appear and it's same as Information Popup. As soon as that is implemented, I'm going to integrate that.