Skip to content

Commit 74c9412

Browse files
rseseOctomerger
andauthored
Use Primer ActionList for sidebar (#22885)
* Initial move to ActionLists nav, handle terminal links * ActionList-ify map topic items * Some 🎨 * ActionList-ify homepage sidebar * More 🎨 * Resolve test failures (and 'better' markup?) * Properties don't exist on ActionList * Use ul + li elements for ActionLists * Workaround for TS error with 'as=' Co-authored-by: Octomerger Bot <63058869+Octomerger@users.noreply.github.com>
1 parent 38a3afe commit 74c9412

2 files changed

Lines changed: 84 additions & 59 deletions

File tree

Lines changed: 44 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { useRouter } from 'next/router'
22
import { LinkExternalIcon } from '@primer/octicons-react'
3+
import { ActionList } from '@primer/components'
34

45
import { useVersion } from 'components/hooks/useVersion'
56
import { useMainContext } from 'components/context/MainContext'
@@ -11,43 +12,53 @@ export const SidebarHomepage = () => {
1112
const router = useRouter()
1213
const { currentVersion } = useVersion()
1314
const { activeProducts, isFPT } = useMainContext()
15+
const navItems = []
16+
17+
for (let i = 0; i < activeProducts.length; i++) {
18+
const product = activeProducts[i]
19+
20+
if (!isFPT && !product.versions?.includes(currentVersion) && !product.external) {
21+
continue
22+
}
23+
24+
const href = `${!product.external ? `/${router.locale}` : ''}${
25+
product.versions?.includes(currentVersion) && !isFPT
26+
? `/${currentVersion}/${product.id}`
27+
: product.href
28+
}`
29+
30+
navItems.push({
31+
renderItem: () => (
32+
<ActionList.Item
33+
as="li"
34+
key={product.id}
35+
title={`${product.name}${product.external ? '(External Site)' : ''}`}
36+
className="my-2"
37+
sx={{ padding: 0 }}
38+
>
39+
<Link
40+
href={href}
41+
target={product.external ? '_blank' : undefined}
42+
className="f4 pl-4 pr-5 py-2 color-fg-default no-underline width-full"
43+
>
44+
{product.name}
45+
{product.external && (
46+
<span className="ml-1">
47+
<LinkExternalIcon size="small" />
48+
</span>
49+
)}
50+
</Link>
51+
</ActionList.Item>
52+
),
53+
})
54+
}
1455

1556
return (
1657
<ul data-testid="sidebar" className="mt-4">
1758
{!isFPT && <AllProductsLink />}
18-
19-
{activeProducts.map((product) => {
20-
if (!isFPT && !product.versions?.includes(currentVersion) && !product.external) {
21-
return null
22-
}
23-
24-
const href = `${!product.external ? `/${router.locale}` : ''}${
25-
product.versions?.includes(currentVersion) && !isFPT
26-
? `/${currentVersion}/${product.id}`
27-
: product.href
28-
}`
29-
30-
return (
31-
<li
32-
key={product.id}
33-
title={`${product.name}${product.external ? '(External Site)' : ''}`}
34-
className="my-3"
35-
>
36-
<Link
37-
href={href}
38-
target={product.external ? '_blank' : undefined}
39-
className="f4 pl-4 pr-5 py-2 color-fg-default no-underline"
40-
>
41-
{product.name}
42-
{product.external && (
43-
<span className="ml-1">
44-
<LinkExternalIcon size="small" />
45-
</span>
46-
)}
47-
</Link>
48-
</li>
49-
)
50-
})}
59+
<li>
60+
<ActionList {...{ as: 'ul' }} items={navItems}></ActionList>
61+
</li>
5162
</ul>
5263
)
5364
}

components/sidebar/SidebarProduct.tsx

Lines changed: 40 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { useRouter } from 'next/router'
22
import cx from 'classnames'
33
import { useState, useEffect, SyntheticEvent } from 'react'
44
import { ChevronDownIcon } from '@primer/octicons-react'
5+
import { ActionList } from '@primer/components'
56

67
import { Link } from 'components/Link'
78
import { ProductTreeNode, useMainContext } from 'components/context/MainContext'
@@ -114,28 +115,37 @@ const CollapsibleSection = (props: SectionProps) => {
114115
const title = page.renderedShortTitle || page.renderedFullTitle
115116

116117
const isCurrent = routePath === page.href
117-
return (
118-
<li
119-
data-testid="sidebar-article"
120-
data-is-current-page={isCurrent}
121-
key={page.href}
122-
className={cx(
123-
'position-relative',
124-
styles.sidebarArticle,
125-
isCurrent && ['text-bold', styles.sidebarArticleActive]
126-
)}
127-
>
128-
<Link
129-
href={page.href}
118+
return {
119+
text: title,
120+
renderItem: () => (
121+
<ActionList.Item
122+
data-testid="sidebar-article"
123+
data-is-current-page={isCurrent}
124+
as="li"
130125
className={cx(
131-
'd-block pl-6 pr-5 py-1 no-underline',
132-
isCurrent ? 'color-fg-accent' : 'color-fg-default'
126+
'position-relative',
127+
styles.sidebarArticle,
128+
isCurrent && ['text-bold', styles.sidebarArticleActive]
133129
)}
130+
sx={{
131+
padding: '2px 0',
132+
':hover': {
133+
borderRadius: 0,
134+
},
135+
}}
134136
>
135-
{title}
136-
</Link>
137-
</li>
138-
)
137+
<Link
138+
href={page.href}
139+
className={cx(
140+
'd-block pl-6 pr-5 py-1 no-underline width-full',
141+
isCurrent ? 'color-fg-accent' : 'color-fg-default'
142+
)}
143+
>
144+
{title}
145+
</Link>
146+
</ActionList.Item>
147+
),
148+
}
139149
}
140150

141151
return (
@@ -172,19 +182,23 @@ const CollapsibleSection = (props: SectionProps) => {
172182
<summary>
173183
<div className={cx('pl-4 pr-5 py-2 no-underline')}>{childTitle}</div>
174184
</summary>
175-
<ul data-testid="sidebar-article-group" className="my-2 pb-2">
176-
{childPage.childPages.map(renderTerminalPageLink)}
177-
</ul>
185+
<div data-testid="sidebar-article-group" className="pb-0">
186+
<ActionList
187+
{...{ as: 'ul' }}
188+
items={childPage.childPages.map((cp) => {
189+
return renderTerminalPageLink(cp)
190+
})}
191+
></ActionList>
192+
</div>
178193
</details>
179194
</li>
180195
)
181196
})}
182197
</ul>
183198
) : page.childPages[0]?.page.documentType === 'article' ? (
184-
<ul data-testid="sidebar-article-group" className="list-style-none pb-2">
185-
{/* <!-- some categories have no maptopics, only articles --> */}
186-
{page.childPages.map(renderTerminalPageLink)}
187-
</ul>
199+
<div data-testid="sidebar-article-group" className="pb-0">
200+
<ActionList {...{ as: 'ul' }} items={page.childPages.map(renderTerminalPageLink)} />
201+
</div>
188202
) : null}
189203
</>
190204
}

0 commit comments

Comments
 (0)