-
Notifications
You must be signed in to change notification settings - Fork 64
Percentage #357
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
base: main
Are you sure you want to change the base?
Percentage #357
Conversation
|
Thank you for opening this PR! Before a maintainer takes a look, it would be really helpful if you could walk through your changes using GitHub's review tools. Please take a moment to:
More information on how to conduct a self review: This helps make the review process smoother and gives us a clearer understanding of your thought process. Once you've added your self-review, we'll continue from our side. Thank you! |
|
@ParasKhandelwal1616 please run |
f5864ee to
dd8a520
Compare
|
|
||
| export interface BottomBarProps { | ||
| projects: string[]; | ||
| projects: string[] | { label: string; value: string }[]; |
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.
value as a prop is a bit vague, needs to be renamed
| setIsLoading: (val: boolean) => void; | ||
| } | ||
| ) => { | ||
| interface Option { |
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.
all interfaces shall be moved to types.ts
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.
and Option seems to be vague as well. please use better variable names
| .sort((a, b) => (a > b ? 1 : -1)); | ||
| setUniqueProjects(filteredProjects); | ||
|
|
||
| const projectOptions = filteredProjects.map((project) => { |
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.
this filtering should be a single function perhaps. easy to test and fix if and when it breaks
its-me-abhishek
left a comment
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.
Please fix and also add some screenshots/videos to the PR description, for the changes
|
Please rename the PR title as well, and follow the commit conventions as set in CONTRIBUTING.MD while naming PRs and commits. That really helps in keeping the merge history clean. |
PR: Display Task Completion Metrics in Filter Dropdowns
1. Description
This PR implements a feature to display task completion percentages directly within the filter dropdowns (Project and Tag).
2. Motivation & Context
The Problem: Previously, users could only see the name of a filter, lacking immediate insight into task progress within that filter. This made it difficult to quickly gauge completion status without navigating into each filter.
3. Changes Implemented
This change enhances the filter dropdowns by showing the number of completed tasks and their percentage alongside the filter name. The stats update dynamically.
Visual Examples:
Project: Website Redesign (3/10 tasks completed, 30%)Tag: Urgent (5/8 tasks completed, 62%)4. User Impact
5. Type of Change