oxidecomputer / console

Oxide Web Console
https://console-preview.oxide.computer
Mozilla Public License 2.0
129 stars 10 forks source link

Convert QueryParamTabs to route tabs #1913

Open benjaminleonard opened 7 months ago

benjaminleonard commented 7 months ago

Sometimes for tabs we use params:

/projects/mock-project/vpcs/mock-vpc?tab=firewall-rules

and sometimes pages:

/projects/mock-project/instances/db1/storage

We should either land on a single approach or document why we use which approach in which context.

david-crespo commented 4 months ago

I think we should probably consolidate on route tabs, which have the advantage of letting you nest further routes under them. This would let us, for example, give routes to the firewall rules create and edit forms.

$ rg '<QueryParamTabs'
app/pages/system/silos/SiloPage.tsx
76:      <QueryParamTabs className="full-width" defaultValue="idps">

app/pages/system/networking/IpPoolPage.tsx
79:      <QueryParamTabs className="full-width" defaultValue="ranges">

app/pages/system/UtilizationPage.tsx
57:      <QueryParamTabs defaultValue="summary" className="full-width">

app/pages/project/vpcs/VpcPage/VpcPage.tsx
68:      <QueryParamTabs className="full-width" defaultValue="firewall-rules">
$ rg '<RouteTabs'
app/pages/system/inventory/sled/SledPage.tsx
72:      <RouteTabs fullWidth>

app/pages/system/inventory/InventoryPage.tsx
33:      <RouteTabs fullWidth>

app/pages/project/instances/instance/InstancePage.tsx
198:      <RouteTabs fullWidth>

Our canonical solution for the linking problem is that we simply make the route of the default tab the default route for that overall page. For example, here we would change the inventory route to /system/inventory/sleds and just don't consider /system/inventory a real route (we can include a little client-side redirect in the route config just to be polite).

https://github.com/oxidecomputer/console/blob/e8e4f576e6ee5c3e3544eaf5230a71e07c79a428/app/util/path-builder.ts#L97-L100