Skip to content

refactor(ui): polish code quality across components #175

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.

Sign up for GitHub

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

Merged
AnnatarHe merged 1 commit into from
Mar 21, 2026
Merged

refactor(ui): polish code quality across components #175

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions src/app/auth/github/content.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ function GithubOAuthContent(props: GithubOAuthContentProps) {
useAuthSuccessed(resp.called, resp.loading, resp.error, resp.data?.githubAuth)
useEffect(() => {
if (!code) {
console.log('no code')
// TODO: redirect
return
}

Expand Down
3 changes: 1 addition & 2 deletions src/app/dash/[userid]/home/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,7 @@ export async function generateMetadata(props: PageProps): Promise<Metadata> {
return profileGenerateMetadata({
profile: profileResponse.data!.me,
})
} catch (e) {
console.error(e)
} catch {
Copy link

@gemini-code-assist gemini-code-assist bot Mar 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

While removing console.error aligns with the PR's goal of cleaning up console output, completely swallowing the error here removes valuable debugging information. If fetching the profile fails, we won't know why. It's better to log the error to a dedicated logging service (like Sentry, if used in the project) instead of just catching and ignoring it.

return profileGenerateMetadata({})
}
}
Expand Down
4 changes: 1 addition & 3 deletions src/app/dash/[userid]/profile/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,12 @@ export async function generateMetadata(props: PageProps): Promise<Metadata> {
}

async function Page(props: PageProps) {
const [params, searchParams, ck, { t }] = await Promise.all([
const [params, , ck, { t }] = await Promise.all([
props.params,
props.searchParams,
cookies(),
getTranslation(),
])
const withProfileEditor = searchParams.with_profile_editor
const pathUid: string = params.userid
const myUidStr = ck.get(USER_ID_KEY)?.value
const myUid = myUidStr ? parseInt(myUidStr, 10) : undefined
Expand All @@ -75,7 +74,6 @@ async function Page(props: PageProps) {
},
})

console.log('withProfileEditor', withProfileEditor)
return (
<section className="w-full">
{/* Main profile section */}
Expand Down
4 changes: 2 additions & 2 deletions src/components/RichTextEditor/markdown-components.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@ const Heading = ({ level, children }: HeadingProps) => {
level === 6 && 'mb-4 text-base text-gray-800 dark:text-gray-200'
)

const Tag = `h${level}` as any
const Tag = `h${level}` as React.ElementType
return <Tag className={className}>{children}</Tag>
}

export const MarkdownComponents: any = {
export const MarkdownComponents: Record<string, React.FC<React.PropsWithChildren<Record<string, unknown>>>> = {
h1: ({
children,
...props
Expand Down
65 changes: 0 additions & 65 deletions src/components/auth.metamask.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,70 +1,5 @@
'use client'

/* METAMASK DISABLED - Package removed to prevent hydration errors
import { useSDK } from '@metamask/sdk-react'
// import MetamaskLogo from './icons/metamask.logo.svg'
import { useRouter } from 'next/navigation'
import { useCallback } from 'react'
import { toast } from 'react-hot-toast'
import { useAuthBy3rdPartSuccessed } from '../hooks/hooks'
import { useAuthByWeb3LazyQuery } from '../schema/generated'
import { signDataByWeb3 } from '../utils/wallet'
import MetamaskButtonView from './auth/metamask'

function AuthByMetamask() {
const router = useRouter()
const [doAuth, doAuthData] = useAuthByWeb3LazyQuery()
const { sdk: metamaskSDK } = useSDK()
// const err = hooks.useError()
const onMetamaskLogin = useCallback(async () => {
if (!metamaskSDK) {
return
}
try {
const res = await signDataByWeb3(metamaskSDK)
const r = await doAuth({
variables: {
payload: {
address: res.address!,
signature: res.signature!,
text: res.text,
},
},
})
if (r.data?.loginByWeb3.noAccountFrom3rdPart) {
router.push(
`/auth/callback/metamask?a=${res.address}&s=${res.signature}&t=${encodeURIComponent(res.text)}`
)
return
}

} catch (err: any) {
toast.error(err.message)
}
}, [doAuth, router, metamaskSDK])

useAuthBy3rdPartSuccessed(
doAuthData.called,
doAuthData.loading,
doAuthData.error,
doAuthData.data?.loginByWeb3
)

const disabled = doAuthData.loading

return (
<MetamaskButtonView
loading={doAuthData.loading}
onClick={onMetamaskLogin}
disabled={disabled}
/>
)
}

export default AuthByMetamask
*/

// Disabled MetaMask component - returns null
function AuthByMetamask() {
return null
}
Expand Down
7 changes: 3 additions & 4 deletions src/components/auth/apple-standalone.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,6 @@ export default function AppleStandaloneLoginButton({

onSuccess?.()
} catch (error) {
Copy link

@gemini-code-assist gemini-code-assist bot Mar 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Removing console.error is part of the PR's goal, but swallowing the error object completely can make debugging difficult. While an error message is shown to the user via toast, the full error context is lost. Consider logging the error object to a monitoring service for better error tracking.

console.error('Apple login error:', error)
const errorMessage =
error instanceof Error ? error.message : 'Login failed'
toast.error(`Apple login failed: ${errorMessage}`, { id: authToast })
Expand All @@ -111,9 +110,9 @@ export default function AppleStandaloneLoginButton({
}
}

const handleAppleError = (error: any) => {
console.error('Apple Sign-In error:', error)
toast.error(`Apple Sign-In failed: ${error?.error || 'Unknown error'}`)
const handleAppleError = (error: unknown) => {
const errorObj = error as Record<string, string> | null
toast.error(`Apple Sign-In failed: ${errorObj?.error || 'Unknown error'}`)
setIsLoading(false)
}

Expand Down
2 changes: 1 addition & 1 deletion src/components/auth/apple.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ type AppleLoginButtonViewProps = {
disabled?: boolean
onSuccess: (resp: AppleAuthResponse) => void

onError: (error: any) => void
onError: (error: unknown) => void
version?: 'v2' | 'v4'
}
const authOptions = {
Expand Down
130 changes: 0 additions & 130 deletions src/components/auth/metamask-standalone.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,140 +1,10 @@
'use client'

/* METAMASK DISABLED - Package removed to prevent hydration errors
import { useSDK } from '@metamask/sdk-react'
import * as sentry from '@sentry/react'
import { useRouter } from 'next/navigation'
import { useState } from 'react'
import toast from 'react-hot-toast'
import { syncLoginStateToServer } from '@/actions/login'
import { COOKIE_TOKEN_KEY, USER_ID_KEY } from '@/constants/storage'
import { useAuthByWeb3LazyQuery } from '@/schema/generated'
import { updateToken } from '@/services/ajax'
import profile from '@/utils/profile'
import { signDataByWeb3 } from '@/utils/wallet'
import MetamaskButtonView from './metamask'

interface MetaMaskStandaloneLoginButtonProps {
className?: string
onSuccess?: () => void
}

export default function MetaMaskStandaloneLoginButton({
className,
onSuccess,
}: MetaMaskStandaloneLoginButtonProps) {
const router = useRouter()
const [isLoading, setIsLoading] = useState(false)
const { sdk: metamaskSDK } = useSDK()
const [doWeb3Auth] = useAuthByWeb3LazyQuery()

const handleMetaMaskLogin = async () => {
if (!metamaskSDK) {
toast.error('MetaMask SDK not initialized')
return
}

setIsLoading(true)

try {
// Step 1: Connect wallet and sign message
const connectToast = toast.loading('Connecting to MetaMask...')
const walletData = await signDataByWeb3(metamaskSDK)
toast.success('Connected to MetaMask', { id: connectToast })

// Step 2: Authenticate with backend
const authToast = toast.loading('Authenticating...')
const authResponse = await doWeb3Auth({
variables: {
payload: {
address: walletData.address,
signature: walletData.signature,
text: walletData.text,
},
},
})

const loginData = authResponse.data?.loginByWeb3

if (!loginData) {
throw new Error('Authentication failed')
}

// Step 3: Handle new user flow
if (loginData.noAccountFrom3rdPart) {
toast.dismiss(authToast)
const { address, signature, text } = walletData
router.push(
`/auth/callback/metamask?a=${address}&s=${signature}&t=${encodeURIComponent(text)}`
)
return
}

// Step 4: Store authentication data
const { user, token } = loginData

await syncLoginStateToServer({ uid: user.id, token })

localStorage.setItem(
COOKIE_TOKEN_KEY,
JSON.stringify({
profile: user,
token,
createdAt: Date.now(),
})
)
sessionStorage.setItem(COOKIE_TOKEN_KEY, token)
sessionStorage.setItem(USER_ID_KEY, user.id.toString())

profile.token = token
profile.uid = user.id

sentry.setUser({
email: user.email,
id: user.id.toString(),
username: user.name,
})

updateToken(token)

toast.success('Successfully logged in!', { id: authToast })

// Step 5: Navigate to dashboard
setTimeout(() => {
const domain = user.domain.length > 2 ? user.domain : user.id
router.push(`/dash/${domain}/home?from_auth=1`)
}, 100)

onSuccess?.()
} catch (error) {
console.error('MetaMask login error:', error)
const errorMessage =
error instanceof Error ? error.message : 'Login failed'
toast.error(`MetaMask login failed: ${errorMessage}`)
} finally {
setIsLoading(false)
}
}

return (
<div className={className}>
<MetamaskButtonView
loading={isLoading}
disabled={isLoading}
onClick={handleMetaMaskLogin}
/>
</div>
)
}
*/

interface MetaMaskStandaloneLoginButtonProps {
className?: string
onSuccess?: () => void
}

// Disabled MetaMask component - returns null
// eslint-disable-next-line @typescript-eslint/no-unused-vars
export default function MetaMaskStandaloneLoginButton(
_props: MetaMaskStandaloneLoginButtonProps
) {
Expand Down
6 changes: 3 additions & 3 deletions src/components/auth/otp-box.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ type OTPBoxProps = {
autoValidate?: boolean
loading: boolean

onSubmit(val: string): Promise<any>
onSubmit(val: string): Promise<unknown>
}

// TODO: resend case
Expand All @@ -25,8 +25,8 @@ function OTPBox(props: OTPBoxProps) {
}
try {
await props.onSubmit(otp)
} catch (e: any) {
setHasErrorMsg(e.message)
} catch (e: unknown) {
setHasErrorMsg(e instanceof Error ? e.message : 'Unknown error')
}
}, [otp, props])

Expand Down
9 changes: 7 additions & 2 deletions src/components/avatar/avatar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,15 @@ function Avatar(props: AvatarProps) {
const { ref, isHovering } = useHover<HTMLDivElement>()

if (props.img === '') {
return (
return props.onClick ? (
<button
type="button"
className={`animate-pulse rounded-full border-none bg-gray-500 p-0 ${cls}`}
onClick={props.onClick}
/>
) : (
<div
className={`animate-pulse rounded-full bg-gray-500 ${cls}`}
onClick={props.onClick}
/>
)
}
Expand Down
9 changes: 3 additions & 6 deletions src/components/background-upload-modal/background-upload-modal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ const BackgroundUploadModal = ({
onClose,
onSave,
}: BackgroundUploadModalProps) => {
const [_selectedFile, setSelectedFile] = useState<File | null>(null)
const [, setSelectedFile] = useState<File | null>(null)
const [previewUrl, setPreviewUrl] = useState<string>('')
const [croppedUrl, setCroppedUrl] = useState<string>('')
const [isProcessing, setIsProcessing] = useState(false)
Expand Down Expand Up @@ -156,12 +156,10 @@ const BackgroundUploadModal = ({
onClose()
}, [previewUrl, croppedUrl, onClose])

// Mock API call function
const mockUploadBackground = useCallback(
async (imageUrl: string): Promise<void> => {
async (_imageUrl: string): Promise<void> => {
return new Promise((resolve) => {
setTimeout(() => {
console.log('Mock API: Background image uploaded:', imageUrl)
resolve()
}, 1000)
})
Expand All @@ -181,9 +179,8 @@ const BackgroundUploadModal = ({
onSave(croppedUrl)
toast.success('Background updated successfully!')
handleClose()
} catch (error) {
} catch {
Copy link

@gemini-code-assist gemini-code-assist bot Mar 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The error object is being swallowed here. While a generic failure toast is shown, logging the actual error to a service would be very helpful for debugging why the background update failed.

toast.error('Failed to update background')
console.error('Background upload error:', error)
} finally {
setIsProcessing(false)
}
Expand Down
5 changes: 3 additions & 2 deletions src/components/confirm-dialog/confirm-dialog.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,13 +120,14 @@ function ConfirmDialog({

return (
<>
<div
<button
ref={refs.setReference}
type="button"
onClick={() => setIsOpen(true)}
className="cursor-pointer"
>
{children}
</div>
</button>
Comment on lines +123 to +130
Copy link

@devin-ai-integration devin-ai-integration bot Mar 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Changing ConfirmDialog wrapper from <div> to <button> creates invalid nested buttons

The ConfirmDialog component's wrapper was changed from <div> to <button>, but all existing usages pass a <button> element as children. This creates invalid HTML with nested <button> elements, which is prohibited by the HTML spec and causes undefined browser behavior (e.g., click events may not propagate correctly, React hydration warnings).

Both callers pass button children

In src/app/dash/[userid]/comments/[commentid]/comment-header.tsx:82-122, a <button> is rendered inside ConfirmDialog's children.

In src/app/dash/[userid]/clippings/[clippingid]/@comments/comment.tsx:85-113, a <button> is also rendered inside ConfirmDialog's children.

Both result in <button><button>...</button></button> in the final DOM.

Prompt for agents 
In src/components/confirm-dialog/confirm-dialog.tsx lines 123-130, the wrapper element was changed from div to button, but the children passed by callers (comment-header.tsx:82 and comment.tsx:85) are already button elements. This creates invalid nested buttons. Either: (1) Revert the wrapper back to a div element with role='button' tabIndex={0} and onKeyDown for accessibility, OR (2) Keep the button wrapper but update all callers to pass non-interactive children (e.g. replace their <button> wrappers with <span> or <div>).
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +123 to +130
Copy link

@gemini-code-assist gemini-code-assist bot Mar 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Great job on improving accessibility by replacing the div with a button. However, the <button> element has default browser styling that might alter the appearance of its children. To ensure the UI remains consistent, it's best to reset the button's styles to make it a transparent wrapper.

Suggested change
<button
ref={refs.setReference}
type="button"
onClick={() => setIsOpen(true)}
className="cursor-pointer"
>
{children}
</div>
</button>
<button
ref={refs.setReference}
type="button"
onClick={() => setIsOpen(true)}
className="cursor-pointer border-none bg-transparent p-0 text-left"
>
{children}
</button>

{isOpen && (
<FloatingPortal>
<FloatingOverlay
Expand Down
Loading
Loading