Skip to content

Commit 8f8c327

Browse files
committed
today is the day to be less stupid. don't detect system route by looking at strings
1 parent f0cdf82 commit 8f8c327

File tree

4 files changed

+19
-12
lines changed

4 files changed

+19
-12
lines changed

app/components/TopBar.tsx

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { Menu, MenuButton, MenuItem, MenuList } from '@reach/menu-button'
2-
import { useLocation, useNavigate, useParams } from 'react-router-dom'
2+
import { useNavigate, useParams } from 'react-router-dom'
33

44
import { navToLogin, useApiMutation, useApiQuery } from '@oxide/api'
55
import {
@@ -15,7 +15,19 @@ import { pb } from 'app/util/path-builder'
1515

1616
import { OrgPicker, ProjectPicker, SiloSystemPicker } from './TopBarPicker'
1717

18-
export function TopBar() {
18+
type TopBarProps = {
19+
/**
20+
* Passing around flags like this is definitely a code smell, but this is the
21+
* simplest thing for now. This allows `SystemLayout` to tell us when we're on
22+
* a system route (instead of detecting it in an unreliable way by, for
23+
* example, checking what the pathname starts with) while at the same time
24+
* keeping the logic for showing/hiding the system/silo picker based on the
25+
* current user's permissions bundled up in here.
26+
*/
27+
isSystemRoute?: boolean
28+
}
29+
30+
export function TopBar({ isSystemRoute = false }: TopBarProps) {
1931
const navigate = useNavigate()
2032
const logout = useApiMutation('logout', {
2133
onSuccess: () => {
@@ -42,20 +54,18 @@ export function TopBar() {
4254
// would see the silo picker pop in when the request resolves. Bad.
4355
const isFleetViewer = !!systemPolicy
4456

45-
const isSystem = pb.isSystemPath(useLocation().pathname)
46-
4757
const { projectName } = useParams()
4858

4959
const [cornerPicker, ...otherPickers] = [
50-
isFleetViewer && <SiloSystemPicker isSystem={isSystem} key={0} />,
60+
isFleetViewer && <SiloSystemPicker isSystemRoute={isSystemRoute} key={0} />,
5161
// TODO: This works ok in most situations, but when an operator user is on
5262
// the orgs page with no org selected, they see this picker, which is
5363
// redundant with the list of orgs. Overall this logic is starting to feel
5464
// silly, which points to a non-centralized approach handled in the layouts
5565
// like we were doing before. That way, for example, we know whether we're
5666
// on a system situation because we're in SystemLayout. Seems pretty obvious
5767
// in hindsight.
58-
!isSystem && <OrgPicker key={1} />,
68+
!isSystemRoute && <OrgPicker key={1} />,
5969
projectName && <ProjectPicker key={2} />,
6070
].filter(isTruthy)
6171

app/components/TopBarPicker.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ const NoOrgLogo = () => (
104104
</div>
105105
)
106106

107-
export function SiloSystemPicker({ isSystem }: { isSystem: boolean }) {
107+
export function SiloSystemPicker({ isSystemRoute }: { isSystemRoute: boolean }) {
108108
const commonProps = {
109109
items: [
110110
{ label: 'System', to: pb.silos() },
@@ -113,7 +113,7 @@ export function SiloSystemPicker({ isSystem }: { isSystem: boolean }) {
113113
'aria-label': 'Switch between system and silo',
114114
}
115115

116-
return isSystem ? (
116+
return isSystemRoute ? (
117117
<TopBarPicker
118118
{...commonProps}
119119
category="System"

app/layouts/SystemLayout.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ SystemLayout.loader = async () => {
3939
export default function SystemLayout() {
4040
return (
4141
<PageContainer>
42-
<TopBar />
42+
<TopBar isSystemRoute />
4343
<Sidebar>
4444
<Sidebar.Nav>
4545
<DocsLinkItem />

app/util/path-builder.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,9 +54,6 @@ export const pb = {
5454
sshKeys: () => '/settings/ssh-keys',
5555

5656
deviceSuccess: () => '/device/success',
57-
58-
// someday we'll be less stupid. not today
59-
isSystemPath: (pathname: string) => pathname.startsWith(pb.system()),
6057
}
6158

6259
// export const jelly = 'just kidding'

0 commit comments

Comments
 (0)