Fix/blabsy w3ds logo #297
Fix/blabsy w3ds logo #297
Conversation
WalkthroughAdded an inline W3DS logo image to the login content area in login-main.tsx, alongside the existing rotated logo on the right. No logic, behavior, or public API changes; purely presentational. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
platforms/blabsy/src/components/login/login-main.tsx (2)
13-31: Close EventSource on unmount to prevent leaks and duplicate subscriptions.The SSE connection created in watchEventStream is never closed, which can leak connections and double-consume events after remounts/navigation.
Minimal refactor to return the EventSource and close it in the effect cleanup:
- function watchEventStream(id: string): void { + function watchEventStream(id: string): EventSource { const sseUrl = new URL( `/api/auth/sessions/${id}`, process.env.NEXT_PUBLIC_BASE_URL ).toString(); const eventSource = new EventSource(sseUrl); eventSource.onopen = (): void => { console.log('Successfully connected.'); }; eventSource.onmessage = async (e): Promise<void> => { const data = JSON.parse(e.data as string) as { token: string }; const { token } = data; - console.log(token); await signInWithCustomToken(token); }; + return eventSource; } - const getOfferData = async (): Promise<void> => { + const getOfferData = async (): Promise<EventSource | null> => { const { data } = await axios.get<{ uri: string }>( new URL( '/api/auth/offer', process.env.NEXT_PUBLIC_BASE_URL ).toString() ); console.log('getting offer data'); setQr(data.uri); - watchEventStream( - new URL(data.uri).searchParams.get('session') as string - ); + return watchEventStream( + new URL(data.uri).searchParams.get('session') as string + ); }; useEffect(() => { - getOfferData() - .then((): void => { - console.log('QR code data fetched successfully.'); - }) - .catch((error): void => { - console.error('Error fetching QR code data:', error); - }); + let es: EventSource | null = null; + getOfferData() + .then((returnedEs): void => { + es = returnedEs; + console.log('QR code data fetched successfully.'); + }) + .catch((error): void => { + console.error('Error fetching QR code data:', error); + }); + return () => { + es?.close(); + }; }, []);Also applies to: 45-53
27-29: Remove all console.log(token) calls to prevent leaking auth tokens
Logging the raw auth token is a security risk. Please remove every occurrence:• platforms/blabsy/src/components/login/login-main.tsx (line 27)
• platforms/blabsy-w3ds-auth-api/src/controllers/AuthController.ts (line 60)Apply these diffs:
--- a/platforms/blabsy/src/components/login/login-main.tsx +++ b/platforms/blabsy/src/components/login/login-main.tsx @@ -25,7 +25,6 @@ const { token } = data; - console.log(token); await signInWithCustomToken(token); };--- a/platforms/blabsy-w3ds-auth-api/src/controllers/AuthController.ts +++ b/platforms/blabsy-w3ds-auth-api/src/controllers/AuthController.ts @@ -58,7 +58,6 @@ const token = await this.authService.createSession(user); - console.log(token); this.eventEmitter.emit(session, { token }); return { session, token }; }After applying these changes, re-run the search below to confirm no remaining token-logging calls:
rg -n --no-ignore-vcs -S 'console\.log\(.{0,10}token' -A 2 || true
🧹 Nitpick comments (2)
platforms/blabsy/src/components/login/login-main.tsx (2)
113-118: Make the bottom logo decorative for screen readers (alt='' + aria-hidden).If this logo is purely decorative (no additional information), avoid redundant announcements for assistive tech by making it decorative.
Apply this diff:
- <Image - src='/assets/w3dslogo.svg' - alt='W3DS logo' - width={100} - height={20} - /> + <Image + src='/assets/w3dslogo.svg' + alt='' + aria-hidden='true' + width={100} + height={20} + />
97-103: DRY: Consider a small W3DSLogo component to keep alt/size consistent.Since the same logo is used in two places, a tiny wrapper component reduces duplication and avoids future inconsistencies (alt text, dimensions).
Example component:
import Image from 'next/image'; import type { ComponentProps } from 'react'; type Props = Omit<ComponentProps<typeof Image>, 'src' | 'width' | 'height'> & { decorative?: boolean; }; export function W3DSLogo({ decorative, alt, ...rest }: Props) { return ( <Image src="/assets/w3dslogo.svg" width={100} height={20} alt={decorative ? '' : alt ?? 'W3DS logo'} aria-hidden={decorative ? true : undefined} {...rest} /> ); }Also applies to: 113-118
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
platforms/blabsy/src/components/login/login-main.tsx(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
platforms/blabsy/src/components/login/login-main.tsx (1)
platforms/pictique/src/lib/types.ts (1)
Image(73-76)
🔇 Additional comments (1)
platforms/blabsy/src/components/login/login-main.tsx (1)
113-118: LGTM: Inline W3DS logo addition matches PR intent.The added logo at the bottom of the intro text satisfies the PR objective and is implemented consistently with the existing usage.
Description of change
adds the w3ds logo back at the bottom of the intro text
Issue Number
n/a (294, dont close it has multiple)
Type of change
How the change has been tested
manual
Change checklist
Summary by CodeRabbit