Conversation
📝 WalkthroughWalkthroughLoadingIcon refactored from animated ChangesLoading Icon Redesign and Integration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/assets/icons/LoadingIcon.tsx (1)
11-21: ⚡ Quick winInline
<style>injects a global class into the document on every renderThe
<style>block is emitted as a DOM node each timeLoadingIconmounts. Multiple simultaneous instances (e.g., skeleton screens, storybook, tests) will accumulate duplicate style tags. More broadly,.nb-loader-birdis a flat global name — any future element using the same class name picks up the pulse animation unintentionally.Prefer moving the keyframe + class rule to a CSS/SCSS module (or the project's global stylesheet), and
import styles from './LoadingIcon.module.css'so the class name is locally scoped.♻️ Suggested approach (CSS module)
LoadingIcon.module.css:+@keyframes nb-loader-pulse { + 0%, 100% { transform: scale(0.85); opacity: 0.35; } + 50% { transform: scale(1); opacity: 1; } +} +.bird { + animation: nb-loader-pulse 1.75s ease-in-out infinite; + transform-origin: 50% 50%; + transform-box: fill-box; +}
LoadingIcon.tsx:+import styles from "./LoadingIcon.module.css"; ... - <style>{` - `@keyframes` nb-loader-pulse { … } - .nb-loader-bird { … } - `}</style> - <g className={"nb-loader-bird"}> + <g className={styles.bird}>🤖 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/assets/icons/LoadingIcon.tsx` around lines 11 - 21, The component LoadingIcon is injecting a <style> tag with global rules (keyframes nb-loader-pulse and .nb-loader-bird) on every mount which causes duplicate tags and global namespace collisions; move these declarations into a CSS module (e.g., create LoadingIcon.module.css containing the `@keyframes` nb-loader-pulse and a locally-scoped loader class) then update the LoadingIcon component to import styles from './LoadingIcon.module.css' and replace usages of "nb-loader-bird" with styles.someClassName so the animation is scoped and the style tag is no longer emitted at render time.
🤖 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/ui/FullScreenLoading.tsx`:
- Line 15: The SVG in LoadingIcon currently hardcodes path fill colors so the
parent className="fill-netbird" in FullScreenLoading is dead; update the
LoadingIcon component's <path> elements to use fill="currentColor" (or remove
presentation fill attributes) so the icon inherits color, then change the parent
usage in FullScreenLoading to apply a text color utility (e.g., replace
className="fill-netbird" with className="text-netbird" or otherwise set CSS
color) to control the brand color via CSS-driven theming.
---
Nitpick comments:
In `@src/assets/icons/LoadingIcon.tsx`:
- Around line 11-21: The component LoadingIcon is injecting a <style> tag with
global rules (keyframes nb-loader-pulse and .nb-loader-bird) on every mount
which causes duplicate tags and global namespace collisions; move these
declarations into a CSS module (e.g., create LoadingIcon.module.css containing
the `@keyframes` nb-loader-pulse and a locally-scoped loader class) then update
the LoadingIcon component to import styles from './LoadingIcon.module.css' and
replace usages of "nb-loader-bird" with styles.someClassName so the animation is
scoped and the style tag is no longer emitted at render time.
🪄 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: CHILL
Plan: Pro
Run ID: ad80c4e8-797d-4b66-8438-a54a8ea0bc5a
📒 Files selected for processing (2)
src/assets/icons/LoadingIcon.tsxsrc/components/ui/FullScreenLoading.tsx
| )} | ||
| > | ||
| <LoadingIcon className="fill-netbird" size={44} /> | ||
| <LoadingIcon className="fill-netbird" size={68} /> |
There was a problem hiding this comment.
fill-netbird is now dead code — the paths' explicit fill attributes block inheritance
Previously, the <rect> elements in LoadingIcon had no explicit fill, so they inherited the fill-netbird color from the parent SVG. The new <path> elements each carry hardcoded fill presentation attributes (#F68330, #F35E32). Presentation attributes contribute to the author level of the cascade with specificity 0, and SVG presentation attributes are overridden by any other style definitions, but they still apply directly to the element and beat inherited values from a parent. As a result, className="fill-netbird" has no visual effect on the rendered bird shapes.
Two paths forward:
Option A — Accept the hardcoded brand palette and drop the dead class:
- <LoadingIcon className="fill-netbird" size={68} />
+ <LoadingIcon size={68} />Option B — Restore CSS-driven theming by using currentColor in the paths:
- fill="#F68330"
+ fill="currentColor"Then control the color via a text-* Tailwind utility on the parent element (SVG fill inherits from CSS color when currentColor is used).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <LoadingIcon className="fill-netbird" size={68} /> | |
| <LoadingIcon size={68} /> |
🤖 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/ui/FullScreenLoading.tsx` at line 15, The SVG in LoadingIcon
currently hardcodes path fill colors so the parent className="fill-netbird" in
FullScreenLoading is dead; update the LoadingIcon component's <path> elements to
use fill="currentColor" (or remove presentation fill attributes) so the icon
inherits color, then change the parent usage in FullScreenLoading to apply a
text color utility (e.g., replace className="fill-netbird" with
className="text-netbird" or otherwise set CSS color) to control the brand color
via CSS-driven theming.
1 participant
Issue ticket number and link
Screen.Recording.2026-05-08.at.16.14.16.mov
Documentation
Select exactly one:
Docs PR URL (required if "docs added" is checked)
Paste the PR link from https://github.com/netbirdio/docs here:
https://github.com/netbirdio/docs/pull/__
Summary by CodeRabbit