mirror of
https://github.com/supabase/supabase.git
synced 2026-10-05 09:25:06 +03:00
fix(docs): guides sidebar a11y elements (#49942)
## What kind of change does this PR introduce? bug fix (accessibility) + test coverage ## What is the current behavior? the guides sidebar renders invalid list markup: group headers and dividers sit directly under the root `ul`, and accordion links render as `li` elements without an owning list fixes [DOCS-1279](https://linear.app/supabase/issue/DOCS-1279/guides-sidebar-put-li-elements-directly-in-the-ul) ## What is the new behavior? - sidebar renders a semantic hierarchy: every `ul` has only `li` children, every `li` has an immediate list parent, and the menu header sits outside the item list. pure markup change, - docs e2e scans the guide navigation separately from the article and blocks the `list` and `listitem` axe rules there against sample pages that include different usages (flat links, grouped links, nested accordion) ## How to test? run the docs dev server, then the scoped a11y suite: ```bash pnpm dev:docs pnpm e2e:docs:a11y ``` ## Follow up visuals and behavior are unchanged here but better parity between guide/reference is handled in the stacked pr <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved documentation navigation rendering for nested guide items, active states, and disabled entries. * Ensured navigation groups and child links use valid, testable list structures. * **Tests** * Added coverage verifying that guide navigation changes run the appropriate documentation pages. * Confirmed unrelated documentation changes can be skipped by the end-to-end workflow. * **Chores** * Updated documentation test scope detection to include guide navigation changes. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
1 parent
a78472ba7c
commit
bdd4b8d369
3 files changed
+130
-91
No files matched your search
@@ -43,9 +43,10 @@ const ContentAccordionLink = React.memo(function ContentAccordionLink(props: any
|
||||
const { resolvedTheme } = useTheme()
|
||||
const activeItem = props.subItem.url === pathname
|
||||
const activeItemRef = useRef<HTMLLIElement>(null)
|
||||
const hasChildren = props.subItem.items && props.subItem.items.length > 0
|
||||
|
||||
const isChildActive =
|
||||
props.subItem.items &&
|
||||
hasChildren &&
|
||||
props.subItem.items.some((child: NavAccordionItem) => hasActiveDescendant(child, pathname))
|
||||
|
||||
const LinkContainer = (props) => {
|
||||
@@ -73,16 +74,8 @@ const ContentAccordionLink = React.memo(function ContentAccordionLink(props: any
|
||||
}
|
||||
})
|
||||
return (
|
||||
<>
|
||||
{props.subItemIndex === 0 && (
|
||||
<>
|
||||
<div className="h-px w-full bg-border my-3"></div>
|
||||
<span className="font-mono text-xs uppercase text-foreground font-medium tracking-wider">
|
||||
{props.parent.name}
|
||||
</span>
|
||||
</>
|
||||
)}
|
||||
{props.subItem.items && props.subItem.items.length > 0 ? (
|
||||
<li ref={!hasChildren && activeItem ? activeItemRef : null}>
|
||||
{hasChildren ? (
|
||||
<Accordion.Root
|
||||
collapsible
|
||||
type="single"
|
||||
@@ -115,67 +108,60 @@ const ContentAccordionLink = React.memo(function ContentAccordionLink(props: any
|
||||
</span>
|
||||
</Accordion.Trigger>
|
||||
<Accordion.Content className="transition data-open:animate-slide-down data-closed:animate-slide-up ml-2">
|
||||
{props.subItem.items
|
||||
.filter((subItem) => subItem.enabled !== false)
|
||||
.map((subSubItem) => {
|
||||
if (subSubItem.items && subSubItem.items.length > 0) {
|
||||
return (
|
||||
<ContentAccordionLink
|
||||
key={subSubItem.name}
|
||||
subItem={subSubItem}
|
||||
subItemIndex={-1}
|
||||
parent={props.subItem}
|
||||
/>
|
||||
)
|
||||
}
|
||||
<ul>
|
||||
{props.subItem.items
|
||||
.filter((subItem) => subItem.enabled !== false)
|
||||
.map((subSubItem) => {
|
||||
if (subSubItem.items && subSubItem.items.length > 0) {
|
||||
return <ContentAccordionLink key={subSubItem.name} subItem={subSubItem} />
|
||||
}
|
||||
|
||||
return (
|
||||
<li key={`${props.subItem.name}-${subSubItem.url}`}>
|
||||
<Link
|
||||
href={`${subSubItem.url}`}
|
||||
className={[
|
||||
'cursor-pointer transition text-sm',
|
||||
subSubItem.url === pathname
|
||||
? 'text-brand-link'
|
||||
: 'hover:text-brand-link text-foreground-lighter',
|
||||
].join(' ')}
|
||||
>
|
||||
{subSubItem.name}
|
||||
</Link>
|
||||
</li>
|
||||
)
|
||||
})}
|
||||
return (
|
||||
<li key={`${props.subItem.name}-${subSubItem.url}`}>
|
||||
<Link
|
||||
href={`${subSubItem.url}`}
|
||||
className={[
|
||||
'cursor-pointer transition text-sm',
|
||||
subSubItem.url === pathname
|
||||
? 'text-brand-link'
|
||||
: 'hover:text-brand-link text-foreground-lighter',
|
||||
].join(' ')}
|
||||
>
|
||||
{subSubItem.name}
|
||||
</Link>
|
||||
</li>
|
||||
)
|
||||
})}
|
||||
</ul>
|
||||
</Accordion.Content>
|
||||
</Accordion.Item>
|
||||
</Accordion.Root>
|
||||
) : (
|
||||
<li key={props.subItem.name} ref={activeItem ? activeItemRef : null}>
|
||||
<LinkContainer
|
||||
url={props.subItem.url}
|
||||
className={[
|
||||
'flex items-center gap-2',
|
||||
'cursor-pointer transition text-sm',
|
||||
activeItem
|
||||
? 'text-brand-link font-medium'
|
||||
: 'hover:text-foreground text-foreground-lighter',
|
||||
].join(' ')}
|
||||
parent={props.subItem.parent}
|
||||
>
|
||||
<div className="flex items-center gap-2">
|
||||
{props.subItem.icon && (
|
||||
<Image
|
||||
alt={props.subItem.name}
|
||||
src={`${props.subItem.icon}${!resolvedTheme?.includes('dark') ? '-light' : ''}.svg`}
|
||||
width={15}
|
||||
height={15}
|
||||
/>
|
||||
)}
|
||||
{props.subItem.name}
|
||||
</div>
|
||||
</LinkContainer>
|
||||
</li>
|
||||
<LinkContainer
|
||||
url={props.subItem.url}
|
||||
className={[
|
||||
'flex items-center gap-2',
|
||||
'cursor-pointer transition text-sm',
|
||||
activeItem
|
||||
? 'text-brand-link font-medium'
|
||||
: 'hover:text-foreground text-foreground-lighter',
|
||||
].join(' ')}
|
||||
parent={props.subItem.parent}
|
||||
>
|
||||
<div className="flex items-center gap-2">
|
||||
{props.subItem.icon && (
|
||||
<Image
|
||||
alt={props.subItem.name}
|
||||
src={`${props.subItem.icon}${!resolvedTheme?.includes('dark') ? '-light' : ''}.svg`}
|
||||
width={15}
|
||||
height={15}
|
||||
/>
|
||||
)}
|
||||
{props.subItem.name}
|
||||
</div>
|
||||
</LinkContainer>
|
||||
)}
|
||||
</>
|
||||
</li>
|
||||
)
|
||||
})
|
||||
|
||||
@@ -210,7 +196,7 @@ const Content = (props) => {
|
||||
}
|
||||
|
||||
return (
|
||||
<ul className={['relative w-full flex flex-col gap-0 pb-5'].join(' ')}>
|
||||
<div className="relative w-full flex flex-col gap-0 pb-5">
|
||||
<Link href={menu.url ?? ''}>
|
||||
<div className="flex items-center gap-3 my-3 text-brand-link">
|
||||
<MenuIconPicker icon={menu.icon} />
|
||||
@@ -218,33 +204,35 @@ const Content = (props) => {
|
||||
</div>
|
||||
</Link>
|
||||
|
||||
{menu.items
|
||||
.filter((item) => item.enabled !== false)
|
||||
.map((x) => {
|
||||
return (
|
||||
<div key={x.name}>
|
||||
{x.items && x.items.length > 0 ? (
|
||||
<ul data-testid="docs-guide-navigation-list" className="flex flex-col gap-0">
|
||||
{menu.items.map((x) => {
|
||||
if (x.enabled === false) return null
|
||||
|
||||
if (x.items && x.items.length > 0) {
|
||||
const enabledItems = x.items.filter((item) => item.enabled !== false)
|
||||
if (enabledItems.length === 0) return null
|
||||
|
||||
return (
|
||||
<li key={x.name}>
|
||||
<div className="flex flex-col gap-2.5">
|
||||
{x.items
|
||||
.filter((item) => item.enabled !== false)
|
||||
.map((subItem, subItemIndex) => {
|
||||
return (
|
||||
<ContentAccordionLink
|
||||
key={subItem.name}
|
||||
subItem={subItem}
|
||||
subItemIndex={subItemIndex}
|
||||
parent={x}
|
||||
/>
|
||||
)
|
||||
<div className="h-px w-full bg-border my-3"></div>
|
||||
<span className="font-mono text-xs uppercase text-foreground font-medium tracking-wider">
|
||||
{x.name}
|
||||
</span>
|
||||
<ul className="flex flex-col gap-2.5">
|
||||
{enabledItems.map((subItem) => {
|
||||
return <ContentAccordionLink key={subItem.name} subItem={subItem} />
|
||||
})}
|
||||
</ul>
|
||||
</div>
|
||||
) : x.url ? (
|
||||
<ContentLink url={x.url} icon={x.icon} name={x.name} key={x.name} />
|
||||
) : null}
|
||||
</div>
|
||||
)
|
||||
</li>
|
||||
)
|
||||
}
|
||||
|
||||
return x.url ? <ContentLink url={x.url} icon={x.icon} name={x.name} key={x.name} /> : null
|
||||
})}
|
||||
</ul>
|
||||
</ul>
|
||||
</div>
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,31 @@
|
||||
import assert from 'node:assert/strict'
|
||||
import test from 'node:test'
|
||||
import { fileURLToPath } from 'node:url'
|
||||
|
||||
import {
|
||||
GUIDE_LIST_COMPONENT,
|
||||
GUIDE_LIST_COMPONENT_PAGES,
|
||||
resolveDocsScope,
|
||||
} from './resolve-docs-scope.ts'
|
||||
|
||||
const repoRoot = fileURLToPath(new URL('../../..', import.meta.url))
|
||||
|
||||
test('maps a guide list component change to its sample pages', async () => {
|
||||
const result = await resolveDocsScope({
|
||||
changedFiles: [GUIDE_LIST_COMPONENT],
|
||||
repoRoot,
|
||||
})
|
||||
|
||||
assert.deepEqual(result.pages, [...GUIDE_LIST_COMPONENT_PAGES].sort())
|
||||
assert.equal(result.skip, false)
|
||||
})
|
||||
|
||||
test('ignores unrelated docs component changes', async () => {
|
||||
const result = await resolveDocsScope({
|
||||
changedFiles: ['apps/docs/components/Navigation/NavigationMenu/NavigationMenu.constants.ts'],
|
||||
repoRoot,
|
||||
})
|
||||
|
||||
assert.deepEqual(result.pages, [])
|
||||
assert.equal(result.skip, true)
|
||||
})
|
||||
@@ -19,6 +19,14 @@ const PARTIALS_PREFIX = 'apps/docs/content/_partials/'
|
||||
const DOCS_GUIDES_URL_PREFIX = '/docs/guides/'
|
||||
const DOCS_TROUBLESHOOTING_URL_PREFIX = '/docs/guides/troubleshooting/'
|
||||
const FEDERATED_CONTENT_SOURCES_DIR = 'apps/docs/scripts/federated-content/sources'
|
||||
export const GUIDE_LIST_COMPONENT =
|
||||
'apps/docs/components/Navigation/NavigationMenu/NavigationMenuGuideListItems.tsx'
|
||||
|
||||
export const GUIDE_LIST_COMPONENT_PAGES = [
|
||||
'/docs/guides/getting-started/api-keys',
|
||||
'/docs/guides/getting-started/quickstarts/nextjs',
|
||||
'/docs/guides/integrations/build-a-supabase-oauth-integration/oauth-scopes',
|
||||
]
|
||||
|
||||
const PARTIAL_PATH_RE = /<\$Partial\b[\s\S]*?\bpath\s*=\s*"([^"]+)"[\s\S]*?\/?>/g
|
||||
const SOURCE_SECTION_RE = /\bsection:\s*'([^']+)'/g
|
||||
@@ -246,9 +254,15 @@ export async function resolveDocsScope(
|
||||
|
||||
const maxPages = options.maxPages ?? MAX_SCOPED_PAGES
|
||||
const pages = new Set<string>()
|
||||
const requiredPages = new Set<string>()
|
||||
const changedPartials: string[] = []
|
||||
|
||||
for (const file of options.changedFiles) {
|
||||
if (normalizeRepoPath(file) === GUIDE_LIST_COMPONENT) {
|
||||
for (const page of GUIDE_LIST_COMPONENT_PAGES) requiredPages.add(page)
|
||||
continue
|
||||
}
|
||||
|
||||
const page = changedFileToPagePath(file)
|
||||
if (page) {
|
||||
pages.add(page)
|
||||
@@ -271,7 +285,13 @@ export async function resolveDocsScope(
|
||||
}
|
||||
}
|
||||
|
||||
const sorted = [...pages].sort().slice(0, maxPages)
|
||||
const selectedRequiredPages = [...requiredPages].sort().slice(0, maxPages)
|
||||
const remainingSlots = Math.max(0, maxPages - selectedRequiredPages.length)
|
||||
const selectedPages = [...pages]
|
||||
.filter((page) => !requiredPages.has(page))
|
||||
.sort()
|
||||
.slice(0, remainingSlots)
|
||||
const sorted = [...selectedRequiredPages, ...selectedPages].sort()
|
||||
|
||||
return {
|
||||
pages: sorted,
|
||||
|
||||
Reference in new issue
Block a user