Conversation
There was a problem hiding this comment.
Pull request overview
This PR extends the frontend design system with a new ProgressV2 component (including overflow handling) and adds a new app-level TaskPopover UI that uses it, alongside a small time parsing utility and Tailwind source configuration updates.
Changes:
- Added
ProgressV2component + public exports in the design-system package. - Added
timeToDecimalHours("HH:MM")utility to support progress calculations. - Added a new
TaskPopovercomponent in the app and updated Tailwind CSS sources to scan design-system source for class usage.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/packages/design-system/src/utils/index.ts | Formatting tweaks + new timeToDecimalHours utility. |
| frontend/packages/design-system/src/components/progress-v2/types.ts | Introduces props type for the new ProgressV2 component. |
| frontend/packages/design-system/src/components/progress-v2/progress-v2.tsx | Implements new progress component (overflow + interval modes). |
| frontend/packages/design-system/src/components/progress-v2/index.ts | Barrel export for ProgressV2. |
| frontend/packages/design-system/src/components/index.ts | Re-exports ProgressV2 from the design-system components entrypoint. |
| frontend/packages/app/src/global.css | Adds Tailwind @source for design-system source scanning + whitespace cleanup. |
| frontend/packages/app/src/components/taskPopover.tsx | New popover UI showing task status + progress using ProgressV2. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
frontend/packages/design-system/src/components/progress-v2/progress-v2.tsx
Outdated
Show resolved
Hide resolved
frontend/packages/design-system/src/components/progress-v2/progress-v2.tsx
Outdated
Show resolved
Hide resolved
frontend/packages/design-system/src/components/progress-v2/index.ts
Outdated
Show resolved
Hide resolved
frontend/packages/design-system/src/components/progress-v2/progress-v2.tsx
Outdated
Show resolved
Hide resolved
frontend/packages/design-system/src/components/progress-v2/progress-v2.tsx
Outdated
Show resolved
Hide resolved
frontend/packages/design-system/src/components/progress-v2/progress-v2.tsx
Outdated
Show resolved
Hide resolved
frontend/packages/design-system/src/components/progress-v2/types.ts
Outdated
Show resolved
Hide resolved
frontend/packages/design-system/src/components/progress-v2/progress-v2.tsx
Outdated
Show resolved
Hide resolved
frontend/packages/design-system/src/components/progress-v2/progress-v2.tsx
Outdated
Show resolved
Hide resolved
2163f80 to
ef24201
Compare
frontend/packages/design-system/src/components/timesheet/rows/task/taskRow.tsx
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
ef24201 to
401190c
Compare
| return ( | ||
| <div className="p-3 rounded-xl shadow-2xl bg-surface-modal w-88"> | ||
| <div className="grid grid-cols-[min-content_auto] items-center gap-x-2 gap-y-1.5 mb-6"> | ||
| <TaskStatus status={status} className="col-start-1 col-end-2" /> | ||
| <div className="col-start-2 col-end-3 text-base font-semibold"> | ||
| {label} | ||
| </div> | ||
|
|
||
| {badges.length > 0 && ( | ||
| <div className="flex flex-wrap col-start-2 col-end-3 gap-1"> | ||
| {badges.map((badge, index) => ( | ||
| <Badge key={index} variant="subtle" size="md" prefix={badge.icon}> | ||
| {badge.text} | ||
| </Badge> | ||
| ))} | ||
| </div> | ||
| )} | ||
| </div> |
There was a problem hiding this comment.
Handle text overflow for very long labels or badge text; it should truncate properly.
| "Pending Review": "text-ink-gray-9", | ||
| Overdue: "text-ink-red-4", | ||
| Completed: "text-ink-gray-9", | ||
| Cancelled: "text-ink-gray-9", |
There was a problem hiding this comment.
Let’s use camelCase for keys and use a map to map them to their values.
There was a problem hiding this comment.
Hey @b1ink0, I deliberately removed the mapping - let's discuss this on call.
There was a problem hiding this comment.
Brief reasoning:
- There is no benefit of using one over the other, except using camel case creates an overhead
- The API returns Capital case, so we can just match the api
- We already have the Status type in app/types
- We don't need to export the map everywhere, just the type
- This was the existing model
| starred = false, | ||
| timeEntries, | ||
| onCellClick, | ||
| taskHoverContent, |
There was a problem hiding this comment.
Lets add a renderName format for any prop which is renders a JSX component.
Description
Screenshot/Screencast
Checklist
Partially addresses #830