-
Notifications
You must be signed in to change notification settings - Fork 21
feat: add reusable datalist component #312
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
feat: add reusable datalist component #312
Conversation
|
@Aqil-Ahmad is attempting to deploy a commit to the Hycom Team Team on Vercel. A member of the Team first needs to authorize it. |
|
/claim #305 |
|
👀 We've notified the reward creators here. |
|
@Aqil-Ahmad please run linter and fix the errors, it reports multiple issues both in the |
packages/blocks/invoice-list/src/frontend/InvoiceList.client.tsx
Outdated
Show resolved
Hide resolved
packages/blocks/invoice-list/src/frontend/InvoiceList.client.tsx
Outdated
Show resolved
Hide resolved
|
@Aqil-Ahmad also, please provide a short feedback as per ticket description, either in a comment or better yet in the PR description: |
Thanks for the feedback! I'll fix all linting and type errors in ui and frontend. The app starts fine (login page appears), but I need to set up authentication to test the actual list components. I'll investigate the seed scripts or test user setup. |
|
@Aqil-Ahmad Ok, so the frontend app itself runs fine when you start it directly ( As for authentication, it should be already pre-configured (we have a simple sqlite database set up with prisma), you can find the credentials in our docs |
3347ed4 to
fd2161c
Compare
packages/blocks/invoice-list/src/frontend/InvoiceList.client.tsx
Outdated
Show resolved
Hide resolved
packages/blocks/invoice-list/src/frontend/InvoiceList.client.tsx
Outdated
Show resolved
Hide resolved
packages/blocks/invoice-list/src/frontend/InvoiceList.client.tsx
Outdated
Show resolved
Hide resolved
packages/blocks/notification-list/src/frontend/NotificationList.client.tsx
Outdated
Show resolved
Hide resolved
|
@Aqil-Ahmad looks better, but please check my comments as I think this component can still be improved |
fd2161c to
5b1539c
Compare
thanks for the review! i've made the updates. let me know if there's anything else you'd like me to adjust |
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.
@Aqil-Ahmad looks almost ready, please check my replies to earlier comments for a few more changes and I think we will be ready to merge it (and submit reward of course)
5b1539c to
8c5def0
Compare
|
@Aqil-Ahmad looks good, we'll merge it once the reward is issued, hopefully in a couple of days |

What does this PR do?
Related Ticket(s)
Notion: Data tables refactor (issue #305 )
Key Changes
DataList
component in packages/ui/src/components/DataList/
with typed column renderers (text, badge, date, price, link, badgeStatus, custom).
NEW DataList in:
packages/blocks/ticket-list/src/frontend/TicketList.client.tsx
packages/blocks/order-list/src/frontend/OrderList.client.tsx
packages/blocks/invoice-list/src/frontend/InvoiceList.client.tsx
packages/blocks/notification-list/src/frontend/NotificationList.client.tsx
[docs/dev] Storybook stories for DataList:
packages/ui/src/components/DataList/DataList.stories.tsx
How to test
Storybook:
npm run storybookVerify all stories render (Basic Text, With Badges, With Actions, With Price, Custom Cell Renderer, Empty State, With Custom Styling).
App test:
Login with test users:
Verify list pages render with DataList:
Feedback
The monorepo is well structured with clear separation between /packages and /apps and setup was smooth after npm install. the workspace organization made sense once explored @o2s/ui, for shared components, and @o2s/blocks, for feature modules, but understanding the relationship between framework types and block-level API harmonization took a bit of time. Overall the project is well organized and maintainable. Prettier configuration was quite strict (good for consistency tho).
Media (Loom or gif)