Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
feat: create social network navigation alike on browse threads #168
feat: create social network navigation alike on browse threads #168
Changes from 6 commits
4e8e8b6
85161a7
79b1cf3
f216fe3
79fcfb9
00ce23a
80b226f
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Ensure that the newly added imports (
useEffect
,useRef
,useThread
) are utilized effectively in the component. The declaration ofinitialUrl
asnull
is a good practice for initialization, but consider encapsulating this within auseRef
for consistency and to avoid potential bugs in future modifications.Committable suggestion
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.
The state initialization correctly captures the initial state of the accordion based on the thread data. However, the first
useEffect
block that setsinitialUrl
could be optimized by checking forinitialUrl.current
instead ofinitialUrl
if changed to useuseRef
.Committable suggestion
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.
The third
useEffect
hook is intended to open the accordion when the active thread matches the current thread and it is not already open. However, the dependency array is empty, which means this effect will only run once at component mount. Consider addingactiveThread
andstate.isOpen
to the dependencies to ensure it reacts to changes.Committable suggestion