Skip to content
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

Fix: Selected framework in Code component #12496

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fix: Selected framework in Code component #12496

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3b33041
Fix: Selected framework in Code component
gkettani Jan 12, 2025
a432e56
Apply review suggestions/requests
gkettani Jan 16, 2025
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
53 changes: 44 additions & 9 deletions docs/components/Code/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { useSearchParams } from "next/navigation"
import { useRouter } from "next/router"
import { useThemeConfig } from "nextra-theme-docs"
import { Tabs } from "nextra/components"
import React, { Children, useEffect, MouseEvent } from "react"
import React, { Children, useEffect, MouseEvent, useState } from "react"

interface ChildrenProps {
children: React.ReactNode
Expand Down Expand Up @@ -58,28 +58,61 @@ export function Code({ children }: ChildrenProps) {

const renderedFrameworks = withNextJsPages ? allFrameworks : baseFrameworks

const [selectedFramework, setSelectedFramework] = useState<string>(
Object.values(renderedFrameworks)[0]
)

const updateFrameworkStorage = (value: string): void => {
const params = new URLSearchParams(searchParams?.toString())
params.set("framework", value)
Copy link
Contributor

@halvaradop halvaradop Jan 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

To avoid repeating the use of parseParams throughout the code, please move it to the updateFrameworkStorage function.

router.push(`${router.pathname}?${params.toString()}`)
}

const handleFrameworkChange = (framework: string) => {
window.localStorage.setItem(AUTHJS_TAB_KEY, framework)
window.dispatchEvent(
new StorageEvent("storage", {
key: AUTHJS_TAB_KEY,
newValue: framework,
})
)
updateFrameworkStorage(parseParams(framework))
}

const handleClickFramework = (event: MouseEvent<HTMLDivElement>) => {
if (!(event.target instanceof HTMLButtonElement)) return
const { textContent } = event.target as unknown as HTMLDivElement
updateFrameworkStorage(parseParams(textContent!))
if (textContent) {
handleFrameworkChange(textContent)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Check if the selected value is within renderedFrameworks, as selecting a tab in the package manager will update the framework value.

}

useEffect(() => {
const length = Object.keys(renderedFrameworks).length
function handleStorage(event: StorageEvent) {
if (
event.key === AUTHJS_TAB_KEY &&
event.newValue &&
Object.values(renderedFrameworks).includes(event.newValue)
) {
setSelectedFramework(event.newValue)
}
}
window.addEventListener("storage", handleStorage)
return () => {
window.removeEventListener("storage", handleStorage)
}
}, [])
Comment on lines 93 to +107
Copy link
Contributor

@halvaradop halvaradop Jan 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I think this logic is unnecessary because changing the framework in the tabs will automatically update the selected framework, regardless of the active tab or whether you have instances of the page open in different windows on your computer. It will synchronize the updates.

Copy link
Author

Choose a reason for hiding this comment

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

You're correct; that was indeed the case when we used the storageKey prop in the Tabs component. The Tabs component managed the synchronization logic across all tabs.

next-auth/docs/components/Code/index.tsx

Lines 90 to 93 in 9d704a0

<Tabs
storageKey={AUTHJS_TAB_KEY}
items={Object.values(renderedFrameworks)}
>

However, because the Tabs component relies on an index to track and sync the selected tab, this approach caused the issue we’re addressing.
To resolve this, I extracted the synchronization logic from the Tabs component and adapted it to our use case. During this process, I reviewed the implementation of the Tabs component to understand its behavior and made targeted adjustments to ensure it works properly in our context, specifically by replacing the index-based tracking with framework names.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for your explanation, that makes sense. When you remove the storageKey prop from Tabs, it becomes necessary to configure how the selected value is updated. For this, you’ve used the selectedIndex prop, which allows changing the selected value. However, by not using the storageKey prop, you’ve synchronized the selected value between the tabs of the page using the storage event to address the issue. Only the currently active tab updates the value, while the others remain unchanged. This is a great approach to solving the problem!


useEffect(() => {
const getFrameworkStorage = window.localStorage.getItem(AUTHJS_TAB_KEY)
const indexFramework = parseInt(getFrameworkStorage ?? "0") % length
if (!getFrameworkStorage) {
window.localStorage.setItem(AUTHJS_TAB_KEY, "0")
handleFrameworkChange(Object.values(renderedFrameworks)[0])
Copy link
Contributor

Choose a reason for hiding this comment

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

You can simply use the value selectedFramework

} else if (
Object.values(renderedFrameworks).includes(getFrameworkStorage)
) {
setSelectedFramework(getFrameworkStorage)
updateFrameworkStorage(parseParams(getFrameworkStorage))
}
updateFrameworkStorage(
parseParams(Object.values(renderedFrameworks)[indexFramework])
)
}, [router.pathname, renderedFrameworks])

return (
Expand All @@ -88,7 +121,9 @@ export function Code({ children }: ChildrenProps) {
onClick={handleClickFramework}
>
<Tabs
storageKey={AUTHJS_TAB_KEY}
selectedIndex={Object.values(renderedFrameworks).indexOf(
selectedFramework
)}
items={Object.values(renderedFrameworks)}
>
{Object.keys(renderedFrameworks).map((f) => {
Expand Down
Loading