CC-110: --footer-bar-height CSS Variable Incorrect#31
Conversation
Resolves CC-110
WalkthroughThis PR refactors the FooterBar component's sizing and layout mechanisms. The height measurement was updated to use the ResizeObserver's border-box dimensions instead of content-rect, and the default reserved height in the theme CSS was reset from 4rem to 0. The SCSS layout was reorganized: footer-bar padding-bottom was simplified, footer-bar-content margin and padding logic was adjusted to handle borders and safe-area insets in a consolidated way, and the mobile variant's padding calculation was refactored to use an explicit horizontal padding variable. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Stylelint (17.12.0)src/style/variables.scssConfigurationError: Could not find "stylelint-config-clean-order". Do you need to install the package or use the "configBasedir" option? src/components/FooterBar/FooterBar.module.scssConfigurationError: Could not find "stylelint-config-clean-order". Do you need to install the package or use the "configBasedir" option? 🔧 ESLint
ESLint install failed due to a network error. 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/FooterBar/FooterBar.tsx`:
- Around line 37-38: The FooterBar currently sets a global CSS var
'--footer-bar-height' in the ResizeObserver callback (in the FooterBar
component) but unconditionally clears it on cleanup; change this to track
ownership so the variable is only removed when the last FooterBar unmounts: add
a module-level counter (e.g., footerBarInstanceCount) that you increment when
the component mounts and decrement in the cleanup, set the CSS variable when the
observer reports a height (same code: entry.borderBoxSize?.[0]?.blockSize ??
entry.target.getBoundingClientRect().height) and only remove/reset
'--footer-bar-height' in the cleanup when footerBarInstanceCount reaches zero;
ensure all references are done inside the FooterBar component's effect/observer
lifecycle so other instances retain the variable while active.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e8869b7d-a77b-4793-b728-fc2888305909
📒 Files selected for processing (3)
src/components/FooterBar/FooterBar.module.scsssrc/components/FooterBar/FooterBar.tsxsrc/style/variables.scss
| const height = entry.borderBoxSize?.[0]?.blockSize ?? entry.target.getBoundingClientRect().height; | ||
| document.documentElement.style.setProperty('--footer-bar-height', `${height}px`); |
There was a problem hiding this comment.
Prevent shared --footer-bar-height from being cleared by the wrong instance.
This component writes a global CSS variable, but cleanup removes it unconditionally. With the default now 0, unmounting one FooterBar can collapse layout even if another FooterBar is still mounted. Track ownership (or active instance count) before removing the variable.
Suggested fix (instance-count guard)
+let activeFooterBars = 0;
+
export const FooterBar = ({
children,
className,
maxWidth = '100vw',
mobile = false,
portalTarget = document.body,
}: FooterBarProps): ReactElement => {
const contentRef = useRef<HTMLDivElement>(null);
useEffect(() => {
+ activeFooterBars += 1;
const content = contentRef.current;
if (!content) {
return;
}
const observer = new ResizeObserver(([entry]) => {
const height = entry.borderBoxSize?.[0]?.blockSize ?? entry.target.getBoundingClientRect().height;
document.documentElement.style.setProperty('--footer-bar-height', `${height}px`);
});
observer.observe(content);
return () => {
observer.disconnect();
- document.documentElement.style.removeProperty('--footer-bar-height');
+ activeFooterBars -= 1;
+ if (activeFooterBars === 0) {
+ document.documentElement.style.removeProperty('--footer-bar-height');
+ }
};
}, []);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/FooterBar/FooterBar.tsx` around lines 37 - 38, The FooterBar
currently sets a global CSS var '--footer-bar-height' in the ResizeObserver
callback (in the FooterBar component) but unconditionally clears it on cleanup;
change this to track ownership so the variable is only removed when the last
FooterBar unmounts: add a module-level counter (e.g., footerBarInstanceCount)
that you increment when the component mounts and decrement in the cleanup, set
the CSS variable when the observer reports a height (same code:
entry.borderBoxSize?.[0]?.blockSize ??
entry.target.getBoundingClientRect().height) and only remove/reset
'--footer-bar-height' in the cleanup when footerBarInstanceCount reaches zero;
ensure all references are done inside the FooterBar component's effect/observer
lifecycle so other instances retain the variable while active.
Resolves CC-110
Summary by CodeRabbit