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.

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

Nahiyan_Added a "Edit Tasks" button to reduce the number of requests #2013

Merged
merged 14 commits into from
Jul 9, 2024

Conversation

Nahiyan-16
Copy link
Contributor

@Nahiyan-16 Nahiyan-16 commented Mar 1, 2024

Description

Added this button because before on-page initialization, the app would have to make too many unnecessary requests to the backend. Now when the users want to edit tasks, they may click the new button which will then make all the requests to the backend needed.

Related PRS (if any):

This frontend PR is related to the dev backend

Main changes explained:

-Added a "Edit tasks" button
-Should be disabled on page load
-When clicked should allow the user to edit tasks

How to test:

  1. check into current branch
  2. do npm install and npm run start:local to run this PR locally
  3. Clear site data/cache
  4. log as owner user
  5. go to dashboard→ reports→ Projects -> any project
  6. verify that edit tasks button is there
  7. verify that edit button is hidden for each task
  8. verify that when clicking on edit tasks button, it enables the edit button next to each tasks
  9. look at network tab to see if requests are being made after clicking on edit tasks button

Screenshots or videos of changes:

Before clicking "Edit Tasks" button
Screenshot 2024-03-01 180758

After clicking "Edit Tasks" button
Screenshot 2024-03-01 180829

Added this button because before on page initialization, the app would have to make too many unnecessary requests to the backend. Now when the users wants to edit tasks, they may click the new button which will then make all the requests to the backend needed.
Copy link

@MALAV1308 MALAV1308 left a comment

Choose a reason for hiding this comment

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

Hello , I reviewed your PR and I can not able to see tasks in task menu and got some warnings. Please look into it and solve it.
image

Copy link
Contributor

@abdel-lall abdel-lall left a comment

Choose a reason for hiding this comment

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

This PR has console errors and unnecessary logs.
Screenshot 2024-03-02 225922

@Nahiyan-16
Copy link
Contributor Author

This PR has console errors and unnecessary logs. Screenshot 2024-03-02 225922

@abdel-lall @MALAV1308 Thanks for the reviews and I have fixed the console log errors.
Screenshot 2024-03-04 123151

Copy link

@meetpadhiar4 meetpadhiar4 left a comment

Choose a reason for hiding this comment

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

Hi, I tested your PR and have encountered few issues. The tasks are not loading on the project info page and when I click the edit task button, no requests are being sent to the server. Please refer the video attached.

PR.2013.mov

Copy link
Contributor

@Pandani07 Pandani07 left a comment

Choose a reason for hiding this comment

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

Im not able to see the tasks and clicking on edit tasks sends no request to the network

Screen.Recording.2024-03-05.at.10.03.38.PM.mov

@dhairya-mehta
Copy link

Hey, I observed no changes after clicking on Edit tasks
image

Copy link
Contributor

@iven-yao iven-yao left a comment

Choose a reason for hiding this comment

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

I just reviewed this pull request
I logged in as an owner role, go to a project reports from Reports->Projects.

  • there's an Edit Tasks button
  • Edit button of each task is disabled --> Not disabled but hidden
  • after clicking Edit Tasks, making more request and show the edit button for each task

please make sure if we need task button for each task being hidden or disabled before clicking edit tasks button.
image
image

@Nahiyan-16
Copy link
Contributor Author

I just reviewed this pull request I logged in as an owner role, go to a project reports from Reports->Projects.

  • there's an Edit Tasks button
  • Edit button of each task is disabled --> Not disabled but hidden
  • after clicking Edit Tasks, making more request and show the edit button for each task

please make sure if we need task button for each task being hidden or disabled before clicking edit tasks button. image image

Yes, the Edit Tasks button should be hidden. The implementation was correct but the description was not.

@Nahiyan-16 Nahiyan-16 requested a review from iven-yao April 1, 2024 17:30
iven-yao
iven-yao previously approved these changes Apr 1, 2024
Copy link
Contributor

@iven-yao iven-yao left a comment

Choose a reason for hiding this comment

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

after clarifying the request, I approve this change according to previous review

@Gayathr-2012
Copy link
Contributor

Gayathr-2012 commented Apr 1, 2024

I just followed this steps it works perfect as expected.
I logged in as an owner role, go to a project reports from Reports->Projects.

there's an Edit Tasks button
Edit button of each task is disabled --> Not disabled but hidden
after clicking Edit Tasks, making more request and show the edit button for each task
please make sure if we need task button for each task being hidden or disabled before clicking edit tasks button.

but I noticed when I clicked on console I saw error on user profile. please correct it.

image image image image image

meetpadhiar4
meetpadhiar4 previously approved these changes Apr 2, 2024
Copy link

@meetpadhiar4 meetpadhiar4 left a comment

Choose a reason for hiding this comment

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

Hi, I reviewed your PR and it is working as intended.

kylepham
kylepham previously approved these changes Apr 2, 2024
Copy link
Contributor

@kylepham kylepham left a comment

Choose a reason for hiding this comment

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

PR worked as described.

Screen.Recording.2024-04-02.at.2.48.54.PM.mov

Tiantian-C

This comment was marked as resolved.

Copy link

@Tiantian-C Tiantian-C left a comment

Choose a reason for hiding this comment

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

The function works,while it has some unexpected errors in the console.
Screenshot 2024-04-03 at 11 51 30 PM

Copy link

@SRamen1999 SRamen1999 left a comment

Choose a reason for hiding this comment

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

Requests are being made after clicking on edit tasks button. However I am also getting an error in the console.

Screen.Recording.2024-04-05.at.11.24.31.AM.mov

This is the error
Screen Shot 2024-04-05 at 11 25 05 AM

anandsesha
anandsesha previously approved these changes Apr 6, 2024
Copy link
Contributor

@anandsesha anandsesha left a comment

Choose a reason for hiding this comment

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

Hi, I've reviewed your PR and the changes work as expected.

  • Logged in as Owner user, verified the Edit Task button is there and each Edit button is hidden, which is only enabled on clicking Edit Task.
  • Verified Networks tab to see requests made when Edit Task is clicked

Screenshot (753)
Screenshot (754)
Screenshot (755)

aaryaneil
aaryaneil previously approved these changes Apr 14, 2024
Copy link
Contributor

@aaryaneil aaryaneil left a comment

Choose a reason for hiding this comment

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

I have reviewed this PR and the changes work as intended.

Screen.Recording.2024-04-14.at.1.26.06.AM.mov

jennZhang93
jennZhang93 previously approved these changes Apr 18, 2024
Copy link
Contributor

@jennZhang93 jennZhang93 left a comment

Choose a reason for hiding this comment

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

Tested with the owner account and all features described in the PR worked. Good job!

Screenshot 2024-04-17 at 9 13 53 PMScreenshot 2024-04-17 at 9 13 58 PM

@Chuehleo Chuehleo added High Priority - Please Review First This is an important PR we'd like to get merged as soon as possible and removed High Priority - Please Review First labels May 1, 2024
cgomezhub
cgomezhub previously approved these changes May 7, 2024
Copy link
Contributor

@cgomezhub cgomezhub left a comment

Choose a reason for hiding this comment

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

The functionality is working, well done!.
the UI needs general improvements

https://www.loom.com/share/cebfd39908934a7d9196f89d365d2180?sid=c0e9e4f4-c524-427a-8116-da762663a033

@ZhangPeizhou
Copy link
Contributor

PR#2013
The code works as expected

Copy link
Contributor

@JatinAgrawal94 JatinAgrawal94 left a comment

Choose a reason for hiding this comment

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

Edit Sign is visible and is working.
Console has an error: "Warning: validateDOMNesting(...):

cannot appear as a child of ."
At some page sizes the headings in task table overlap with each other.
Number of requests do not increase to a large number on clicking edit button.
Rest everything works fine.
Great work!

PR.2013.mp4

JatinAgrawal94
JatinAgrawal94 previously approved these changes Jun 10, 2024
Copy link
Contributor

@akshit0211 akshit0211 left a comment

Choose a reason for hiding this comment

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

Hi, the functionality is working as expected but there are some warnings in the console. Please look into this.

edit tasks edit button warnings

@one-community
Copy link
Member

Thank you all, merging!

@one-community one-community merged commit fe996e1 into development Jul 9, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
High Priority - Please Review First This is an important PR we'd like to get merged as soon as possible
Projects
None yet
Development

Successfully merging this pull request may close these issues.