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

fix: 🐛 Sidebar content should not hide the sidebar footer #1298

Merged
merged 1 commit into from
Oct 2, 2023

Conversation

matthieujacq
Copy link
Contributor

@matthieujacq matthieujacq commented Oct 2, 2023

Epic

#1231

Description

Fixes the issue where the sidebar footer is not visible if the content is too long (like a too large chat history).

Screenshots

Before (we cannot see the footer) :
image

After (we can see it and scroll through the content):
image

@matthieujacq matthieujacq added bug Something isn't working area: frontend Related to frontend functionality or under the /frontend directory labels Oct 2, 2023
@matthieujacq matthieujacq self-assigned this Oct 2, 2023
@vercel
Copy link

vercel bot commented Oct 2, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 2, 2023 1:30pm
quivr-strapi ✅ Ready (Inspect) Visit Preview Oct 2, 2023 1:30pm
quivrapp ✅ Ready (Inspect) Visit Preview Oct 2, 2023 1:30pm

@github-actions
Copy link
Contributor

github-actions bot commented Oct 2, 2023

Risk Level 2 - /home/runner/work/quivr/quivr/frontend/lib/components/Sidebar/Sidebar.tsx

The code seems to be well written and follows the SOLID principles. However, there is a potential issue with the useEffect hook. The pathname variable is included in the dependency array but it's not used inside the hook. This might cause unnecessary re-renders of the component. If pathname is not supposed to affect the state of open, it should be removed from the dependencies.

useEffect(() => {
  setOpen(!isMobile);
}, [isMobile]);

Also, the dragConstraints prop in the motion.div component is set to { right: 0, left: 0 }, which means the sidebar cannot be dragged. If this is the intended behavior, then the drag prop is unnecessary and can be removed. If the sidebar is supposed to be draggable, then the dragConstraints should be adjusted accordingly.


🔍🔄🚫


Powered by Code Review GPT

@mamadoudicko mamadoudicko marked this pull request as ready for review October 2, 2023 13:35
@matthieujacq matthieujacq merged commit 1e0c0c4 into main Oct 2, 2023
11 checks passed
@matthieujacq matthieujacq deleted the fix/sidebar-footer-sometimes-hidden branch October 4, 2023 07:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: frontend Related to frontend functionality or under the /frontend directory bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants