ReviewOS

stacks/lastsoulultra

workspace-nav is the only nav still on plain anchors, so every link full-page-loads

#24
Closed glennmichael123 opened this 4 days ago ·
4 days ago

workspace-nav.stx is the one navigation component still on plain anchors, so every link in it does a full page load while the rest of the app navigates client side.

Counted across the navigation components in hq.training:

componentStxLinkplain <a href>
resources/components/app-nav.stx280
resources/components/marketing-nav.stx250
resources/components/workspace-nav.stx09

Repo wide it is 392 StxLink against 250 plain anchors, so StxLink is clearly the house idiom and the two main navigations are fully converted. This one looks like it was simply missed.

Not part of this

Most of the remaining 250 anchors are defensible and should stay as they are: 9 external, 11 in-page # anchors, 3 mailto, 5 query-string (the footer language switcher among them, which calls preventDefault anyway), and 64 dynamic expressions. Of the 57 internal paths, nearly all sit in the public and SEO pages ([username].stx, profile.stx, explore/*, blog/[slug].stx, share/[token].stx), where a real anchor is arguably correct for crawlers and cold landings.

So this is specifically about the nine in workspace-nav.stx.

4 days ago

Closing this: I was wrong, and workspace-nav.stx is correct as written.

I counted anchors against StxLink and concluded the component had been missed in a migration. I did not read what the hrefs resolve to. Every in-app link in there is :href="workspaceUrl(...)", and workspaceUrl (resources/functions/workspace-brand.ts:15) can return a URL on a different origin:

else if (['/privacy', '/terms'].includes(url.pathname)) return brand.settings[... 'privacy_url' : 'terms_url']
else if (url.pathname === '/welcome' || ...)              return new URL('/docs/getting-started', brand.settings.home_url).href
else if (url.pathname === '/explore' || url.pathname === '/app' || ...) return new URL('/docs', brand.settings.home_url).href
else if (url.pathname === '/store' || ...)                return brand.settings.home_url
else url.searchParams.set('brand', brand.slug)            // same origin

This component renders a white-labelled workspace, so the same call site resolves to an internal path for one brand and an external absolute URL for another, decided at runtime by brand settings. A client router cannot navigate the external cases. A plain <a href> handles both, which is presumably why it is one.

The two remaining anchors in the file are explicitly external already: the brand logo pointing at settings.home_url, and a Guide link with target="_blank" rel="noopener".

So the nine anchors are deliberate, not drift. My apologies for the noise.

The general observation in #25 still stands, but this changes what a lint rule there would have to allow. Noted over there.

Sign in to comment on this issue.