-
Notifications
You must be signed in to change notification settings - Fork 108
Fix Github Button Link #176
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
Conversation
WalkthroughAdds a click handler to the GitHub button, removes Lenis setup and unused comments from App, and refactors the WebGL background component (geometry, shader noise, rendering loop, input/visibility handling, resize logic, and public props). Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant BrowserWindow
participant ThreadsComponent
participant WebGLRenderer
participant Shader
User->>BrowserWindow: move mouse / resize / click
BrowserWindow->>ThreadsComponent: deliver events (mouse coords, resize, visibility)
ThreadsComponent->>WebGLRenderer: update uniforms (uMouse, iResolution, devicePixelRatio)
ThreadsComponent->>WebGLRenderer: requestAnimationFrame -> animate()
alt visible
WebGLRenderer->>Shader: draw frame with updated uniforms (hash noise, line loop)
Shader-->>WebGLRenderer: fragment output
WebGLRenderer-->>BrowserWindow: canvas pixels drawn
else not visible
ThreadsComponent->>WebGLRenderer: skip rendering (IntersectionObserver short-circuits)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
LandingPage/src/components/github.tsx (1)
9-14: Consider using an anchor tag instead of button for external navigation.While the current implementation works, using a
<button>for external navigation is not semantically correct. An anchor (<a>) tag would provide better accessibility, allow native browser features (right-click context menu, keyboard shortcuts), and improve SEO.Apply this diff to refactor to an anchor tag:
- <button onClick={openLink} className="btn-github"> + <a href="https://github.com/AOSSIE-Org/" target="_blank" rel="noopener noreferrer" className="btn-github"> <svg width={16} height={16} viewBox="0 0 16 16" fill="none" xmlns="http://www.w3.org/2000/svg"> <path d="M7.99992 1.33331C7.12444 1.33331 6.25753 1.50575 5.4487 1.84078C4.63986 2.17581 3.90493 2.66688 3.28587 3.28593C2.03563 4.53618 1.33325 6.23187 1.33325 7.99998C1.33325 10.9466 3.24659 13.4466 5.89325 14.3333C6.22659 14.3866 6.33325 14.18 6.33325 14C6.33325 13.8466 6.33325 13.4266 6.33325 12.8733C4.48659 13.2733 4.09325 11.98 4.09325 11.98C3.78659 11.2066 3.35325 11 3.35325 11C2.74659 10.5866 3.39992 10.6 3.39992 10.6C4.06659 10.6466 4.41992 11.2866 4.41992 11.2866C4.99992 12.3 5.97992 12 6.35992 11.84C6.41992 11.4066 6.59325 11.1133 6.77992 10.9466C5.29992 10.78 3.74659 10.2066 3.74659 7.66665C3.74659 6.92665 3.99992 6.33331 4.43325 5.85998C4.36659 5.69331 4.13325 4.99998 4.49992 4.09998C4.49992 4.09998 5.05992 3.91998 6.33325 4.77998C6.85992 4.63331 7.43325 4.55998 7.99992 4.55998C8.56659 4.55998 9.13992 4.63331 9.66659 4.77998C10.9399 3.91998 11.4999 4.09998 11.4999 4.09998C11.8666 4.99998 11.6333 5.69331 11.5666 5.85998C11.9999 6.33331 12.2533 6.92665 12.2533 7.66665C12.2533 10.2133 10.6933 10.7733 9.20659 10.94C9.44659 11.1466 9.66659 11.5533 9.66659 12.1733C9.66659 13.0666 9.66659 13.7866 9.66659 14C9.66659 14.18 9.77325 14.3933 10.1133 14.3333C12.7599 13.44 14.6666 10.9466 14.6666 7.99998C14.6666 7.1245 14.4941 6.25759 14.1591 5.44876C13.8241 4.63992 13.333 3.90499 12.714 3.28593C12.0949 2.66688 11.36 2.17581 10.5511 1.84078C9.7423 1.50575 8.8754 1.33331 7.99992 1.33331V1.33331Z" fill="currentcolor" /> </svg> <span>Contribute on Github</span> - </button> + </a>Then remove the
openLinkfunction (lines 4-6) as it would no longer be needed. You'll also need to update the CSS selector from.btn-githubbutton styles to work with anchor tags (addtext-decoration: none;anddisplay: inline-flex;ordisplay: flex;).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
LandingPage/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (1)
LandingPage/src/components/github.tsx(1 hunks)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
LandingPage/src/components/bg.tsx (2)
132-144: Unused props and refs - dead code.Several props and refs are defined but never used:
enableMouseInteraction(line 132) - not used anywheremaxDevicePixelRatio(line 133) - not passed to the renderermouseTargetRef(line 143) - initialized but never read/writtenmouseCurrentRef(line 144) - never updated, so mouse interaction is effectively disabled (always[0.5, 0.5])Either implement the mouse interaction logic or remove the unused code:
const Threads: React.FC<ThreadsProps> = ({ color = [1, 1, 1], amplitude = 1, distance = 0, - enableMouseInteraction = false, - maxDevicePixelRatio = 1.5, className, ...rest }) => { const containerRef = useRef<HTMLDivElement | null>(null); const rendererRef = useRef<Renderer | null>(null); const programRef = useRef<Program | null>(null); const meshRef = useRef<Mesh | null>(null); const animationFrameRef = useRef<number | null>(null); const visibleRef = useRef(true); - const mouseTargetRef = useRef<[number, number]>([0.5, 0.5]); - const mouseCurrentRef = useRef<[number, number]>([0.5, 0.5]);Also update the interface to remove
maxDevicePixelRatioif not implementing it.
167-171: Props used in uniforms won't update after mount.The
useEffecthas an empty dependency array[], butcolor,amplitude, anddistanceare used to initialize uniforms (lines 168-170). If these props change after mount, the WebGL uniforms won't reflect the new values.If props are expected to be static, this is fine. Otherwise, either:
- Add the props to the dependency array and handle re-initialization, or
- Update uniforms in the animation loop or a separate effect:
+ useEffect(() => { + if (!programRef.current) return; + const uniforms = programRef.current.uniforms; + (uniforms.uColor.value as Float32Array).set(color); + uniforms.uAmplitude.value = amplitude; + uniforms.uDistance.value = distance; + }, [color, amplitude, distance]);Also applies to: 221-221
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
LandingPage/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
LandingPage/src/App.tsx(1 hunks)LandingPage/src/components/bg.tsx(5 hunks)
✅ Files skipped from review due to trivial changes (1)
- LandingPage/src/App.tsx
🔇 Additional comments (4)
LandingPage/src/components/bg.tsx (4)
4-10: LGTM on the interface extension.Extending
React.ComponentPropsWithoutRef<"div">is a good pattern for forwarding div props.
41-56: LGTM on the noise function refactor.The
hash21andfastNoiseimplementation is a standard value noise approach with smooth Hermite interpolation. The early-exit optimization at lines 114-116 is a good addition for performance.
223-236: LGTM on the JSX structure.Good pattern for forwarding
classNameand merging styles with sensible defaults.
1-2: Changes appear unrelated to the PR objective.The PR title is "Fix Github Button Link" but this file (
bg.tsx) contains WebGL background rendering logic with no GitHub-related functionality. Consider verifying that this file change was intended to be part of this PR, or update the PR description to clarify the scope of changes.
| const m = program.uniforms.uMouse.value as Float32Array; | ||
| m.set(mouseCurrentRef.current); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mouse uniform always stays at default value.
mouseCurrentRef.current is read here but is never updated anywhere in the component, so uMouse will always be [0.5, 0.5]. If mouse interaction is intended, you need to add mouse event handlers to update the ref.
🤖 Prompt for AI Agents
In LandingPage/src/components/bg.tsx around lines 202-203, the code reads
mouseCurrentRef.current but never updates it so the uMouse uniform stays at the
default; add mouse event handlers (mousemove and touchmove) that update
mouseCurrentRef.current with normalized coordinates (e.g., [x / canvasWidth, y /
canvasHeight] or flipped Y if shader expects it), attach those listeners on
mount and remove on unmount (or when the canvas element changes), ensure the ref
is initialized and throttled/debounced if needed to avoid performance issues,
and update any resize handler so normalization uses the current canvas size.
📌 Issue Overview
The GitHub button link in the landing page header is incorrect and does not redirect to the intended repository. This causes users to land on the wrong page or receive an error.
🔍 Steps to Reproduce
🎯 Expected Behavior
The GitHub button should correctly redirect users to the proper GitHub repository.
🚨 Actual Behavior
The button currently points to an incorrect/invalid link, leading users to the wrong page or showing an error.
📷 Screenshot
💡 Suggested Improvements
Summary by CodeRabbit
Bug Fixes
New Features
Performance / Refactor
Chores
✏️ Tip: You can customize this high-level summary in your review settings.