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

Close related section #1207

Merged
merged 5 commits into from
May 31, 2024
Merged

Conversation

tsmith123
Copy link
Contributor

Fixes #837

Description

This PR ensures that the Related Posts section is closed once a comment is made.

This issue took me a while to find but eventually found the problem within Accordion component.

  • Made use of activeKey and onSelect props
  • Implemented onSelect handler to update activeKey value
  • Set activeKey value on load
  • Use null instead of undefined (Accordion doesn't seem to behave when using undefined)

Video

Screen.Recording.2024-05-29.at.11.54.24.mov

Testing

Please QA this if possible as I may have overlooked something.

Copy link
Member

@ekzyis ekzyis left a comment

Choose a reason for hiding this comment

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

Looks good except one two things, see comments

components/item-full.js Outdated Show resolved Hide resolved
@tsmith123
Copy link
Contributor Author

Thanks @ekzyis - good spot on the conditional. I've put that back. My testing was done with the proper conditional btw.

@huumn huumn merged commit 0576716 into stackernews:master May 31, 2024
6 checks passed
@tsmith123 tsmith123 deleted the close-related-section branch November 21, 2024 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Related section not collapsed after first reply
3 participants