fix: show correct project size + refactor overview table #318
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hello @atrincas, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
This pull request focuses on improving the project details overview page. The primary goals are to display the correct project size and refactor the overview table for better usability and maintainability. I've introduced a new useProjectDetails hook to centralize project details management, updated the footer component to dynamically display the project size, and enhanced table row styling and interactions. Additionally, I've addressed navigation issues between tables, ensuring the project details state is maintained, and incorporated proper Next.js Link components for improved accessibility.
Highlights
- Project Size: The project size label in the footer is now dynamic, displaying the correct size based on the project data. This enhances the user experience by providing accurate information at a glance (client/src/containers/overview/project-details/footer/index.tsx, client/src/containers/overview/project-details/utils.ts).
- Code Refactoring: A new
useProjectDetailshook has been created to centralize the management of project details, promoting code reusability and maintainability (client/src/containers/overview/hooks.ts). - Table Improvements: The table rows have been restyled for better visual appeal and user interaction. Navigation between tables now preserves the project details state, ensuring a seamless user experience (client/src/containers/overview/table/view/key-costs/index.tsx, client/src/containers/overview/table/view/overview/index.tsx, client/src/containers/overview/table/view/scorecard-prioritization/index.tsx).
- Accessibility: Next.js Link components have been properly implemented, and accessibility improvements have been made throughout the affected components (client/src/containers/overview/project-details/footer/index.tsx).
Changelog
Click here to see the changelog
- client/src/containers/overview/hooks.ts
- Created a
useProjectDetailshook to manage project details state using Jotai. - The hook provides
projectDetails,setProjectDetails, andhandleOpenDetailsfor managing the project details modal.
- Created a
- client/src/containers/overview/project-details/footer/index.tsx
- Replaced
lucide-reactLink withnext/link. - Modified the Footer component to accept a
projectSizeprop to dynamically display the project size. - Updated DialogClose and Button components to use
asChildfor better composition with Next.js Link.
- Replaced
- client/src/containers/overview/project-details/index.tsx
- Imported
getProjectSizeLabelutility function. - Calculated
projectSizeLabelusinguseMemoand passed it to the Footer component. - Removed unused handleOpenDetails function.
- Imported
- client/src/containers/overview/project-details/utils.ts
- Added
getProjectSizeLabelfunction to format the project size label based on project data. - Imported
formatNumberto format the project size.
- Added
- client/src/containers/overview/table/index.tsx
- Imported and rendered the
ProjectDetailscomponent.
- Imported and rendered the
- client/src/containers/overview/table/view/key-costs/index.tsx
- Removed direct import and usage of
projectDetailsAtom. - Utilized the
useProjectDetailshook to get thehandleOpenDetailsfunction. - Extracted
getVisibleProjectIdslogic into a shared utility function. - Passed
visibleProjectIdsto thehandleOpenDetailsfunction. - Added styling to the projectName column to allow for dynamic width.
- Removed direct import and usage of
- client/src/containers/overview/table/view/overview/index.tsx
- Removed direct import and usage of
projectDetailsAtom. - Utilized the
useProjectDetailshook to get thehandleOpenDetailsfunction. - Extracted
getVisibleProjectIdslogic into a shared utility function. - Passed
visibleProjectIdsto thehandleOpenDetailsfunction. - Removed the ProjectDetails component from this file, as it's rendered at the TableVisualization level.
- Removed direct import and usage of
- client/src/containers/overview/table/view/scorecard-prioritization/index.tsx
- Removed direct import and usage of
projectDetailsAtom. - Utilized the
useProjectDetailshook to get thehandleOpenDetailsfunction. - Extracted
getVisibleProjectIdslogic into a shared utility function. - Passed
visibleProjectIdsto thehandleOpenDetailsfunction. - Added styling to the projectName column to show underline on hover.
- Removed the ProjectDetails component from this file, as it's rendered at the TableVisualization level.
- Removed direct import and usage of
- client/src/containers/overview/utils.ts
- Added
getVisibleProjectIdsfunction to extract visible project IDs from paginated data.
- Added
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
In realms of code, where projects reside,
A hook emerges, with details to confide.
From table's depths to footer's grace,
Project size finds its rightful place.
Accessibility's touch, a gentle guide,
Ensuring all can join the ride.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces a new useProjectDetails hook, improves the project details overview page by displaying the correct project size, refactors the overview table, and enhances navigation and accessibility. The changes are well-structured and address the issues outlined in the Jira tickets. However, there are a few areas where the code can be improved for better readability and maintainability.
Summary of Findings
- Redundant
ProjectDetailscomponent: TheProjectDetailscomponent is being rendered in multiple table components (OverviewTable,KeyCostsTable,ScoredCardPrioritizationTable) and inTableVisualization. This can lead to unexpected behavior and should be rendered in one place. - Inconsistent
onClickhandlers: TheonClickhandlers in the table rows are not consistent across all table components. Some components use an arrow function directly, while others call thehandleOpenDetailsfunction directly. Using thehandleOpenDetailsfunction directly is preferred for consistency and readability. - Missing type definitions: The
dataparameter in thegetVisibleProjectIdsfunction could benefit from more specific type definitions to improve type safety and code clarity.
Merge Readiness
The pull request is almost ready for merging. However, the redundant ProjectDetails component and inconsistent onClick handlers should be addressed before merging. The missing type definitions in the getVisibleProjectIds function should also be considered for future improvement. I am unable to approve this pull request, and recommend that others review and approve this code before merging.
| @@ -17,6 +18,7 @@ export default function TableVisualization() { | |||
| <ScoredCardPrioritizationTable /> | |||
| )} | |||
| {tableView === "key-costs" && <KeyCostsTable />} | |||
| <ProjectDetails /> | |||
There was a problem hiding this comment.
The ProjectDetails component is rendered here, but it's also rendered in other table components. It should be rendered in one place to avoid unexpected behavior. Consider removing it from here and rendering it only in the parent component or a dedicated container.
| <ScrollableTable> | ||
| <TableHeader className="sticky top-0"> |
There was a problem hiding this comment.
The ProjectDetails component is rendered here, but it's also rendered in other table components. It should be rendered in one place to avoid unexpected behavior. Consider removing it from here and rendering it only in the parent component or a dedicated container.
| @@ -203,7 +204,6 @@ export function ScoredCardPrioritizationTable() { | |||
| }} | |||
| /> | |||
| )} | |||
There was a problem hiding this comment.
The ProjectDetails component is rendered here, but it's also rendered in other table components. It should be rendered in one place to avoid unexpected behavior. Consider removing it from here and rendering it only in the parent component or a dedicated container.
| className="group cursor-pointer transition-colors hover:bg-big-stone-950" | ||
| onClick={() => | ||
| handleOpenDetails(row.original.id!, visibleProjectIds) | ||
| } | ||
| > |
There was a problem hiding this comment.
This onClick handler can be simplified by directly passing the handleOpenDetails function with the required arguments. This improves readability and consistency with other table components.
onClick={() =>
handleOpenDetails(row.original.id!, visibleProjectIds)
}| data?: | ||
| | ApiPaginationResponse<Project> | ||
| | PaginatedProjectsWithMaximums | ||
| | ApiPaginationResponse<ProjectKeyCosts>, |
There was a problem hiding this comment.
Consider adding more specific type definitions for the data parameter to improve type safety and code clarity. For example, you could use a union type of the possible data structures.
atrincas
force-pushed
the
TBCCT-329-fe-update-project-overview-detail-footer-by-project-size
branch
from
265a200 to
7109ebe
Compare
| ? { | ||
| minWidth: "fit-content", | ||
| maxWidth: "100%", |
There was a problem hiding this comment.
Any reason to not use tailwind classes here?
There was a problem hiding this comment.
Applying tailwind classes directly didn't work here
Project Details Overview Page: show correct project size + refactor overview table
Changes
useProjectDetailshook to centralize project details managementJira