Skip to content

Bug/fix library jsx use effect issues - #161

Merged
LOGESH-B merged 3 commits into
gbowne1:masterfrom
BlackBond06:Bug/fix-library-jsx-useEffect-issues
Sep 1, 2023
Merged

Bug/fix library jsx use effect issues#161
LOGESH-B merged 3 commits into
gbowne1:masterfrom
BlackBond06:Bug/fix-library-jsx-useEffect-issues

Conversation

@BlackBond06

Copy link
Copy Markdown
Collaborator

This PR resolves issue #160 opened by @gbowne1.

Changes Made:

  • Added setFilter prop to Library component.
  • Added clean up function to the useEffect to clean up setMyRows and setFilter functions when component unmounts.
  • Added 'myRows' to the dependency array of useEffect.

@BlackBond06
BlackBond06 requested a review from gbowne1 as a code owner August 31, 2023 16:30
@gbowne1
gbowne1 requested a review from LOGESH-B August 31, 2023 17:34
@gbowne1 gbowne1 added the bug Something isn't working label Aug 31, 2023
@gbowne1 gbowne1 added this to the Frontend milestone Aug 31, 2023
@gbowne1 gbowne1 linked an issue Aug 31, 2023 that may be closed by this pull request

@gbowne1 gbowne1 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

  • Checked out this change locally for testing and review
  • Manually reviewed browser console for errors/warnings

This change does appear to fix the useEffect issue #161 and this change also does not appear to cause additional errors.

Thank you for the opportunity to review your PR for merging.

Approving for merge. Looking forwards to seeing some working tests with Jest, etc.

@LOGESH-B LOGESH-B left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hii @BlackBond06
Thanks for your contribution
Approving for merge🙌

@LOGESH-B
LOGESH-B merged commit 6a9112f into gbowne1:master Sep 1, 2023
@gbowne1

gbowne1 commented Sep 1, 2023

Copy link
Copy Markdown
Owner

@BlackBond06 That was your 5th successful merge. I sent you a Collaborator invite. Thanks so much for all of your great contributions. Welcome aboard.

@LOGESH-B

LOGESH-B commented Sep 1, 2023

Copy link
Copy Markdown
Collaborator

Welcome buddy @BlackBond06 🤝🙌

@BlackBond06

Copy link
Copy Markdown
Collaborator Author

Yay!!! Thank you @gbowne1 @LOGESH-B

@gbowne1

gbowne1 commented Sep 1, 2023

Copy link
Copy Markdown
Owner

You're welcome! @BlackBond06

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

Labels

bug Something isn't working

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

[BUG] Library.jsx has some useEffect issues after today's merge(s).

3 participants