Skip to content
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.

Sign up for GitHub

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

Jump to bottom

New UI for task accordion #606

Merged
merged 13 commits into from
Nov 9, 2023
Merged

New UI for task accordion #606

merged 13 commits into from
Nov 9, 2023

Conversation

joyguptaa
Copy link
Contributor

@joyguptaa joyguptaa commented Nov 7, 2023 •
edited
Loading

Developer Name: Joy Gupta

PR Number(s): 606

Description:

Currently, the UI of the task accordion on the dashboard site does not seem to be satisfactory. It lacks in terms of UI, UX, color combinations, and readability. Additionally, pagination is implemented in the frontend.

image

Requirements:

To enhance the UI and UX of the accordion, the following modifications/improvements should be implemented:

  • The UI of each task card should match the one we have on the status site.
  • Instead of using next and previous buttons, implement infinite scroll with pagination being handled on the backend site.
  • Change the current API with the paginated one that we are using in my-site

UI Fixes/Improvements:

  • Replace assigned to with Created By
  • Add Priority field to the right
  • Conditional rendering of Progress based on Status
  • Add Type field to the right
  • Implement infinite scroll and pagination in the backend, instead of handling it in the frontend
  • Use the tasks UI from my site as it also serves the same purpose
  • Change the current API with paginated one

Issue Ticket Number:

Backend Changes

  • Yes
  • No

Frontend Changes

  • Yes
  • No

Is Under Feature Flag

  • Yes
  • No

Database changes:

  • Yes
  • No

Breaking changes (If your feature is breaking/missing something please mention pending tickets):

  • Yes
  • No

Deployment notes:

NA

Tested in local

  • Yes
  • No

QA Instructions, Screenshots, Recordings

Test cases added :
image
image

Test Stats:
Run yarn test __tests__/user-details/task-duedate-hover.test.js and there you can see the test case running.
image

Run yarn test and there you can see the test case running.
image

Working Proof

Functionality & Responsiveness

simplescreenrecorder-2023-11-08_09.46.56.mp4

@joyguptaa joyguptaa self-assigned this Nov 7, 2023
@joyguptaa
Copy link
Contributor Author

Some of the cards may appear red even though the progress is 100%. This is because I've implemented the same logic that is currently used on the status side. Additionally, the progress bar will persist no matter what the status is. This is because this page does not allow anyone to change it; it is only in read-only mode. Also, hiding it wouldn't make sense because the Super User wouldn't be able to find out the task completion percentage.

@Ajeyakrishna-k
Copy link
Contributor

Ajeyakrishna-k commented Nov 7, 2023 •
edited
Loading

Some of the cards may appear red even though the progress is 100%. This is because I've implemented the same logic that is currently used on the status side.

@Ajeyakrishna-k
Copy link
Contributor

Additionally, the progress bar will persist no matter what the status is. This is because this page does not allow anyone to change it; it is only in read-only mode. Also, hiding it wouldn't make sense because the Super User wouldn't be able to find out the task completion percentage.

We cannot update the status to DONE or VERIFIED unless the percentage is 100%. So even if you display that percentage it will be always 100% providing no extra value. That is the reason why it's proposed to remove the progress bar for other statuses in my-site and status site.

@Ajeyakrishna-k
Copy link
Contributor

Also please add more description. Currently it's not really clear what this PR does.

@shubhamsinghbundela
Copy link

  • Implement infinite scroll and pagination in the backend, instead of handling it in the frontend

Hi @ardourApeX, before I review your pull request, I'd like to clarify a few things to avoid any confusion:

Where is your backend pull request?
Why is this point included in the pull request description?

Could you please provide more information about these points? This will help me better understand your pull request.

@shubhamsinghbundela
Copy link

shubhamsinghbundela commented Nov 7, 2023 •
edited
Loading

what you done in this pr...please give more information in description(high level implementation in words please else not understand what you done in this pr)
easy for us to review your pr as a reviewer.

Where is your design doc???

@joyguptaa
Copy link
Contributor Author

joyguptaa commented Nov 8, 2023 •
edited
Loading

what you done in this pr...please give more information in description(high level implementation in words please else not understand what you done in this pr) easy for us to review your pr as a reviewer.

Where is your design doc???

While reviewing the issue ticket @iamitprakash suggested that design doc is not required as it just an UI update. Although I have updated the description for both issue and PR

@joyguptaa
Copy link
Contributor Author

  • Implement infinite scroll and pagination in the backend, instead of handling it in the frontend

Hi @ardourApeX, before I review your pull request, I'd like to clarify a few things to avoid any confusion:

Where is your backend pull request? Why is this point included in the pull request description?

Could you please provide more information about these points? This will help me better understand your pull request.

Also please add more description. Currently, it's not really clear what this PR does.

There are no backend changes related to this PR as I have just implemented an existing API. Also, I have updated both the PR and issue description. Let me know if anything else is required. Thank You!

@joyguptaa
Copy link
Contributor Author

Some of the cards may appear red even though the progress is 100%. This is because I've implemented the same logic that is currently used on the status side.

Given that we have not yet decided on the criteria for determining if a task will be considered overdue before the ETA, I have not implemented it at this time. The same we have discussed today.

@joyguptaa
Copy link
Contributor Author

Additionally, the progress bar will persist no matter what the status is. This is because this page does not allow anyone to change it; it is only in read-only mode. Also, hiding it wouldn't make sense because the Super User wouldn't be able to find out the task completion percentage.

We cannot update the status to DONE or VERIFIED unless the percentage is 100%. So even if you display that percentage it will be always 100% providing no extra value. That is the reason why it's proposed to remove the progress bar for other statuses in my-site and status site.

Done

sahsisunny
sahsisunny suggested changes Nov 8, 2023
View reviewed changes
Copy link
Contributor

@sahsisunny sahsisunny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test stats are useless showing coverage for mock data

@joyguptaa
Copy link
Contributor Author

joyguptaa commented Nov 8, 2023 •
edited
Loading

Test stats are useless showing coverage for mock data

Done

@joyguptaa joyguptaa requested a review from sahsisunny November 8, 2023 08:57
sahsisunny
sahsisunny reviewed Nov 8, 2023
View reviewed changes
utils.js Show resolved Hide resolved
@joyguptaa joyguptaa requested a review from sahsisunny November 8, 2023 13:02
@iamitprakash iamitprakash merged commit a717686 into Real-Dev-Squad:develop Nov 9, 2023
@joyguptaa joyguptaa mentioned this pull request Nov 9, 2023
Merged
12 tasks
heyrandhir
heyrandhir reviewed Nov 9, 2023
View reviewed changes
__tests__/taskRequests/taskRequestDetails.test.js
headless: true,
headless: 'new',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @ardourApeX ,
I'm curious about why we changed the headless mode from true to new. Could you please provide some context

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@heyrandhir heyrandhir heyrandhir left review comments

@shubhamsinghbundela shubhamsinghbundela shubhamsinghbundela approved these changes

@sahsisunny sahsisunny sahsisunny approved these changes

@iamitprakash iamitprakash iamitprakash approved these changes

@Ajeyakrishna-k Ajeyakrishna-k Awaiting requested review from Ajeyakrishna-k

Assignees

@joyguptaa joyguptaa

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@joyguptaa @Ajeyakrishna-k @shubhamsinghbundela @iamitprakash @sahsisunny @heyrandhir