Skip to content

refactored code to trigger less re-rendering#632

Merged
prakashchoudhary07 merged 4 commits intoRealDevSquad:developfrom
bksh05:fix/stop-unnecessary-rerendering-of-issue-list
Jun 22, 2023
Merged

refactored code to trigger less re-rendering#632
prakashchoudhary07 merged 4 commits intoRealDevSquad:developfrom
bksh05:fix/stop-unnecessary-rerendering-of-issue-list

Conversation

@bksh05
Copy link

@bksh05 bksh05 commented Jun 13, 2023

Issue:

Fix the code to render IssueList in /issues route.

Description:

On every input change on the search field, the issueList was being re-rendered. This was because the IssueList components was controlling the state of the SearchField component as well.
I fixed this issue by refactoring the code and moving the search field-related text to the SearchField component.

Fixes #605

Anything you would like to inform the reviewer about:

Dev Tested:

  • Yes

Images/video of the change:

Before the changes, the profiler shows it was re-rendered every time I typed something in the text box.
image

After the change, the profiler shows no re-rendering of the IssueList while changing the search field text

image

Follow-up Issues (if any)

@vercel
Copy link

vercel bot commented Jun 13, 2023

@bksh05 is attempting to deploy a commit to the RDS-Team Team on Vercel.

A member of the Team first needs to authorize it.

yesyash
yesyash previously approved these changes Jun 15, 2023
Copy link
Contributor

@shubhamsinghbundela shubhamsinghbundela left a comment

Choose a reason for hiding this comment

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

LGTM

@bksh05 bksh05 requested a review from Pratiyushkumar June 15, 2023 16:19
Pratiyushkumar
Pratiyushkumar previously approved these changes Jun 17, 2023
Copy link
Contributor

@Pratiyushkumar Pratiyushkumar left a comment

Choose a reason for hiding this comment

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

I am approving it but please take care of my comment as well please

@bksh05 bksh05 changed the base branch from develop to main June 19, 2023 13:42
@bksh05 bksh05 dismissed stale reviews from shubhamsinghbundela, Pratiyushkumar, and yesyash June 19, 2023 13:42

The base branch was changed.

@bksh05 bksh05 changed the base branch from main to develop June 19, 2023 13:42
Pratiyushkumar
Pratiyushkumar previously approved these changes Jun 19, 2023
Copy link
Contributor

@Pratiyushkumar Pratiyushkumar left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@RitikJaiswal75 RitikJaiswal75 left a comment

Choose a reason for hiding this comment

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

Please put in the test coverage report, in case if there is none, please write them

@bksh05
Copy link
Author

bksh05 commented Jun 20, 2023

Please put in the test coverage report, in case if there is none, please write them

image
image
image
image

@RitikJaiswal75 The test is above 85% of the component that I worked with.
My changes do not add any functionality. It's just some refactoring to reduce re-rendering. Hence I did not write a test for that.

@bksh05 bksh05 requested a review from RitikJaiswal75 June 20, 2023 11:54
@shubhamsinghbundela shubhamsinghbundela added bug Something isn't working react and removed bug Something isn't working labels Jun 21, 2023
Copy link
Contributor

@shubhamsinghbundela shubhamsinghbundela left a comment

Choose a reason for hiding this comment

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

Great work @bksh05

@RitikJaiswal75
Copy link
Contributor

Are these multiple re-renders visible visually if yes please add a video of that, can you please add a video of changes after your code change

@bksh05
Copy link
Author

bksh05 commented Jun 21, 2023

Are these multiple re-renders visible visually if yes please add a video of that, can you please add a video of changes after your code change

Hey @RitikJaiswal75,
These are not visible visually on the screen. You can see them only when you run performance tools.
After fixing I have pasted a screenshot which shows it is not re-rendering anymore even if I have inputted long string in the textbox.

I have updated the PR description that shows multiple re-renders when we type something in the search field.

@prakashchoudhary07 prakashchoudhary07 merged commit b4a2589 into RealDevSquad:develop Jun 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix the code to render IssueList in /issues route.

6 participants