Improve footer animations - smooth accordion transitions#4049
Improve footer animations - smooth accordion transitions#4049khushal-winner wants to merge 9 commits intoOWASP:mainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdded viewport-driven mobile detection and refactored the Footer’s collapsible sections to use grid-template-rows transitions, conditional ARIA attributes, an inert wrapper for hidden mobile content, and a reorganized link container/chevron structure while keeping link semantics and bottom content unchanged. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file
Confidence score: 3/5
- There is a concrete user-facing risk: footer links can remain hidden on desktop when
openSectionis null because inlinegridTemplateRowsoverrides thelg:grid-rows-[1fr]rule infrontend/src/components/Footer.tsx. - Score reflects a moderate regression risk in a visible UI area, though the confidence is low based on the report’s uncertainty.
- Pay close attention to
frontend/src/components/Footer.tsx- inline grid row styles may override desktop layout and keep sections collapsed.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="frontend/src/components/Footer.tsx">
<violation number="1" location="frontend/src/components/Footer.tsx:33">
P2: Inline `gridTemplateRows` style overrides `lg:grid-rows-[1fr]`, so sections stay collapsed on desktop when `openSection` is null, hiding footer links.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
frontend/src/components/Footer.tsx (1)
75-75:mb-0on the social icons container is a no-op.
margin-bottom: 0is the browser default fordiv; removing it simplifies the class list with no visual change.🧹 Suggested cleanup
- <div className="mb-0 flex flex-row justify-center gap-6"> + <div className="flex flex-row justify-center gap-6">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/Footer.tsx` at line 75, Remove the redundant mb-0 utility from the social icons container's className in Footer.tsx (the div with className "mb-0 flex flex-row justify-center gap-6"); update that element to omit mb-0 so the class list becomes "flex flex-row justify-center gap-6" to simplify styling with no visual change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/components/Footer.tsx`:
- Around line 44-47: The inline style setting gridTemplateRows is overriding
Tailwind's responsive class and keeping sections collapsed on desktop; replace
the style usage in the Footer component by toggling Tailwind classes based on
openSection (e.g., add a conditional class like "grid-rows-[1fr]" vs
"grid-rows-[0fr]" or "grid-rows-0" on the element that currently uses style={{
gridTemplateRows: ... }}), keeping the existing responsive utility
"lg:grid-rows-[1fr]" so the lg: rule can win via cascade; update any references
to openSection and section.title to compute the className string instead of
using the inline style.
- Around line 49-68: The collapsed grid (grid-rows-[0fr]) + the inner wrapper
(the div with class "overflow-hidden" that wraps the links rendered by
section.links.map) leaves links tabbable; when collapsed make that inner wrapper
removed from the accessibility tree by toggling visibility/inert only when
collapsed on mobile. Update the collapse state logic used for the grid rows to
also add either visibility:hidden / visibility:visible (or Tailwind's
invisible/visible classes) or set the inert attribute on the inner wrapper div
when the grid is collapsed (and ensure this only applies for the
mobile/collapsed variant, not when lg:grid-rows-[1fr] is open) so hidden links
are not keyboard-focusable or exposed to assistive tech.
---
Nitpick comments:
In `@frontend/src/components/Footer.tsx`:
- Line 75: Remove the redundant mb-0 utility from the social icons container's
className in Footer.tsx (the div with className "mb-0 flex flex-row
justify-center gap-6"); update that element to omit mb-0 so the class list
becomes "flex flex-row justify-center gap-6" to simplify styling with no visual
change.
|
i have passsed all the tests and reopened the PR |
|
Hi @khushal-winner, closing and reopening the PR isn’t the best approach. If the PR is still in progress, you can mark it as draft and switch it to “Ready for review” once it’s complete. |
Please complete this form before requesting a review. Also, there are some unaddressed review comments from the AI bots that need to be resolved first. |
okay thank you :) |
i was exactly doing these things that's why i closed the PR for a while |
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="frontend/src/components/Footer.tsx">
<violation number="1" location="frontend/src/components/Footer.tsx:51">
P1: `inert` is applied whenever `openSection` doesn’t match the section, but `openSection` starts as `null` and never changes on desktop. Because the layout forces sections open at `lg`, all visible links are inert and unusable on desktop by default.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/components/Footer.tsx`:
- Around line 49-51: The inert attribute is being applied unconditionally
causing desktop links to be non-interactive; add an SSR-safe media-query flag
(e.g., isMobile state initialized false, updated in a useEffect with
window.matchMedia("(max-width:1023px)") listener) inside the Footer component
and change the inert prop to apply only when isMobile is true and openSection
!== section.title (i.e., inert={isMobile && openSection !== section.title ? true
: undefined}); ensure the listener is cleaned up on unmount so behavior updates
when resizing across the lg breakpoint.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/components/Footer.tsx (1)
44-65:⚠️ Potential issue | 🟡 MinorAlign
aria-expandedwith the actual visibility state on desktop
lg:grid-rows-[1fr]keeps sections visually open on desktop, butaria-expandedcan still befalse, which is misleading for assistive tech. Consider gatingaria-expanded(and optionallyaria-controls) to mobile-only so it reflects the real collapsed/expanded state.🔧 Suggested adjustment
- aria-expanded={openSection === section.title} - aria-controls={`footer-section-${section.title}`} + aria-expanded={isMobile ? openSection === section.title : undefined} + aria-controls={isMobile ? `footer-section-${section.title}` : undefined}Based on learnings: When implementing dropdown menus or similar interactive components, always include proper accessibility features: ARIA attributes (aria-expanded, aria-haspopup, aria-controls), keyboard navigation support (Enter, Space, Escape keys), and mechanisms to close dropdowns when clicking outside.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/Footer.tsx` around lines 44 - 65, The aria-expanded (and aria-controls) on the Footer Button can be inaccurate on desktop because sections are forced open with Tailwind's lg:grid-rows; update the Button to only expose expand/collapse ARIA attributes when the UI is actually collapsible by adding a viewport check (e.g., a useMediaQuery or window.matchMedia for the lg breakpoint) and use that boolean (e.g., isMobileOrNarrow) to gate aria-expanded and aria-controls: keep aria-expanded={openSection === section.title} and aria-controls only when isMobileOrNarrow is true, otherwise set aria-expanded to true (or omit the attribute) so assistive tech reflects the real desktop state; reference the Button element, toggleSection, openSection, and the id template footer-section-{section.title} when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@frontend/src/components/Footer.tsx`:
- Around line 44-65: The aria-expanded (and aria-controls) on the Footer Button
can be inaccurate on desktop because sections are forced open with Tailwind's
lg:grid-rows; update the Button to only expose expand/collapse ARIA attributes
when the UI is actually collapsible by adding a viewport check (e.g., a
useMediaQuery or window.matchMedia for the lg breakpoint) and use that boolean
(e.g., isMobileOrNarrow) to gate aria-expanded and aria-controls: keep
aria-expanded={openSection === section.title} and aria-controls only when
isMobileOrNarrow is true, otherwise set aria-expanded to true (or omit the
attribute) so assistive tech reflects the real desktop state; reference the
Button element, toggleSection, openSection, and the id template
footer-section-{section.title} when making the change.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/components/Footer.tsx (1)
52-57:⚠️ Potential issue | 🟡 MinorUse a single chevron and rotate it to avoid flicker.
The icon still swaps between
FaChevronUpandFaChevronDown, so the flicker noted in#4048can remain. Consider a singleFaChevronDownand rotate it on open.🔁 Suggested change
- <div className="transition-transform duration-200 lg:hidden"> - {openSection === section.title ? ( - <FaChevronUp className="h-4 w-4" /> - ) : ( - <FaChevronDown className="h-4 w-4" /> - )} - </div> + <div + className={`transition-transform duration-200 lg:hidden ${ + openSection === section.title ? 'rotate-180' : '' + }`} + > + <FaChevronDown className="h-4 w-4" /> + </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/Footer.tsx` around lines 52 - 57, Replace the conditional swap of FaChevronUp/FaChevronDown with a single FaChevronDown and rotate it based on the open state to avoid flicker: in the Footer component where openSection === section.title is checked (the block that currently renders FaChevronUp or FaChevronDown), always render FaChevronDown and apply a CSS transform class (e.g., rotate-180 when open) that toggles based on the same condition (openSection === section.title) so the icon rotates instead of being swapped.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/components/Footer.tsx`:
- Around line 60-61: The ID built for footer sections uses section.title
directly (id={`footer-section-${section.title}`}) which yields invalid IDs with
spaces (e.g., "OWASP Nest") and breaks the aria-controls reference; fix by
slugifying section.title (e.g., section.title.toLowerCase().replace(/\s+/g,
'-')) when constructing the id and ensure the corresponding aria-controls value
(the control that references `footer-section-...`) uses the same slugified
string so IDs and ARIA references match and contain no spaces or invalid
characters.
---
Outside diff comments:
In `@frontend/src/components/Footer.tsx`:
- Around line 52-57: Replace the conditional swap of FaChevronUp/FaChevronDown
with a single FaChevronDown and rotate it based on the open state to avoid
flicker: in the Footer component where openSection === section.title is checked
(the block that currently renders FaChevronUp or FaChevronDown), always render
FaChevronDown and apply a CSS transform class (e.g., rotate-180 when open) that
toggles based on the same condition (openSection === section.title) so the icon
rotates instead of being swapped.
|
@coderabbitai please review |
|
✅ Actions performedReview triggered.
|
|
Hi @khushal-winner their are 4 new issues that are showing by sonarqubecloud please fix them all before asking for review and while you are fixing and everything is done keep the PR in draft state only make it ready to review once all the check are passed and all the issue's are resolved and always run |
|
Hi @khushal-winner you also need to resolve the SonarQube warnings shown in the report 
|
- Remove unused FaChevronUp import - Replace window with globalThis for better portability - Replace replace() with replaceAll() for ES2021 compliance
- Make header Button non-interactive on desktop (sections always open) - Add conditional rendering: interactive Button on mobile, static div on desktop - Derive sectionId and isOpen variables to eliminate code duplication - Optimize performance by reducing redundant string operations - Improve maintainability with single source of truth for section logic
khushal-winner
force-pushed
the
footer-animation-improvements
branch
from
fd36e57 to
306d353
Compare
anurag2787
left a comment
anurag2787
left a comment
There was a problem hiding this comment.
Hi @khushal-winner have you run make check-test locally?
kasya
left a comment
kasya
left a comment
There was a problem hiding this comment.
@khushal-winner thanks for working on this!
As Anurag mentioned above - it seems that you haven't ran make check-test. I can see some checks are failing.Please run those and address all failing cases.
It is required to run this command before pushing anything to remote and before we actually review.
|
@khushal-winner hi! Do you plan to address requested changes? If not I'll have to reassign the issue. |
@kasya yeah sorry i will address the changes within 24 hrs |
- Replace custom mobile detection logic with useIsMobile hook - Remove duplicate useEffect and useState for mobile detection - Fix import order to satisfy ESLint rules - Apply code formatting to meet project standards Addresses code review feedback about using existing hooks instead of custom implementation.
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="frontend/src/components/Footer.tsx">
<violation number="1" location="frontend/src/components/Footer.tsx:15">
P1: Breakpoint mismatch between `useIsMobile()` and `lg`-based footer visibility can make footer links inaccessible on tablet widths.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
- Change lg:grid-rows-[1fr] to md:grid-rows-[1fr] to align with useIsMobile hook - Ensures footer links are accessible on tablet screens (768px-1023px) - Resolves accessibility issue where links were hidden on tablet breakpoints Fixes P1: Breakpoint mismatch between useIsMobile() and lg-based footer visibility
|
|
@kasya @arkid15r @anurag2787 can u check is this fine now? |
kasya
left a comment
kasya
left a comment
There was a problem hiding this comment.
hi @khushal-winner ! The transition works well 👍🏼
However, please run make check-test and address found issues. Both checks and tests are failing now.
Also, please resolve the conflict you have on this branch 👌🏼
@kasya , i have ran tests and they're passing it. |
@khushal-winner no, they are not. Also, the Conflict is still there - you need to resolve it. Normally, I would have reassigned this already, but this was an issue that you found and created. So i'm giving you another chance to update this. If not, I'll have to reassign this to someone else. You're not following out guidelines. |
anurag2787
left a comment
anurag2787
left a comment
There was a problem hiding this comment.
@khushal-winner are you still planning to work on this?
|
@kasya , pls assign this to @anurag2787 if he is ok with it or leave it for a new comer it is good first issue, i am not able to spare time for this focused on other things |
|
This is a good first issue so it can be assigned to newcomers and If needed i am also happy to take up this issue |
5 participants
Add the PR description here.
Proposed change
Resolves #4048
Replaces the janky
max-heightaccordion animation in the Footer component with agrid-template-rowstransition (0fr→1fr). The previous approach animated through an arbitrary max value (384px) with no knowledge of the content's actual height, causing a jerky, rushed collapse/expand. The new approach lets the browser resolve the natural height automatically, producing a smooth, fluid animation.Also replaces the
FaChevronUp/FaChevronDownicon swap with a singleFaChevronDownthat rotates 180° on open, eliminating a subtle flicker on toggle.Changes:
components/Footer.tsx: switched accordion frommax-htogrid-rowsanimation, simplified chevron to a single rotating iconvideo demo
2026-02-24.10-27-07.mp4
Checklist
make check-testlocally: all warnings addressed, tests passed